Decode replay orders against the replay's format version

[kylelutze]

NetSendOrder::decodeData hardcoded VERSION_MINOR when handing the payload to Order::getOrder:

order = Order::getOrder(buffer.data(), size, VERSION_MINOR);

That is right for live network traffic — both peers run this build, so the current version is by definition the correct one. It is wrong for a replay. A replay is a stored format that may predate the running build, and ReplayReader accepts anything from REPLAY_MINIMUM_VERSION_MINOR (86) up to VERSION_MINOR (88). The same decodeData serves both paths.

Nothing misparses today

FILE_FORMAT_VERSION_ORDER_CREATE_FLAG_RADIUS (78) is the only version gate anywhere in the order layer — every other order's setData takes versionMinor and ignores it. 78 sits below the replay floor of 86, so every order in every accepted replay decodes identically at 86, 87 and 88.

The next order-format gate would not be so lucky. It would silently misread every replay written before it — wrong offset or wrong width, no error raised, and no checksum to catch it (see the note below).

Approach

decodeData is a NetMessage virtual with 62 overrides, so changing its signature is off the table. Instead NetSendOrder carries the version as a member defaulting to VERSION_MINOR. The network path, the writer, and the other 61 subclasses are untouched; only ReplayReader overrides it, from the version in the replay's own header — mirroring how the adjacent wideStepCounter flag is already derived from that same value.

Testing

ReplayStepCounterTest gains a case asserting the version actually reaches Order::getOrder. Because no order parses differently across the supported replay range, there is no behavioural delta to observe — the plumbing has to be checked directly, so the test's stubbed getOrder records the version it was called with.

• ReplayStepCounterTest: 22/22 PASS.
• The new case was verified to fail with the one-line fix reverted (FAILURES: 1), then pass again once restored — it is not vacuous.
• Replay verification: ./build/src/glob2 --nox games/G2.game 90000 1 is byte-equal to tests/baselines/cpp-refactor.replay. No determinism impact; no baseline change.

Playback could not be exercised end-to-end: -replay is unreachable headlessly, since Glob2::run returns from runNoX() before the replay branch and runNoX() only supports initCustom.

Unrelated pre-existing issue noticed

ReplayReader::setCheckSum is never called by the engine — only the ReplayWriter counterpart is, at EngineRun.cpp:121. So ReplayReader::checksum stays 0 and the "checksums don't match!" guard in retrieveOrder is dead code: a replay that decoded wrong would play on silently. Not touched here.

[Giszmo]

Reviewed on head 943727e. Approve, no blocking findings.

What it does. NetSendOrder gets a decodeVersionMinor member defaulting to VERSION_MINOR; only ReplayReader overrides it from the replay header, at both decode sites (initial scan and retrieveOrder). Network path, ReplayWriter and the other NetMessage subclasses are untouched. Minimal and the right shape given decodeData is a virtual with a fixed signature.

Claims verified:

• The only version gate in the order layer is FILE_FORMAT_VERSION_ORDER_CREATE_FLAG_RADIUS (78) in OrderBuilding.cpp:49-60; grepped every versionMinor comparison across Order*.cpp. 78 is below the replay floor of 86, so this is behaviour-neutral today and only matters for the next format bump.
• Built test/ with scons: ReplayStepCounterTest ALL PASS, NetSendOrderDecodeTest 8/8.
• Not vacuous: reverting the one-line fix in OrderMessages.cpp and rebuilding gives FAILURES: 2 (both new decodeVersion checks).
• CI runs every ./*Test binary in test/ ("Build and run the tests" step), so the regression test is live.
• The ReplayReader::setCheckSum side note is correct: no caller anywhere in src, only the writer side is fed from EngineRun.cpp:121, so the checksum-mismatch guard in retrieveOrder is dead code. Worth a follow-up issue, not this PR.

Nits, non-blocking:

• ReplayReader now holds both versionMinor and wideStepCounter derived from the same header field. Deriving the flag from the member would remove one piece of duplicated state; mirroring the existing pattern is fine too.
• Member init in the ctor body rather than the initializer list matches the surrounding code.

Review by Bob (Fable 5.1 agent) at Leo's request.