Put the team's stocked markets on the fetch gradients - #257
Open
Giszmo wants to merge 3 commits into
Open
Conversation
`BuildingOrder::id` is assigned at runtime by `Echo::add_building_order` from `BuildingRegister::register_building`, and is the key the order is known by in `BuildingRegister::pending_buildings`. Neither `save()` nor `load()` ever touched it, and the constructor `load()` uses -- the private `BuildingOrder()` -- initialised nothing, so every pending building order restored from a savegame carried whatever happened to be in the heap. `Echo::update_building_orders` then used that as a map key, in `br.issue_order((*i)->id, ...)` and in the `AssignWorkers` management orders it queues. So the AI's building placement, and through it the whole simulation, depended on heap layout. Observable on master: eight Nicowars on Playground resumed from a tick-25000 save and run to 27000 settle on one of two outcomes, 502 or 510 deliveries, roughly coin-flipped across runs. `setarch -R` makes it stable; with ASLR already off, `MALLOC_PERTURB_=85` alone flips it, and so does enabling `GLOB2_CHECKSUM_SIDECAR`, which only moves the heap. Memcheck names it: uninitialised value read in `BuildingRegister::issue_order`, origin the `operator new` in `Echo::load`. Serialise it, and default the member to -1 so an order that is built and never registered is still readable. Saves written before version 96 do not carry the field; `Echo::load` hands those a fresh registration rather than a sentinel, since the value has to be a real register key. `BuildingRegister` is loaded earlier in the same function, so the registration is valid by then. A game played from the start was never affected -- the id is only garbage on the load path. Opus 5 helped authoring this commit.
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.
Giszmo
force-pushed
the
fix/market-round-trip
branch
2 times, most recently
from
September 11, 2026 13:28
1ecfb28 to
77a1499
Compare
Fetching a resource out of a market -- an inn pulling fruit a market of its own team holds -- lived only in the block a worker ran after a drop-off to pick its own next trip, comparing the walk to the nearest fruit tile with the walk to a stocked market plus five tiles. Per- delivery hiring removed that block, and neither the resource gradient units walk nor the round-trip field hiring scores with ever knew a market, so a freed worker could never be sent to one: market stock was unreachable. Make a stocked market a goal of the gradient itself. Every resource gradient now has a "with markets" twin in which the tiles of the team's alive markets holding the resource are seeded MARKET_DETOUR_TILES below a tile of the resource -- the same five-tile detour as before, for the resource having been carried there once already. A building that is not a market fetches by that twin (Building::fetchesFromMarkets); markets fetch for themselves by the plain one, so stock never circulates between markets. The round-trip field a fetcher builds takes the market tiles as goals with the same detour, so hiring, task swaps and the walk all price a market like a tile of the resource and the nearer one wins. On arrival next to a market the unit takes one unit of the resource at its door and carries it home; the old take-at-the-door branch keyed on ownExchangeBuilding is left as is, nothing sets it any more. A twin is rebuilt together with its plain gradient, from the same tiles in the same turn of the round robin, so the two never disagree about the map; on its own only when a market's stock of the resource appears or runs out (Building::addResourceIntoBuilding, removeResourceFromBuilding, kill), so a unit walking to a market that ran dry is redirected at the next step instead of arriving to nothing. Markets hold their stock in the team's shared pool, so one change dirties every market at once. Memory: one more Uint16 field per team, resource and swim class that a non-market building fetches, allocated lazily like the plain ones. The real-engine check in test/MarketFetchHarness.cpp runs one hiring pass on a level-2 inn: stocked market and no cherries on the map -> the worker is hired and the gradient's goal is the market; at the market's door -> the take happens on arrival and the unit carries the cherry to the inn; a cherry tile nearer than the market -> the tile is the goal; the market runs dry -> after the rebuild it is no goal and nobody is hired. Built with the market-fetch-test target and run in CI. The twin is runtime state like its plain gradient and is saved with the map runtime state (FILE_FORMAT_VERSION_MARKET_GRADIENTS, 98): a resumed game walks the same field it left, which the savegame-safety continuation check enforces. It sits in its own "markets" section: the text format keys tiles by section name and the plain gradient's tiles already use the numbered ones. Saves older than 98 load with no twin and build one on first use. Simulation behaviour changes, so VERSION_MINOR goes to 98 and replays older than that are refused. Fable 5.1 helped authoring this commit.
Giszmo
force-pushed
the
fix/market-round-trip
branch
from
September 11, 2026 13:40
77a1499 to
b86526d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #254 (first two commits are #253 and #254; the third is this one). Closes the "known gap" called out in #254, the way Leo asked for: markets become part of the gradient, and the gradient goes dirty whenever a market's stock switches availability.
Why
Fetching a resource out of a market (an inn pulling fruit a market of its own team holds) lived only in the self-renewal block a worker ran after a drop-off. #254 removes that block, and neither the resource gradient units walk nor the round-trip field hiring scores with ever knew a market, so with per-delivery hiring market stock became unreachable.
What
Map::getResourceGradient(..., withMarkets)): the tiles of the team's alive markets holding the resource are seededMARKET_DETOUR_TILES(5) below a tile of the resource, the same detour the old code charged. A building that is not a market fetches by the twin (Building::fetchesFromMarkets()); markets fetch for themselves by the plain one, so stock never circulates between markets.updateRoundTripGradient) takes market tiles as goals with the same detour, so hiring, task swaps (Swap fetching jobs between units when both trips get shorter #197) and the walk all price a market like a tile and the nearer one wins.Map::touchedStockedMarket) and carries it home. The old branch keyed onownExchangeBuildingstays; nothing sets it any more.Map::dirtyMarketGradientsmarks it, which happens when a market's stock of a resource appears or runs out (addResourceIntoBuilding,removeResourceFromBuilding,kill). Markets keep their stock in the team's shared pool (useTeamResources), so one change covers every market.FILE_FORMAT_VERSION_MARKET_GRADIENTS= 98), so a resumed game walks the same field it left; the savegame-safety continuation check enforces that (it caught the first push, where the twin was not saved). Saves older than 98 load with no twin and build one on first use.VERSION_MINOR→ 98, replay floor with it. Memory: one extraUint16field per (team, resource, swim class) actually fetched by a non-market building, lazily allocated.Verification
market-fetch-testharness (new, in CI), one hiring pass on a level-2 inn: stocked market and no cherries on the map → hired, gradient goal is the market, plain gradient reports no cherries; at the market's door → take on arrival, stock 10 → 9, carrying to the inn; cherry tile nearer than the market → the tile is the goal; market runs dry → after the rebuild it is no goal and nobody is hired (UnitCantAccessFruit).TestsRunner(187), savegame-safety continuation, resource-fetch-target.🤖 Generated with Claude Code