From 571553daba7c841e1db5bbbec828fdceb1218e88 Mon Sep 17 00:00:00 2001 From: Leo Wandersleb Date: Tue, 8 Sep 2026 20:33:39 +0200 Subject: [PATCH 1/4] Follow the round-trip gradient for the resource-fetch target too targetX/Y for a fetch task (also the debug path line, hotkey T) are set once, by ascending the plain resource gradient, when the task starts. pathfindResource() re-reads whichever gradient actually governs the per-tick step -- the round-trip field when attachedBuilding has one and it is valid here, the plain resource gradient otherwise -- and either field can be rebuilt, or the preference between them can flip, while the unit is still walking. The stored target never tracked either change, so the debug line and the range checks that read targetX/Y could point at the nearest tile to the unit while the unit actually walked toward the cheaper round trip through a farther one. Add Map::isGradientPeak: whether a tile is a local maximum of a gradient, true for anywhere getGlobalGradientDestination's ascent could end, including a round-trip field whose seeded goal is a finite cost rather than the type's max the way GRADIENT_AT_GOAL is (so its own 'reached exact goal' check does not generalize). handleMovementGoingToResource now resolves the same gradient pathfindResource prefers and re-ascends from the unit's position whenever the stored target stops being a peak of it -- a cheap check every action, the ascent itself only when stale. New ResourceFetchTargetHarness (test/, wired as the resource-fetch-target-test scons alias) seeds a tile close to the unit but far from the building and one far from the unit but close to it, confirms the target follows the cheaper round trip rather than the nearer tile, and that it refreshes once that tile is harvested away. Fails without the fix. Full test/TestsRunner suite (169 tests) passes; scons release=1 and release=1 server=1 build clean. (cherry picked from commit 6bb00b01a8d8cc55eaf02c04942691adc6b3413a) Fable 5.1 helped authoring this commit. --- src/map/Map.h | 7 ++- src/map/MapResources.cpp | 20 +++++++ src/unit/UnitMovement.cpp | 22 ++++++-- test/ResourceFetchTargetHarness.cpp | 81 +++++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 6 deletions(-) diff --git a/src/map/Map.h b/src/map/Map.h index 2e89ec4c2..e36356b01 100644 --- a/src/map/Map.h +++ b/src/map/Map.h @@ -619,7 +619,12 @@ class Map //! on the AIs' Uint8 maps alike: the goal is the maximum of the element type. template bool getGlobalGradientDestination(const T *gradient, int x, int y, Sint32 *targetX, Sint32 *targetY) const; - + //! Whether (x, y) is a local maximum of gradient: no neighbour holds a strictly higher + //! value. True at any tile getGlobalGradientDestination's ascent could end on, including + //! gradients like a round-trip field whose seeded goal is a finite cost, not the type's max. + template + bool isGradientPeak(const T *gradient, int x, int y) const; + Uint16 getGradient(int teamNumber, Uint8 resourceType, int swimClass, int x, int y) { return getResourceGradient(teamNumber, resourceType, swimClass)[coordToIndex(x, y)]; diff --git a/src/map/MapResources.cpp b/src/map/MapResources.cpp index 57d8c422c..ca552b150 100644 --- a/src/map/MapResources.cpp +++ b/src/map/MapResources.cpp @@ -252,5 +252,25 @@ bool Map::getGlobalGradientDestination(const T *gradient, int x, int y, Sint32 * template bool Map::getGlobalGradientDestination(const Uint8 *gradient, int x, int y, Sint32 *targetX, Sint32 *targetY) const; template bool Map::getGlobalGradientDestination(const Uint16 *gradient, int x, int y, Sint32 *targetX, Sint32 *targetY) const; +template +bool Map::isGradientPeak(const T *gradient, int x, int y) const +{ + // A round-trip gradient's goal is seeded at a finite cost, not the type's + // max the way GRADIENT_AT_GOAL is, so getGlobalGradientDestination's own + // "reached exact goal" check does not generalize to it. This is the + // weaker, gradient-agnostic property an ascent target actually needs: + // no neighbour holds a strictly higher value, so an ascent from anywhere + // nearby would still stop here. + size_t index = coordToIndex(x, y); + T here = gradient[index]; + for (int d=0; d<8; d++) + if (gradient[coordToIndex(x+deltaOne[d][0], y+deltaOne[d][1])]>here) + return false; + return true; +} + +template bool Map::isGradientPeak(const Uint8 *gradient, int x, int y) const; +template bool Map::isGradientPeak(const Uint16 *gradient, int x, int y) const; + diff --git a/src/unit/UnitMovement.cpp b/src/unit/UnitMovement.cpp index f82d50ad9..0c21e2a51 100644 --- a/src/unit/UnitMovement.cpp +++ b/src/unit/UnitMovement.cpp @@ -557,15 +557,27 @@ void Unit::handleMovementGoingToResource() { Map *map=owner->map; int teamNumber=owner->teamNumber; + int swim=swimClass(); bool stopWork; - if (map->pathfindResource(teamNumber, destinationPurpose, swimClass(), posX, posY, &dx, &dy, &stopWork, attachedBuilding)) + if (map->pathfindResource(teamNumber, destinationPurpose, swim, posX, posY, &dx, &dy, &stopWork, attachedBuilding)) { directionFromDxDy(); movement=MOV_GOING_DX_DY; - // Routing can follow a rebuilt gradient while the stored target is stale. - // Recompute the target only when it no longer marks a resource goal. - if (map->getGradient(teamNumber, destinationPurpose, swimClass(), targetX, targetY)!=GRADIENT_AT_GOAL) - map->resourceAvailableUpdate(teamNumber, destinationPurpose, swimClass(), posX, posY, &targetX, &targetY, NULL); + // targetX/Y (also the debug path line, hotkey T) were set once, by + // ascending a gradient, when the fetch task started. pathfindResource + // above re-reads whichever gradient actually governs the step fresh + // every action -- the round-trip field when attachedBuilding has one + // and it is valid here, the plain resource gradient otherwise -- and + // either field can be rebuilt, or the preference between them can + // flip, while the unit is still walking. Re-ascend from here whenever + // the stored target has stopped being a peak of that same gradient; + // isGradientPeak is a cheap check to run every action, the ascent + // itself only when it actually goes stale. + const Uint16 *roundTrip = attachedBuilding ? map->roundTripGradient(attachedBuilding, destinationPurpose, swim) : NULL; + const Uint16 *gradient = (roundTrip && roundTrip[map->coordToIndex(posX, posY)]>GRADIENT_UNREACHABLE) + ? roundTrip : map->getResourceGradient(teamNumber, destinationPurpose, swim); + if (!map->isGradientPeak(gradient, targetX, targetY)) + map->getGlobalGradientDestination(gradient, posX, posY, &targetX, &targetY); } else { diff --git a/test/ResourceFetchTargetHarness.cpp b/test/ResourceFetchTargetHarness.cpp index c3fdfc0c0..23e75d442 100644 --- a/test/ResourceFetchTargetHarness.cpp +++ b/test/ResourceFetchTargetHarness.cpp @@ -14,6 +14,7 @@ #include "GameGUI.h" #include "Unit.h" #include "Team.h" +#include "Building.h" #include "MapInternal.h" #include "Race.h" #include "Ressource.h" @@ -101,6 +102,85 @@ static void staleTargetIsRefreshedAfterGradientRebuild(int expectedClass, int sw std::puts("PASS resource-fetch target is refreshed when the gradient it was ascended from is rebuilt"); } +// targetX/Y (also the debug path line, hotkey T) are set once, by ascending +// a gradient, when a fetch task starts (Unit.cpp, UnitDisplacement.cpp). +// pathfindResource (UnitMovement.cpp) re-reads whichever gradient actually +// governs the unit's step fresh every action instead -- the building's +// round-trip field when it minimises fetch-plus-carry, the plain resource +// gradient otherwise -- and either field can be rebuilt, or the preference +// between them can flip, while the unit is still walking. +static void targetTracksTheGradientTheUnitActuallyFollows() +{ + GameGUI gui; + Game& game = gui.game; + game.map.setSize(5, 5, GRASS); // 32x32 + game.map.setGame(&game); + // setSize leaves immobileUnits[] zeroed, not IMMOBILE_UNIT_NONE (255), so + // every cell reads as immobile-unit-occupied until cleared; a real game + // never observes this because something else sweeps it first. + for (int y = 0; y < game.map.getH(); ++y) + for (int x = 0; x < game.map.getW(); ++x) + game.map.clearImmobileUnit(x, y); + game.addTeam(0); + Team* team = game.teams[0]; + const int teamNumber = team->teamNumber; + const int innType = globalContainer->buildingsTypes.getTypeNum("inn", 0, false); + require(innType >= 0, "inn type exists"); + Building* inn = game.addBuilding(5, 5, innType, 0); + require(inn != nullptr, "inn placed"); + game.map.setBuilding(5, 5, inn->type->width, inn->type->height, inn->gid); + + const int unitX = 16, unitY = 16; + // A is close to the unit but a long carry from the building; B is a + // longer fetch but a short carry, so the round trip through B is + // cheaper even though A is the nearer tile to ascend to from the unit. + const int nearUnitX = 20, nearUnitY = 16; + const int nearBuildingX = 7, nearBuildingY = 7; + require(game.map.incResource(nearUnitX, nearUnitY, CORN, 0), "seed the tile near the unit"); + require(game.map.incResource(nearBuildingX, nearBuildingY, CORN, 0), "seed the tile near the building"); + + TestUnit* unit = new TestUnit(unitX, unitY, 0, WORKER, team, 0); + team->myUnits[0] = unit; + game.map.setGroundUnit(unitX, unitY, unit->gid); + unit->attachedBuilding = inn; + unit->destinationPurpose = CORN; + unit->activity = Unit::ACT_FILLING; + unit->displacement = Unit::DIS_GOING_TO_RESOURCE; + unit->validTarget = true; + const int swimClass = unit->swimClass(); + + require(game.map.getGlobalGradientDestination(game.map.getResourceGradient(teamNumber, CORN, swimClass), unit->posX, unit->posY, &unit->targetX, &unit->targetY), + "sanity: ascending the plain gradient reaches an exact goal"); + require(unit->targetX == nearUnitX && unit->targetY == nearUnitY, + "sanity: the plain gradient's nearest tile is the one close to the unit, not the building"); + + // One action: pathfindResource builds and prefers the round-trip field, + // so the unit steps toward the tile that is cheaper to fetch and carry, + // and the target must follow it, not the plain gradient's nearer tile. + unit->stepGoingToResource(); + require(unit->targetX == nearBuildingX && unit->targetY == nearBuildingY, + "target follows the round-trip gradient's cheaper tile, not the nearest one to the unit"); + + // Another action while nothing changed: the target must hold steady. + unit->stepGoingToResource(); + require(unit->targetX == nearBuildingX && unit->targetY == nearBuildingY, + "target holds steady while still valid"); + + // The near-building tile gets fully harvested by someone else. Neither + // the resource gradient nor the round-trip field notice by themselves. + game.map.getTile(nearBuildingX, nearBuildingY).resource.clear(); + game.map.updateResourcesGradient(teamNumber, CORN, swimClass); + game.map.updateRoundTripGradient(inn, CORN, swimClass); + + // Next action: only the near-unit tile is left on either gradient: the + // target must be refreshed to it. + unit->stepGoingToResource(); + require(unit->targetX == nearUnitX && unit->targetY == nearUnitY, + "target is refreshed to the only remaining corn tile"); + + std::puts("PASS resource-fetch target tracks the round-trip gradient and refreshes when it is rebuilt"); +} + int main(int argc, char** argv) { SDL_SetMainReady(); @@ -117,6 +197,7 @@ int main(int argc, char** argv) const int swimSpeeds[] = {0, 20, 14, 10, 7, 5, 3}; for (int swimClass = 0; swimClass < SWIM_CLASS_COUNT; ++swimClass) staleTargetIsRefreshedAfterGradientRebuild(swimClass, swimSpeeds[swimClass]); + targetTracksTheGradientTheUnitActuallyFollows(); std::puts("Resource fetch target regressions passed"); return 0; } From bc68571813bf7e40ceabde5c8b992294767dead5 Mon Sep 17 00:00:00 2001 From: Leo Wandersleb Date: Tue, 8 Sep 2026 02:20:07 +0200 Subject: [PATCH 2/4] Swap fetching jobs between units when both trips get shorter A unit that becomes free takes the job of whichever building asks first, however far away, and units never trade jobs. Team::swapTask compares a fetcher's job with every team mate's: a job costs the round trip for an empty-handed unit or the delivery distance for one carrying that resource, and when the two trips together shrink by more than four tiles the units exchange building, resource and target and keep walking from where they are. It runs for a unit right after it is hired and, four units a tick, over every fetcher every 256 ticks. (cherry picked from commit 4ba5c919f3aee53dc1399cce49d9653885f96943) Fable 5.1 helped authoring this commit. --- src/building/Building.h | 3 +- src/building/Step.cpp | 1 + src/team/Team.h | 3 ++ src/team/TeamStep.cpp | 101 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 107 insertions(+), 1 deletion(-) diff --git a/src/building/Building.h b/src/building/Building.h index c30dea296..668b6177b 100644 --- a/src/building/Building.h +++ b/src/building/Building.h @@ -206,6 +206,8 @@ class Building : public BuildingUtils ///It is considered greedy, hiring as many units as it needs in order of its preference ///Returns true if a unit was hired bool subscribeToBringResourcesStep(void); + //! Whether the unit's type and level qualify it to work for this building. + bool canUnitWorkHere(Unit* unit); ///This function subscribes any flag that needs units. ///It is considered greedy, hiring as many units as it needs in order of its preference ///Returns true if a unit was hired @@ -396,7 +398,6 @@ class Building : public BuildingUtils /// Tells whether a particular unit can work at this building. Takes into account this buildings level, /// the units type and level, and whether this building is a flag, because flags get a couple of special /// rules. - bool canUnitWorkHere(Unit* unit); /// Per-zonable candidate-selection helpers for subscribeForFlagingStep. /// Each tests one unit against the per-flag-type requirements (activity, diff --git a/src/building/Step.cpp b/src/building/Step.cpp index b5fef96c4..c730b1ea8 100644 --- a/src/building/Step.cpp +++ b/src/building/Step.cpp @@ -288,6 +288,7 @@ bool Building::subscribeToBringResourcesStep() { unitsWorking.push_back(sel.choosen); sel.choosen->subscriptionSuccess(this, false); + owner->swapTask(sel.choosen); hired=true; } } diff --git a/src/team/Team.h b/src/team/Team.h index cccae5ce9..ec5d3c3a6 100644 --- a/src/team/Team.h +++ b/src/team/Team.h @@ -110,6 +110,9 @@ class Team:public BaseTeam void removeBuildingNeedingWork(Building* b, Sint32 priority); //! Update every building in buildingsNeedingUnits, highest priority first. void updateAllBuildingTasks(); + //! Give `unit`'s fetching job to a team mate and take the mate's job, when that + //! shortens the two trips together by more than a few tiles (see TeamStep.cpp). + void swapTask(Unit *unit); //! Highest build level any unit of the team has. int maxBuildLevel(void); diff --git a/src/team/TeamStep.cpp b/src/team/TeamStep.cpp index 00dad09cd..4f6462271 100644 --- a/src/team/TeamStep.cpp +++ b/src/team/TeamStep.cpp @@ -171,6 +171,105 @@ void Team::updateAllBuildingTasks() +namespace +{ + // A swap has to save this many tiles over the two trips to be worth the churn. + constexpr int SWAP_MIN_GAIN = 4; + // Fetchers checked for a swap per tick; every unit gets its turn every 256 ticks. + constexpr int SWAP_CHECKS_PER_TICK = Unit::MAX_COUNT / 256; + + // A unit on its way to fetch, or to deliver, a resource for its building. + bool isFetching(const Unit *u) + { + return u && u->activity == Unit::ACT_FILLING && u->attachedBuilding && u->ownExchangeBuilding == NULL + && u->medical == Unit::MED_FREE && u->destinationPurpose >= 0 + && (u->displacement == Unit::DIS_GOING_TO_RESOURCE || u->displacement == Unit::DIS_GOING_TO_BUILDING); + } + + // Tiles `u` would walk to do the job (building, resource): deliver what it + // carries, or fetch and carry. False when it cannot take the job. + bool jobCost(Unit *u, Building *b, int resource, int *cost) + { + Map *map = b->owner->map; + int swimClass = u->swimClass(); + if (u->carriedResource >= 0) + return u->carriedResource == resource && map->buildingAvailable(b, swimClass, u->posX, u->posY, cost); + if (map->roundTripDistance(b, resource, swimClass, u->posX, u->posY, cost)) + return true; + int toBuilding, toResource; + if (!map->buildingAvailable(b, swimClass, u->posX, u->posY, &toBuilding) + || !map->resourceAvailable(b->owner->teamNumber, resource, swimClass, u->posX, u->posY, &toResource)) + return false; + *cost = toBuilding + toResource; + return true; + } + + void assignTask(Unit *u, Building *b, int resource) + { + u->attachedBuilding->removeUnitFromWorking(u); + u->attachedBuilding = b; + u->destinationPurpose = resource; + b->unitsWorking.push_back(u); + b->updateCallLists(); + if (u->carriedResource == resource) + { + u->displacement = Unit::DIS_GOING_TO_BUILDING; + u->setTargetBuilding(b); + } + else + { + u->displacement = Unit::DIS_GOING_TO_RESOURCE; + u->setTargetBuilding(NULL); + b->owner->map->resourceAvailableUpdate(b->owner->teamNumber, resource, u->swimClass(), u->posX, u->posY, &u->targetX, &u->targetY, NULL); + } + u->validTarget = true; + } +} + +void Team::swapTask(Unit *unit) +{ + if (!isFetching(unit)) + return; + Building *a = unit->attachedBuilding; + int r = unit->destinationPurpose; + int own; + if (!jobCost(unit, a, r, &own)) + return; + int timeLeft = (unit->hungry - unit->trigHungry) / unit->race->hungriness; + int swimClass = unit->swimClass(); + Unit *best = NULL; + int bestGain = SWAP_MIN_GAIN; + for (int i = 0; i < Unit::MAX_COUNT; i++) + { + Unit *mate = myUnits[i]; + // Same swim class only: the costs then come from gradients the two + // fetchers already keep alive, and none is built for the comparison. + if (mate == unit || !isFetching(mate) || mate->swimClass() != swimClass) + continue; + Building *b = mate->attachedBuilding; + int s = mate->destinationPurpose; + if ((b == a && s == r) || !b->canUnitWorkHere(unit) || !a->canUnitWorkHere(mate)) + continue; + int mateOwn, mine, theirs; + if (!jobCost(mate, b, s, &mateOwn) || !jobCost(unit, b, s, &mine) || !jobCost(mate, a, r, &theirs)) + continue; + if (mine >= timeLeft || theirs >= (mate->hungry - mate->trigHungry) / mate->race->hungriness) + continue; + int gain = own + mateOwn - mine - theirs; + if (gain > bestGain) + { + bestGain = gain; + best = mate; + } + } + if (best == NULL) + return; + Building *b = best->attachedBuilding; + int s = best->destinationPurpose; + assignTask(unit, b, s); + assignTask(best, a, r); +} + void Team::syncStep(void) { integrity(); @@ -272,6 +371,8 @@ void Team::syncStep(void) } updateAllBuildingTasks(); + for (int k = 0; k < SWAP_CHECKS_PER_TICK; k++) + swapTask(myUnits[(game->stepCounter * SWAP_CHECKS_PER_TICK + k) % Unit::MAX_COUNT]); bool isEnoughFoodInSwarm=false; From adc7191de64d777dfc94bc707d0043a23cfac122 Mon Sep 17 00:00:00 2001 From: Leo Wandersleb Date: Tue, 8 Sep 2026 03:26:04 +0200 Subject: [PATCH 3/4] Swap jobs across swim classes when the gradients already exist Requiring the same swim class was only a proxy for what matters: that every cost in the comparison is read from a gradient some unit already keeps alive, so a comparison never builds one. Check that directly, so a swimmer and a walker swap whenever both fields are there. (cherry picked from commit 3613f934a47a5cacfad9c6484a358008e984def1) Fable 5.1 helped authoring this commit. --- src/team/TeamStep.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/team/TeamStep.cpp b/src/team/TeamStep.cpp index 4f6462271..5b6da637f 100644 --- a/src/team/TeamStep.cpp +++ b/src/team/TeamStep.cpp @@ -187,15 +187,19 @@ namespace } // Tiles `u` would walk to do the job (building, resource): deliver what it - // carries, or fetch and carry. False when it cannot take the job. + // carries, or fetch and carry. Only gradients that already exist are read, + // so a comparison never builds one. False when it cannot take the job. bool jobCost(Unit *u, Building *b, int resource, int *cost) { Map *map = b->owner->map; int swimClass = u->swimClass(); + if (b->globalGradient[swimClass] == NULL) + return false; if (u->carriedResource >= 0) return u->carriedResource == resource && map->buildingAvailable(b, swimClass, u->posX, u->posY, cost); if (map->roundTripDistance(b, resource, swimClass, u->posX, u->posY, cost)) return true; + // No round-trip field for this class yet: the plain distances, as hiring uses them. int toBuilding, toResource; if (!map->buildingAvailable(b, swimClass, u->posX, u->posY, &toBuilding) || !map->resourceAvailable(b->owner->teamNumber, resource, swimClass, u->posX, u->posY, &toResource)) @@ -236,15 +240,12 @@ void Team::swapTask(Unit *unit) if (!jobCost(unit, a, r, &own)) return; int timeLeft = (unit->hungry - unit->trigHungry) / unit->race->hungriness; - int swimClass = unit->swimClass(); Unit *best = NULL; int bestGain = SWAP_MIN_GAIN; for (int i = 0; i < Unit::MAX_COUNT; i++) { Unit *mate = myUnits[i]; - // Same swim class only: the costs then come from gradients the two - // fetchers already keep alive, and none is built for the comparison. - if (mate == unit || !isFetching(mate) || mate->swimClass() != swimClass) + if (mate == unit || !isFetching(mate)) continue; Building *b = mate->attachedBuilding; int s = mate->destinationPurpose; From 8278566caacd9dbbb128cb83e99ca0367c2b41b3 Mon Sep 17 00:00:00 2001 From: Leo Wandersleb Date: Thu, 10 Sep 2026 22:23:00 +0200 Subject: [PATCH 4/4] Swap inns between hungry units when both walks get shorter A hungry unit books the nearest inn with a free meal, and the booking takes the meal: the next unit, standing closer to that inn, finds it full and walks to the next one, crossing the first on the way. Two units heading for the wrong inns each is the food side of the crossing that swapTask fixes for fetching. Team::swapInn runs the moment a unit books its place. It compares the unit's walk with every team mate still walking to an inn of this team, reading only the building gradients that choosing an inn already built (or the crow-flight distance for a flyer), and when trading inns shortens the two walks together by more than four tiles it moves both bookings: guest lists, attached and target building. Each inn keeps its head count, so the meals stay reserved. A trade that would put an inn beyond what a unit can still walk is not made. Inn bookings are rare next to hiring, so one scan per booking is cheap. InnSwapHarness covers the crossing and a booking that is already the shorter one, and walks both units into their inns afterwards. Fable 5.1 helped authoring this commit. --- .github/workflows/build.yml | 5 ++ src/SConscript | 7 ++ src/team/Team.h | 6 ++ src/team/TeamRouting.cpp | 4 - src/team/TeamStep.cpp | 67 +++++++++++++++++ src/unit/UnitActivity.cpp | 1 + test/InnSwapHarness.cpp | 145 ++++++++++++++++++++++++++++++++++++ 7 files changed, 231 insertions(+), 4 deletions(-) create mode 100644 test/InnSwapHarness.cpp diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f951bdfcc..adcbfe9c3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -169,6 +169,11 @@ jobs: scons -j$(nproc) release=1 server=0 round-trip-hunger-gate-test python3 test/run-savegame-safety-tests.py --check-preferences build/src/RoundTripHungerGateHarness . + - name: Build and run the inn swap regression + run: | + scons -j$(nproc) release=1 server=0 inn-swap-test + timeout 120s ./build/src/InnSwapHarness + - name: Test savegame loading and atomic autosaves run: | scons -j$(nproc) release=1 server=0 savegame-safety-test buffered-file-test diff --git a/src/SConscript b/src/SConscript index 967056496..35abb8883 100644 --- a/src/SConscript +++ b/src/SConscript @@ -672,6 +672,13 @@ if not env['server']: regression_test = local.Program('BuildingExpelHarness', regression_sources) local.Alias('building-expel-test', regression_test) +# Real engine regression for inn swaps between hungry units, built only by its explicit target. +if not env['server']: + regression_sources = [source for source in source_files if source != 'Glob2.cpp'] + regression_sources += local.Object('InnSwapHarness.o', '#test/InnSwapHarness.cpp') + regression_test = local.Program('InnSwapHarness', regression_sources) + local.Alias('inn-swap-test', regression_test) + # Real engine regression for the resource hunger gate, built only by its explicit target. if not env['server']: regression_sources = [source for source in source_files if source != 'Glob2.cpp'] diff --git a/src/team/Team.h b/src/team/Team.h index ec5d3c3a6..3f1cee05f 100644 --- a/src/team/Team.h +++ b/src/team/Team.h @@ -27,6 +27,9 @@ class Unit; class Game; +//! Tiles a unit can still walk before it starves: what is left of its hunger, then its hp. +Sint32 starvationLimitedTravelDistance(const Unit *unit); + class Team:public BaseTeam { public: @@ -113,6 +116,9 @@ class Team:public BaseTeam //! Give `unit`'s fetching job to a team mate and take the mate's job, when that //! shortens the two trips together by more than a few tiles (see TeamStep.cpp). void swapTask(Unit *unit); + //! Give `unit` a team mate's inn and the mate `unit`'s, when that shortens the + //! two walks together by more than a few tiles; called as `unit` books its place. + void swapInn(Unit *unit); //! Highest build level any unit of the team has. int maxBuildLevel(void); diff --git a/src/team/TeamRouting.cpp b/src/team/TeamRouting.cpp index 9212e846f..c5c11eeb1 100644 --- a/src/team/TeamRouting.cpp +++ b/src/team/TeamRouting.cpp @@ -12,15 +12,11 @@ #include "Team.h" #include "Unit.h" -namespace { - Sint32 starvationLimitedTravelDistance(const Unit *unit) { return std::max(0, unit->hungry) / unit->race->hungriness + unit->hp; } -} // namespace - Building *Team::findNearestHeal(Unit *unit) { if (unit->hungry < 0) diff --git a/src/team/TeamStep.cpp b/src/team/TeamStep.cpp index 5b6da637f..2f5c14ecb 100644 --- a/src/team/TeamStep.cpp +++ b/src/team/TeamStep.cpp @@ -2,6 +2,7 @@ // Copyright (C) 2001-2004 Stephane Magnenat & Luc-Olivier de Charrière #include +#include #include "BuildingType.h" #include "Game.h" @@ -208,6 +209,35 @@ namespace return true; } + // A unit walking to the inn where it booked its meal. + bool isWalkingToInn(const Unit *u) + { + return u && u->activity == Unit::ACT_UPGRADING && u->destinationPurpose == FEED + && u->attachedBuilding && u->displacement == Unit::DIS_GOING_TO_BUILDING; + } + + // Tiles `u` walks to reach `b`: the building's gradient, which choosing an inn + // already built, or the crow-flight distance findNearestFood uses for a flyer. + bool innCost(Unit *u, Building *b, int *cost) + { + Map *map = b->owner->map; + if (u->performance[FLY]) + { + *cost = 1 + (Sint32)sqrt(map->warpDistSquare(u->posX, u->posY, b->posX, b->posY)); + return true; + } + return map->buildingAvailable(b, u->swimClass(), u->posX, u->posY, cost); + } + + // Move `u`'s booking from one inn to the other; both keep their head count. + void rebook(Unit *u, Building *from, Building *to) + { + from->unitsInside.remove(u); + to->unitsInside.push_back(u); + u->attachedBuilding = to; + u->setTargetBuilding(to); + } + void assignTask(Unit *u, Building *b, int resource) { u->attachedBuilding->removeUnitFromWorking(u); @@ -271,6 +301,43 @@ void Team::swapTask(Unit *unit) assignTask(best, a, r); } +void Team::swapInn(Unit *unit) +{ + if (!isWalkingToInn(unit)) + return; + Building *a = unit->attachedBuilding; + int own; + if (a->owner != this || !innCost(unit, a, &own)) + return; + Unit *best = NULL; + int bestGain = SWAP_MIN_GAIN; + for (int i = 0; i < Unit::MAX_COUNT; i++) + { + Unit *mate = myUnits[i]; + if (mate == unit || !isWalkingToInn(mate) || mate->attachedBuilding == a || mate->attachedBuilding->owner != this) + continue; + Building *b = mate->attachedBuilding; + int mateOwn, mine, theirs; + if (!innCost(mate, b, &mateOwn) || !innCost(unit, b, &mine) || !innCost(mate, a, &theirs)) + continue; + if (mine >= starvationLimitedTravelDistance(unit) || theirs >= starvationLimitedTravelDistance(mate)) + continue; + int gain = own + mateOwn - mine - theirs; + if (gain > bestGain) + { + bestGain = gain; + best = mate; + } + } + if (best == NULL) + return; + Building *b = best->attachedBuilding; + rebook(unit, a, b); + rebook(best, b, a); + a->updateCallLists(); + b->updateCallLists(); +} + void Team::syncStep(void) { integrity(); diff --git a/src/unit/UnitActivity.cpp b/src/unit/UnitActivity.cpp index deaaabde0..0a0cd9fc6 100644 --- a/src/unit/UnitActivity.cpp +++ b/src/unit/UnitActivity.cpp @@ -154,6 +154,7 @@ void Unit::handleActivity(void) if (verbose) printf("guid=(%d) Subscribed to food at building gbid=(%d)\n", gid, b->gid); b->subscribeUnitForInside(this); + owner->swapInn(this); } else activity=ACT_RANDOM; diff --git a/test/InnSwapHarness.cpp b/test/InnSwapHarness.cpp new file mode 100644 index 000000000..a7cb39a3a --- /dev/null +++ b/test/InnSwapHarness.cpp @@ -0,0 +1,145 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Hungry units swap inns when that shortens both walks: a unit that booked the +// last place in the inn near a team mate, from farther away, hands it over. +#include "GlobalContainer.h" +#include "Game.h" +#include "GameGUI.h" +#include "Unit.h" +#include "Building.h" +#include "BuildingType.h" +#include "IntBuildingType.h" +#include "Race.h" +#include "Ressource.h" +#include +#include + +GlobalContainer* globalContainer = nullptr; + +static void require(bool ok, const char* message) +{ + if (!ok) { std::fprintf(stderr, "FAIL: %s\n", message); std::exit(1); } +} + +struct World +{ + GameGUI gui; + Game& game = gui.game; + Team* team = nullptr; + + World() + { + game.map.setSize(6, 6, GRASS); // 64x64 + game.map.setGame(&game); + game.addTeam(0); + team = game.teams[0]; + // The gradient scheduler expects an in-use field; allocation is lazy (#243 lifts this). + game.map.getResourceGradient(0, WOOD, 0); + } + + // An inn with `meals` wheat: it takes bookings while it has more wheat than guests. + Building* addInn(int x, int y, int meals) + { + const int typeNum = globalContainer->buildingsTypes.getTypeNum("inn", 0, false); + require(typeNum >= 0, "inn type exists"); + Building* b = game.addBuilding(x, y, typeNum, 0); + require(b != nullptr, "inn placed"); + game.map.setBuilding(x, y, b->type->width, b->type->height, b->gid); + b->resources[CORN] = meals; + b->update(); + return b; + } + + // A worker that looks for food on its next activity check. + Unit* addHungryWorker(int x, int y) + { + Unit* u = game.addUnit(x, y, 0, WORKER, 0, 0, 0, 0); + require(u != nullptr, "worker placed"); + u->hungry = 0; + u->medical = Unit::MED_HUNGRY; + u->needToRecheckMedical = true; + return u; + } + + static bool booked(const Building* inn, const Unit* u) + { + for (const Unit* guest : inn->unitsInside) + if (guest == u) + return true; + return false; + } +}; + +// Inn A (west) has one meal, inn B (east) plenty. The far unit books A first +// and fills it; the near unit then has to book B. At that moment both walks +// together are longer than with the inns exchanged, so they trade. +static void theLaterBookerTradesWithTheOneItWouldCross() +{ + World world; + Building* a = world.addInn(4, 8, 1); + Building* b = world.addInn(44, 8, 10); + Unit* far = world.addHungryWorker(20, 9); + Unit* near = world.addHungryWorker(8, 9); + require(world.game.integrity(), "scenario setup is consistent"); + + // Units look for food at the end of their current action. + for (int i = 0; i < 100 && !(far->attachedBuilding && near->attachedBuilding); ++i) + world.game.syncStep(0); + + require(far->activity == Unit::ACT_UPGRADING && far->destinationPurpose == FEED, "the far unit goes to eat"); + require(near->activity == Unit::ACT_UPGRADING && near->destinationPurpose == FEED, "the near unit goes to eat"); + require(near->attachedBuilding == a && near->targetBuilding == a, "the near unit ends up with the near inn"); + require(far->attachedBuilding == b && far->targetBuilding == b, "the far unit ends up with the far inn"); + require(World::booked(a, near) && !World::booked(a, far), "inn A's guest list follows"); + require(World::booked(b, far) && !World::booked(b, near), "inn B's guest list follows"); + require(a->unitsInside.size() == 1 && b->unitsInside.size() == 1, "each inn keeps one booking"); + require(world.game.integrity(), "integrity after the swap"); + + // Both walk to their inn (a tile takes a couple of dozen ticks) and eat. + bool nearAte = false, farAte = false; + for (int i = 0; i < 3000 && !(nearAte && farAte); ++i) + { + world.game.syncStep(0); + nearAte |= near->displacement == Unit::DIS_INSIDE && near->attachedBuilding == a; + farAte |= far->displacement == Unit::DIS_INSIDE && far->attachedBuilding == b; + } + if (!(nearAte && farAte)) + std::fprintf(stderr, "near: act=%d dis=%d att=%p dead=%d hp=%d | far: act=%d dis=%d att=%p dead=%d hp=%d (a=%p b=%p)\n", + near->activity, near->displacement, (void*)near->attachedBuilding, near->isDead, near->hp, + far->activity, far->displacement, (void*)far->attachedBuilding, far->isDead, far->hp, (void*)a, (void*)b); + require(nearAte, "the near unit eats at inn A"); + require(farAte, "the far unit eats at inn B"); + std::puts("PASS a later booker trades inns with the team mate it would have crossed"); +} + +// With the inns already matched to the units, booking changes nothing. +static void aGoodBookingStays() +{ + World world; + Building* a = world.addInn(4, 8, 10); + Building* b = world.addInn(44, 8, 10); + Unit* west = world.addHungryWorker(8, 9); + Unit* east = world.addHungryWorker(40, 9); + + for (int i = 0; i < 100 && !(west->attachedBuilding && east->attachedBuilding); ++i) + world.game.syncStep(0); + + require(west->attachedBuilding == a && east->attachedBuilding == b, "each unit keeps its nearest inn"); + require(World::booked(a, west) && World::booked(b, east), "the guest lists match"); + require(world.game.integrity(), "integrity without a swap"); + std::puts("PASS a booking that is already the shorter one is kept"); +} + +int main() +{ + GlobalContainer globals; + globalContainer = &globals; + globals.runNoX = true; + globals.settings.rememberUnit = false; + globals.buildingsTypes.init(); + IntBuildingType::init(); + Race::loadDefault(); + theLaterBookerTradesWithTheOneItWouldCross(); + aGoodBookingStays(); + std::puts("Inn swap regressions passed"); + return 0; +}