From c449ab4ccc9724dcb002de5bf5ee9a344eb96a14 Mon Sep 17 00:00:00 2001 From: Leo Wandersleb Date: Fri, 11 Sep 2026 12:48:56 +0200 Subject: [PATCH 1/2] Hire a fetcher for one delivery at a time A worker stayed subscribed to its building until that building's whole wish list was satisfied. After every drop-off it picked its own next resource from `attachedBuilding->computeWishedResources()`, so the trip belonged to whoever happened to deliver there last -- never to whoever was closest to it, and never to another building that wanted a worker more. Release it instead. `Team::updateAllBuildingTasks` runs later in the same tick, after every unit has stepped, and `considerUnitForBuilding` only accepts ACT_RANDOM units, so a unit freed during its own step is back on the market before that tick ends. The random step it starts on release is still in flight while the auction runs, so a unit that loses its own job back pays one tile for it. This is not a new mechanism, only a wider one: the release path already ran whenever a building wanted nothing the unit could reach, which was 35-40% of deposits in an eight-Nicowar Playground run. Known gap: fetching a resource out of a market -- an inn pulling fruit a market holds -- lives only in the self-renewal block this removes, and the building-side hiring scorer has no exchange option, so that path is now unreachable. It never fired in six 40000-tick eight-Nicowar runs (exchange_picked=0, exchange_taken=0), but it is a real gap and porting the exchange option into `Building::considerUnitForResource` is the fix. `Building::insertUnitToHarvesting` is left in place for that. Simulation behaviour changes, so VERSION_MINOR goes to 97 and replays older than that are refused. (cherry picked from commit b1ab3cb65b3c2540dfa78077b459dfc78aeed383) Fable 5.1 helped authoring this commit. --- src/ReplayReader.h | 8 +-- src/Version.h | 5 +- src/unit/UnitDisplacement.cpp | 126 ++++------------------------------ 3 files changed, 23 insertions(+), 116 deletions(-) diff --git a/src/ReplayReader.h b/src/ReplayReader.h index c0332d926..5f1cceb21 100644 --- a/src/ReplayReader.h +++ b/src/ReplayReader.h @@ -23,11 +23,11 @@ static constexpr Uint32 REPLAY_MIN_VALID_ORDERS = 5; //! Oldest replay format (the VERSION_MINOR the replay was written with) that //! the reader still accepts. Replays older than this are rejected: versions 90, 92, -//! 93, 94, 95 and 96 changed the simulation (weighted pathfinding and diagonal timing, +//! 93, 94, 95, 96 and 97 changed the simulation (weighted pathfinding and diagonal timing, //! hiring-bucket iteration, trapped-colony elimination, fetch-job apportionment, -//! round-trip routing and hiring, Echo building-order ids surviving a load), so -//! earlier replays would diverge from what happened. -static constexpr Uint16 REPLAY_MINIMUM_VERSION_MINOR = 96; +//! round-trip routing and hiring, Echo building-order ids surviving a load, +//! per-delivery hiring), so earlier replays would diverge from what happened. +static constexpr Uint16 REPLAY_MINIMUM_VERSION_MINOR = 97; /// This class is used for reading replays. /// The replay stream is kept open and read every time you do retrieveOrder. diff --git a/src/Version.h b/src/Version.h index 4f513f95d..29a1b016b 100644 --- a/src/Version.h +++ b/src/Version.h @@ -6,7 +6,7 @@ // This is the version of map and savegame format, and all of the recorded data on the server #define VERSION_MAJOR 0 #define MINIMUM_VERSION_MINOR 58 -#define VERSION_MINOR 96 +#define VERSION_MINOR 97 // version 91 saves the live RNG and routing state for deterministic continuation. // version 10 adds script saved in game // version 11 the gamesfiles do saves which building has been seen under fog of war. @@ -105,6 +105,9 @@ // version 96 saves AIEcho::Construction::BuildingOrder::id, which was assigned at // runtime and never serialised, so every pending building order restored // from a save carried an uninitialised heap value as its register key +// version 97 hires a fetcher for one delivery at a time: a unit that has just +// dropped off goes back to the free pool instead of re-hiring itself +// for its building's next trip, so every trip is auctioned //This must be updated when there are changes to YOG, MapHeader, GameHeader, BasePlayer, BaseTeam, //NetMessage, and the likes, in parallel to change of the VERSION_MINOR above diff --git a/src/unit/UnitDisplacement.cpp b/src/unit/UnitDisplacement.cpp index 1011c40f8..e322393ff 100644 --- a/src/unit/UnitDisplacement.cpp +++ b/src/unit/UnitDisplacement.cpp @@ -142,117 +142,21 @@ void Unit::handleDisplacement(void) } else { - ///Find a resource that the building wants and a location to get it from - ///The location may be a market, or the harvesting the resource from the - ///map. - int needs[MAX_NB_RESOURCES]; - attachedBuilding->computeWishedResources(needs); - int teamNumber=owner->teamNumber; - int timeLeft = numberOfStepsLeftUntilHungry(); - if (timeLeft > 0) - { - int bestResource=-1; - int minValue=owner->map->getW()+owner->map->getW(); - bool takeInExchangeBuilding=false; - Map* map=owner->map; - for (int r=0; r0) - { - int distToResource; - bool available=map->roundTripDistance(attachedBuilding, r, swimClass(), posX, posY, &distToResource); - if (available) - distToResource=(distToResource+1)/2; // half the round trip: the unit is at the building - else - available=map->resourceAvailable(teamNumber, r, swimClass(), posX, posY, &distToResource); - if (available) - { - if ((distToResource<<1)>=timeLeft) - continue; //We don't choose this resource, because it won't have time to reach the resource and bring it back. - int value=distToResource/need; - if (valuetype->canFeedUnit) - for (std::list::iterator bi=owner->canExchange.begin(); bi!=owner->canExchange.end(); ++bi) - if ((*bi)->resources[r]>0) - { - int buildingDist; - if (map->buildingAvailable(*bi, swimClass(), posX, posY, &buildingDist)) - { - // We increase the cost to get a resource in an exchange building to reflect the costs to get the resources to the exchange building. - // increase is +5 as markets will in general be very close to fruits as they are the fruit teleporters. - int value=(buildingDist+5)/need; - if (value=0) - { - destinationPurpose=bestResource; - assert(activity==ACT_FILLING); - if (takeInExchangeBuilding) - { - displacement=DIS_GOING_TO_BUILDING; - targetX=targetBuilding->getMidX(); - targetY=targetBuilding->getMidY(); - targetBuilding->insertUnitToHarvesting(this); - validTarget=true; - } - else - { - int dummyDist; - if (auto off = owner->map->doesUnitTouchResource(this, destinationPurpose)) - { - dx = off->dx; - dy = off->dy; - displacement=DIS_HARVESTING; - validTarget=false; - } - else if (map->resourceAvailableUpdate(teamNumber, destinationPurpose, swimClass(), posX, posY, &targetX, &targetY, &dummyDist)) - { - displacement=DIS_GOING_TO_RESOURCE; - validTarget=true; - } - else - { - assert(false);//You can remove this assert(), but *do* notice me! - stopAttachedForBuilding(false); - } - } - } - else - { - if (verbose) - printf("guid=(%d) can't find any wished resource, unsubscribing.\n", gid); - stopAttachedForBuilding(false); - } - } - else - { - if (verbose) - printf("guid=(%d) not enough time for anything, unsubscribing.\n", gid); - stopAttachedForBuilding(false); - } + // One delivery is one gig. Hand the unit back to the free pool + // instead of letting it re-hire itself for the next trip out of + // its own building's wish list: Team::updateAllBuildingTasks runs + // later in this same tick, after every unit has stepped, and only + // ACT_RANDOM units are candidates. So the next trip is auctioned + // among every worker and every building that wants one, instead of + // belonging to whoever happened to deliver here last. + // + // The unit standing at the door is usually the cheapest hire and + // wins its own job back. When it does not, the random step it + // starts below is still in flight while the auction runs, so + // losing costs it that one tile and nothing else. + if (verbose) + printf("guid=(%d) delivered; back on the market.\n", gid); + stopAttachedForBuilding(false); } } } From f9aacfa216110a5649f1867bf3d4ae8e89e603a9 Mon Sep 17 00:00:00 2001 From: Leo Wandersleb Date: Sat, 12 Sep 2026 02:13:03 +0200 Subject: [PATCH 2/2] Release a deliverer for the auction only while a fifth of the team is idle Per-delivery hiring on its own moved no work: on the petri2 and Nicowar beds deliveries were flat to within two percent, in the early minutes as much as later. The auction only changes anything when a building has to wait for a worker, and with nobody idle it can only hand the trip back to the unit that just delivered, one tile later. So gate the release on idle hands. After a deposit the worker goes back to the pool only when at least RELEASE_IDLE_WORKER_PERCENT (20) of the team's living workers are idle -- those are the ones that may by chance stand closer to the next job. Otherwise it keeps its building and picks its next trip itself, the way master does, including the market path for an inn's fruit. Team::idleWorkerShareAtLeast counts workers and idle workers over the unit table; it runs once per deposit. The real-engine check in test/GigReleaseHarness.cpp puts a worker at an inn's door with wheat on its back and deposits: with four of five workers idle it is released and off the inn's crew; with nobody idle it keeps the inn and is already heading for the next wheat; with exactly one in five idle it is released. Built with gig-release-test and run in CI. Fable 5.1 helped authoring this commit. --- .github/workflows/build.yml | 4 + src/SConscript | 6 ++ src/team/Team.h | 4 + src/team/TeamStep.cpp | 15 ++++ src/unit/UnitConsts.h | 3 + src/unit/UnitDisplacement.cpp | 138 ++++++++++++++++++++++++++++++---- test/GigReleaseHarness.cpp | 136 +++++++++++++++++++++++++++++++++ 7 files changed, 292 insertions(+), 14 deletions(-) create mode 100644 test/GigReleaseHarness.cpp diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index adcbfe9c3..9c5b36158 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -159,6 +159,10 @@ jobs: scons -j$(nproc) release=1 server=0 hiring-bucket-test python3 test/run-savegame-safety-tests.py --check-preferences build/src/HiringBucketHarness . + - name: Build and run the gig-release regression + run: | + scons -j$(nproc) release=1 server=0 gig-release-test + python3 test/run-savegame-safety-tests.py --check-preferences build/src/GigReleaseHarness . - name: Build and run the resource-fetch target regression run: | scons -j$(nproc) release=1 server=0 resource-fetch-target-test diff --git a/src/SConscript b/src/SConscript index 35abb8883..183c49f92 100644 --- a/src/SConscript +++ b/src/SConscript @@ -659,6 +659,12 @@ if not env['server'] and 'hiring-bucket-test' in COMMAND_LINE_TARGETS: hiring_sources += local.Object('HiringBucketHarness.o', '#test/HiringBucketHarness.cpp') local.Alias('hiring-bucket-test', local.Program('HiringBucketHarness', hiring_sources)) +# After a delivery the worker is released only while a fifth of the team is idle. +if not env['server'] and 'gig-release-test' in COMMAND_LINE_TARGETS: + gig_sources = [source for source in source_files if source != 'Glob2.cpp'] + gig_sources += local.Object('GigReleaseHarness.o', '#test/GigReleaseHarness.cpp') + local.Alias('gig-release-test', local.Program('GigReleaseHarness', gig_sources)) + if not env['server'] and 'custom-setup-test' in COMMAND_LINE_TARGETS: custom_sources = [source for source in source_files if source != 'Glob2.cpp'] custom_sources += local.Object('CustomGameSetupHarness.o', '#test/CustomGameSetupHarness.cpp') diff --git a/src/team/Team.h b/src/team/Team.h index 3f1cee05f..c5d07120b 100644 --- a/src/team/Team.h +++ b/src/team/Team.h @@ -116,6 +116,10 @@ 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); + /// Whether at least `percent` of the team's living workers are idle + /// (ACT_RANDOM). Read after a delivery to decide whether to release the + /// worker for the hiring auction. + bool idleWorkerShareAtLeast(int percent) const; //! 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); diff --git a/src/team/TeamStep.cpp b/src/team/TeamStep.cpp index 2f5c14ecb..843a51914 100644 --- a/src/team/TeamStep.cpp +++ b/src/team/TeamStep.cpp @@ -260,6 +260,21 @@ namespace } } +bool Team::idleWorkerShareAtLeast(int percent) const +{ + int workers=0, idle=0; + for (int i=0; itypeNum!=WORKER || u->isDead) + continue; + workers++; + if (u->activity==Unit::ACT_RANDOM) + idle++; + } + return workers>0 && idle*100>=percent*workers; +} + void Team::swapTask(Unit *unit) { if (!isFetching(unit)) diff --git a/src/unit/UnitConsts.h b/src/unit/UnitConsts.h index 444281c19..b5df703e3 100644 --- a/src/unit/UnitConsts.h +++ b/src/unit/UnitConsts.h @@ -54,6 +54,9 @@ const int NB_UNIT_LEVELS=4; //! when the counter is treated as "fully arrived" (Unit.cpp:296, 304; //! UnitMovement.cpp; MapQuery.cpp; TypeSteps.cpp turret bullet timing). static constexpr int UNIT_DELTA_MAX = 255; +/// Share of a team's workers that must be idle for a worker to be released +/// after a delivery instead of keeping its building (Unit::handleDisplacement). +static constexpr int RELEASE_IDLE_WORKER_PERCENT = 20; //! Modular quantum that wraps a unit's `delta` counter, equal to //! UNIT_DELTA_MAX + 1. Used in expressions like (256 - delta) / speed. static constexpr int UNIT_DELTA_QUANTUM = 256; diff --git a/src/unit/UnitDisplacement.cpp b/src/unit/UnitDisplacement.cpp index e322393ff..268182a74 100644 --- a/src/unit/UnitDisplacement.cpp +++ b/src/unit/UnitDisplacement.cpp @@ -140,24 +140,134 @@ void Unit::handleDisplacement(void) validTarget=false; assert(needToRecheckMedical); } - else + else if (owner->idleWorkerShareAtLeast(RELEASE_IDLE_WORKER_PERCENT)) { - // One delivery is one gig. Hand the unit back to the free pool - // instead of letting it re-hire itself for the next trip out of - // its own building's wish list: Team::updateAllBuildingTasks runs - // later in this same tick, after every unit has stepped, and only - // ACT_RANDOM units are candidates. So the next trip is auctioned - // among every worker and every building that wants one, instead of - // belonging to whoever happened to deliver here last. - // - // The unit standing at the door is usually the cheapest hire and - // wins its own job back. When it does not, the random step it - // starts below is still in flight while the auction runs, so - // losing costs it that one tile and nothing else. + // One delivery is one gig while there are idle hands: give the trip + // back to the pool and let Team::updateAllBuildingTasks, which runs + // later in this same tick after every unit has stepped, auction it + // among every free worker and every building that wants one. An + // idle worker may by chance stand closer to the next job than the + // one that just delivered. With nobody idle the auction could only + // hand the job back to this unit a tile later, so it keeps its + // building and picks its next trip itself, below. if (verbose) - printf("guid=(%d) delivered; back on the market.\n", gid); + printf("guid=(%d) delivered, released for the auction.\n", gid); stopAttachedForBuilding(false); } + else + { + ///Find a resource that the building wants and a location to get it from + ///The location may be a market, or the harvesting the resource from the + ///map. + int needs[MAX_NB_RESOURCES]; + attachedBuilding->computeWishedResources(needs); + int teamNumber=owner->teamNumber; + int timeLeft = numberOfStepsLeftUntilHungry(); + if (timeLeft > 0) + { + int bestResource=-1; + int minValue=owner->map->getW()+owner->map->getW(); + bool takeInExchangeBuilding=false; + Map* map=owner->map; + for (int r=0; r0) + { + int distToResource; + bool available=map->roundTripDistance(attachedBuilding, r, swimClass(), posX, posY, &distToResource); + if (available) + distToResource=(distToResource+1)/2; // half the round trip: the unit is at the building + else + available=map->resourceAvailable(teamNumber, r, swimClass(), posX, posY, &distToResource); + if (available) + { + if ((distToResource<<1)>=timeLeft) + continue; //We don't choose this resource, because it won't have time to reach the resource and bring it back. + int value=distToResource/need; + if (valuetype->canFeedUnit) + for (std::list::iterator bi=owner->canExchange.begin(); bi!=owner->canExchange.end(); ++bi) + if ((*bi)->resources[r]>0) + { + int buildingDist; + if (map->buildingAvailable(*bi, swimClass(), posX, posY, &buildingDist)) + { + // We increase the cost to get a resource in an exchange building to reflect the costs to get the resources to the exchange building. + // increase is +5 as markets will in general be very close to fruits as they are the fruit teleporters. + int value=(buildingDist+5)/need; + if (value=0) + { + destinationPurpose=bestResource; + assert(activity==ACT_FILLING); + if (takeInExchangeBuilding) + { + displacement=DIS_GOING_TO_BUILDING; + targetX=targetBuilding->getMidX(); + targetY=targetBuilding->getMidY(); + targetBuilding->insertUnitToHarvesting(this); + validTarget=true; + } + else + { + int dummyDist; + if (auto off = owner->map->doesUnitTouchResource(this, destinationPurpose)) + { + dx = off->dx; + dy = off->dy; + displacement=DIS_HARVESTING; + validTarget=false; + } + else if (map->resourceAvailableUpdate(teamNumber, destinationPurpose, swimClass(), posX, posY, &targetX, &targetY, &dummyDist)) + { + displacement=DIS_GOING_TO_RESOURCE; + validTarget=true; + } + else + { + assert(false);//You can remove this assert(), but *do* notice me! + stopAttachedForBuilding(false); + } + } + } + else + { + if (verbose) + printf("guid=(%d) can't find any wished resource, unsubscribing.\n", gid); + stopAttachedForBuilding(false); + } + } + else + { + if (verbose) + printf("guid=(%d) not enough time for anything, unsubscribing.\n", gid); + stopAttachedForBuilding(false); + } + } } } else diff --git a/test/GigReleaseHarness.cpp b/test/GigReleaseHarness.cpp new file mode 100644 index 000000000..6bb8d8846 --- /dev/null +++ b/test/GigReleaseHarness.cpp @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Real-engine regression: after a delivery a worker is released for the hiring +// auction only when at least RELEASE_IDLE_WORKER_PERCENT of the team's workers +// are idle; otherwise it keeps its building and picks its next trip itself. +#define SDL_MAIN_HANDLED +#ifdef main +#undef main +#endif +#include "GlobalContainer.h" +#include "FileManager.h" +#include +#include +#include "Game.h" +#include "GameGUI.h" +#include "Unit.h" +#include "UnitConsts.h" +#include "Team.h" +#include "MapInternal.h" +#include "Race.h" +#include "Ressource.h" +#include "IntBuildingType.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 TestUnit : Unit +{ + using Unit::Unit; + void deliver() { needToRecheckMedical = true; handleDisplacement(); } +}; + +// A worker standing at the door of an inn that wants wheat, wheat on its back, +// about to deposit; `others` more workers of the team, idle or busy. +struct Bed +{ + GameGUI gui; + Game& game; + Team* team; + Building* inn; + TestUnit* carrier; + Bed(int others, bool othersIdle) : game(gui.game) + { + game.map.setSize(5, 5, GRASS); + game.map.setGame(&game); + 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 = game.teams[0]; + team->race.loadDefault(); + int innType = globalContainer->buildingsTypes.getTypeNum("inn", 0, false); + require(innType >= 0, "inn type exists"); + inn = game.addBuilding(8, 8, innType, 0); + require(inn != nullptr, "inn placed"); + game.map.setBuilding(8, 8, inn->type->width, inn->type->height, inn->gid); + inn->maxUnitWorking = 2; + inn->resources[CORN] = 0; + inn->updateCallLists(); + require(game.map.incResource(14, 8, CORN, 0), "wheat within reach for the next trip"); + // The carrier: attached, at the door, wheat on its back, depositing. + carrier = new TestUnit(7, 8, Unit::GIDfrom(0, 0), WORKER, team, 0); + team->myUnits[0] = carrier; + game.map.setGroundUnit(7, 8, carrier->gid); + carrier->activity = Unit::ACT_FILLING; + carrier->medical = Unit::MED_FREE; + carrier->attachedBuilding = inn; + inn->unitsWorking.push_back(carrier); + carrier->setTargetBuilding(inn); + carrier->destinationPurpose = CORN; + carrier->carriedResource = CORN; + carrier->displacement = Unit::DIS_FILLING_BUILDING; + carrier->dx = 1; carrier->dy = 0; + carrier->validTarget = false; + for (int i = 0; i < others; ++i) + { + Unit* u = game.addUnit(2 + i, 2, 0, WORKER, 0, 0, 0, 0); + require(u != nullptr, "place another worker"); + u->medical = Unit::MED_FREE; + u->activity = othersIdle ? Unit::ACT_RANDOM : Unit::ACT_FILLING; + } + } +}; + +int main(int argc, char** argv) +{ + SDL_SetMainReady(); + require(argc == 3, "usage: harness PROFILE ROOT"); + require(std::string(argv[1]).find("glob2-save-test-") == 0, "disposable profile required"); + GlobalContainer globals(argv[1]); + globals.fileManager->addDir(argv[2]); + globalContainer = &globals; + globals.runNoX = true; + globals.settings.rememberUnit = false; + globals.buildingsTypes.init(); + IntBuildingType::init(); + Race::loadDefault(); + require(RELEASE_IDLE_WORKER_PERCENT == 20, "this harness assumes the one-in-five threshold"); + + { + // Four idle workers of five: released. + Bed bed(4, true); + require(bed.team->idleWorkerShareAtLeast(RELEASE_IDLE_WORKER_PERCENT), "four idle of five is above the threshold"); + bed.carrier->deliver(); + require(bed.inn->resources[CORN] == 1, "the wheat was deposited"); + require(bed.carrier->activity == Unit::ACT_RANDOM && bed.carrier->attachedBuilding == NULL, "released for the auction"); + require(bed.inn->unitsWorking.empty(), "no longer working for the inn"); + std::puts("gig release: with idle hands the deliverer goes back to the pool"); + } + { + // Four busy workers of five: keeps its building and heads out again. + Bed bed(4, false); + require(!bed.team->idleWorkerShareAtLeast(RELEASE_IDLE_WORKER_PERCENT), "nobody idle is below the threshold"); + bed.carrier->deliver(); + require(bed.inn->resources[CORN] == 1, "the wheat was deposited"); + require(bed.carrier->activity == Unit::ACT_FILLING && bed.carrier->attachedBuilding == bed.inn, "keeps its building"); + require(bed.carrier->displacement == Unit::DIS_GOING_TO_RESOURCE && bed.carrier->destinationPurpose == CORN, "already heading for the next wheat"); + std::puts("gig release: with nobody idle the deliverer keeps its job"); + } + { + // Exactly one idle of five: the threshold is inclusive. + Bed bed(4, false); + bed.team->myUnits[1]->activity = Unit::ACT_RANDOM; + require(bed.team->idleWorkerShareAtLeast(RELEASE_IDLE_WORKER_PERCENT), "one in five counts"); + bed.carrier->deliver(); + require(bed.carrier->activity == Unit::ACT_RANDOM, "released at exactly one in five"); + std::puts("gig release: one idle worker in five is enough"); + } + std::puts("PASS a delivery releases the worker only while a fifth of the team is idle"); + return 0; +}