Read one worker level where hiring read harvest and the menu read build

[Giszmo]

Marked as a bugfix on Leo's call; #255 (any worker stocks completed buildings) will sit on top of this.

Bug

A worker carries two schooling levels, level[HARVEST] and level[BUILD], for one concept. Only the school raises either, and it raises both in the same visit (upgradeInParallel), so in play they are always equal. The code, however, reads them inconsistently:

question	reads
may this worker be hired for a building of tier N? (Building::canUnitWorkHere)	harvest
may the player upgrade a building to tier N? (Team::maxBuildLevel, upgrade button)	build
fetch ranking (bringResourcesLevel)	harvest
chop speed / fill speed	harvest / build

The editor exposes both as separate boxes, and the unit panel shows two rows with the same number. A map with the two set apart can offer an upgrade the colony's workers are then refused for.

Real-engine run on master, one hiring pass, worker four tiles from a level-2 inn site carrying wood: harvest=0 build=1 → refused (UnitTooLowLevel), harvest=1 build=0 → hired. The upgrade menu would say the opposite.

Fix

• Unit::workerLevel() is the one read; Unit::setWorkerLevel() sets both halves and their performance together.
• Hiring gate, Team::maxBuildLevel and the fetch ranking read through it.
• Load normalises a unit whose two levels differ to the higher one, so existing maps keep working and cannot desync on it.
• The editor keeps the Build box (it now drives both) and drops the Harvest box. The unit panel drops the Harvest row.
• Save format and race tables untouched: both abilities are still stored and still drive their own speeds.
• VERSION_MINOR → 96, replay floor with it.

Verification

• level-gate-test harness (new, run in CI): tier gate on completed inns and sites, plus build=1, harvest=0 hired by a level-2 site and build=0, harvest=1 refused.
• TestsRunner (187).

🤖 Generated with Claude Code

[genixpro]

Not a review of the change itself yet — one thing that will bite on rebase, flagging it early since you'll be rebasing anyway.

The format version this claims is now taken. master is at VERSION_MINOR 96 (#253, Save the Echo AI's building-order id, took it). This branch bumps 95 → 96, so after a rebase the number collides with a format that has already been published.

That matters beyond the constant: the gated reads in this PR would then be versionMinor>=96, and a save written by master at 96 never wrote those fields — so the reads consume the next values in the stream and the load desynchronises. That is exactly the failure you diagnosed on #232 when master moved out from under it, down to the mechanism.

So on rebase this wants 97, and both the bump and every gate that references it need to move together. #232 is currently sitting on 97 as well, so whichever of the two lands second wants 98.

Worth borrowing the pattern #232 ended up with: a named constant beside master's existing ones in src/FileFormatVersions.h rather than a bare >=96 at each read site, so the next time master moves there is one place to change and a grep that finds it.

Happy to review the substance — the one-worker-level unification — whenever you'd like, rebased or not. The design question in it (dropping the editor's Harvest box, normalising existing maps to the higher of the two levels) doesn't depend on the rebase.

[Giszmo]

Rebased onto master and renumbered — thanks for flagging it before it bit. Two adjustments to your note, then what I did.

It wants 98, not 97. #232 has landed (95c6a60), so master already holds 97 as FILE_FORMAT_VERSION_TOPOLOGY_GENERATION. This branch is the one landing second, so it takes 98.

The desync mechanism you describe doesn't apply to this PR. It adds no version-gated reads and no new fields to the save stream. The only serialization it touched was one ungated in-memory normalisation at the end of Unit::load — the two halves of the worker level evened out after they were read, not an extra read. So a save written by master at 96 or 97 would not have gone off the rails here; the collision was a wrong number on a replay-refusal marker, not a stream misalignment. The bump exists because simulation behaviour changes for maps that have the two levels apart, which is what REPLAY_MINIMUM_VERSION_MINOR is for.

Took the named-constant point anyway, and gave it something to hold. src/FileFormatVersions.h now has

//! Harvest and build became one worker level. Saves before this could hold the
//! two apart — the map editor offered a box for each — so the loader evens a
//! worker out to the higher of the two (UnitSerialization.cpp:90).
static constexpr int FILE_FORMAT_VERSION_ONE_WORKER_LEVEL = 98;

and the load-time evening-out is now gated on it rather than running unconditionally:

if (versionMinor < FILE_FORMAT_VERSION_ONE_WORKER_LEVEL
    && canLearn[BUILD] && level[HARVEST] != level[BUILD])
    setWorkerLevel(std::max(level[HARVEST], level[BUILD]));

That is safe because after this change nothing leaves the two apart in a save at 98 or later: every school tier has upgrade[BUILD] and upgrade[HARVEST] both set (BuildingTypesUpgrade.cpp), the editor's build box now calls setWorkerLevel, and the non-parallel upgrade path routes through it too. It also gives the constant a real consumer, so the next time master moves there is one place to change and a grep that finds it.

Also updated REPLAY_MINIMUM_VERSION_MINOR to 98 and merged its comment with master's list, moved the Version.h changelog entry to 98 alongside master's 96 and 97 entries, and dropped the stale (cherry picked from commit e5b1cf6a) trailer since the commit is no longer that content.

Verified locally on the rebased head before pushing: level-gate-test passes, savegame-safety-test and buffered-file-test pass, team-stats-save-test passes including the version88 and version84 legacy fixtures (stdout matches), and check_translations.py --strict / test_translations.py report 0 structural errors. Full CI is running on the pushed head.

The substance is unreviewed and the design question you raised — dropping the editor's Harvest box, normalising existing maps to the higher of the two — is untouched by the rebase, so it is still open whenever you want to look.