Parallel battlescape turns + per-action desync detection, reporting, and crash bundling - #166
Draft
NonPolynomialTim wants to merge 135 commits into
Draft
Parallel battlescape turns + per-action desync detection, reporting, and crash bundling#166NonPolynomialTim wants to merge 135 commits into
NonPolynomialTim wants to merge 135 commits into
Conversation
Test-only introspection surface the parallel-turns PRDs (P2/P6/P7/P8) will
assert against. No gameplay change.
src/CoopMod/connectionTCP.{h,cpp}
- rxHoldSize() plus g_rxRotateCount / g_rxHoldMaxSeen: depth, rotate count
and high-water mark of updateCoopTask()'s hold queue, so a test can tell a
packet parked behind the receive gate from a dropped one. Counters are
process-monotonic, matching g_txDropCount (never reset).
src/CoopMod/TestServer.cpp
- battle_state gains taskCompleted, pathLock, coopWalkInit, coopInitDeath,
coopEnd, rxHold, rxRotates, rxHoldMax and parallelActive (false until P5).
- new parallel_state command (executeBattle12): the same gate fields without
battle_state's unit dump. P6/P7/P8 append their fields here.
- set_option accepts battleXcomSpeed (also disarming battleXcomSpeedOrig,
the ctrl-s quick-mode swap that would otherwise restore the old speed),
battleAlienSpeed, EnableCoopParallelTurns, coopParallelDebugClientInput.
All four echo the applied value back as "value" (no get_option exists).
src/Engine/Options.{inc.h,cpp}
- EnableCoopParallelTurns / coopParallelDebugClientInput declared inert (no
OptionInfo) so set_option has something to set before P5 consumes them.
PRD-P0 asked for "accept-and-set" but the symbols did not exist; P5 owns
the OptionInfo registration (UI + options.cfg) and should drop the
default-init added here.
tools/coop_test/harness.py
- make_user_dir(options=...) splices per-instance keys into the options.cfg
options block (host slow / client fast from the first frame).
tools/coop_test/session.py + callers
- can_drive(state) = activeSync or parallelActive. Driver-selection sites
migrated: repro74_probe, repro74_setup, test_coop_alien_launcher_item_loss,
test_coop_blast_item_damage, test_coop_inventory_item_theft,
test_coop_pvp_blaster, test_skirmish_battle_turn_control (walk driver only).
Left alone deliberately: diagnostic prints, and that test's step-2
assertion that exactly one machine has activeSync - that one is about the
executor invariant, not about who may drive.
- No behaviour change today: parallelActive is false.
tools/coop_test/test_parallel_introspection.py (new) + tools/ci/test_weights.json
- Skirmish co-op battle; asserts the options splice boots, both commands
carry the gate fields on both machines, parallelActive is false,
can_drive() == activeSync, the hold-queue high-water hook is wired, and
set_option round-trips the four new names while still rejecting unknowns.
- Weight 18.8s (measured).
Adjacent, NOT fixed (noted only):
- updateCoopTask()'s exception path also re-queues the packet to the back of
the hold deque; that is a teardown path (onConnect = -3) so it is not
counted as a gate rotation.
- The two rx counters never reset across co-op sessions. If P6/P7 want
per-battle numbers they need a reset in clearNetworkSessionQueues().
Full suite green: 110 tests + boot_check, 0 failures (test_shared_checksum and
test_shared_arrival_owner_labels each passed on the runner's retry - known
flakes, unrelated).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…PRD-P1)
Replayed peer actions used to run through the LOCAL player's state: every
handler called _save->setSelectedUnit(remoteUnit) and then wrote the singleton
BattlescapeGame::_currentAction (actor/type/target/targeting/weapon/waypoints)
before pushing the BState. On the watching machine that stole the selection and
the stat panel, and after watching a teammate shoot the watcher's own next map
click fired instead of walking (primaryAction dispatches on _currentAction).
Each replay handler now builds a stack-local BattleAction via the new
BattlescapeGame::makeReplayAction(actor) and hands it to the BState (states copy
the action, BattleState.h:33), so nothing of the peer's action touches the local
player's singleton:
movePlayerTarget / turnPlayerTarget / psi_attack / melee_attack
(BattlescapeGame.cpp) - setSelectedUnit deleted, all _currentAction
writes ported onto the local
coopActionClick / shootPlayerTarget (BattlescapeState.cpp) - same;
shootPlayerTarget's waypoint list is now local too, so a teammate's shot
no longer wipes the watcher's in-progress blaster waypoints
handleNonTargetAction gained a (BattleAction&) overload and CoopShoot now
takes the action, so those two tails run on the replay's own action; the
no-argument handleNonTargetAction() delegates with _currentAction, leaving
every classic call site unchanged
Preserved on purpose: the PlayerTurnYour setSelectedUnit (a HANDOFF, not a
replay - the newly-active player must start its turn with a unit selected),
setSelectedCoopUnit as-is (PRD-P5 stops SENDING selected_unit), and the #74
invariant - coopResolveWeapon returning null still SKIPS the action, now with a
log line, and never fabricates a BattleItem.
Also:
- turnPlayerTarget's "already facing there => this is a door-open" test used to
compare against _currentAction.target, which only worked because the replay
wrote the singleton. It now keeps its own BattlescapeGame::_replayTurnTarget.
- UnitWalkBState's mid-walk updateSoldierInfo() is gated on the walking unit
being the locally selected one (a no-op for local and AI walks, where the
acting unit IS the side-selected unit).
- UnitWalkBState's PVP sneak-silence test reads _action.sneak instead of the
singleton, which the replay no longer sets.
- movePlayerTarget calls Pathfinding::removePreview() before its calculate(),
which would otherwise strand the local player's hover-preview tiles lit.
- New Options::coopFollowPeerActions (bool, default TRUE = pre-P1 behaviour,
registered in createAdvancedOptionsOTHER as "Follow Teammate Actions with
Camera"/STR_BATTLESCAPE) gates movePlayerTarget's centerOnPosition. The AI
phase (_isActiveAISync) keeps centring unconditionally.
Test: tools/coop_test/test_parallel_replay_decouple.py drives a 2-instance
skirmish battle and asserts, on the machine that is NOT driving, that a
replayed walk / turn / shot / psi / melee leaves selectedId, the camera map
offset and _currentAction (type, actor, weapon, targeting, target, waypoints)
untouched, with a control sample proving the watcher is at rest first and a
per-step arrival check so a pass is never vacuous. The classic PlayerTurnYour
handoff is asserted to still select an own unit. battle_state gained read-only
camera/_currentAction fields, battle_fire gained hit/psi/panic modes and
battle_action gained end_turn_button (the real END TURN button; the existing
end_turn is the vanilla one and never ships PlayerTurnYour).
Adjacent, NOT fixed here (out of PRD-P1 scope):
- BattlescapeGame::melee_attack computes found_unit and never checks it, then
dereferences a null unit; every sibling handler has the guard.
- The melee replay double-charges TU on the receiver (melee_attack applies the
packet's TU, then MeleeAttackBState::init spends again because spendTU() is
the left operand of `!spendTU(...) && !coop_action`). Measured 49 vs 29.
- Two camera sites still follow a peer: UnitWalkBState's setViewLevel level
follow and Map::draw()'s projectile follow. Both are stock OpenXcom, fire for
AI actions too, and are not in the PRD's verified hijack table; PRD-P5's
"no camera follow during the parallel player side" owns them.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Makes the battlescape invariant OBSERVABLE: after every replicated action host
and peer must hold identical (id -> BattleItem) maps AND equal
SavedBattleGame::_itemId counters. True at generation, first thing to drift once
the two machines simulate independently. Every later P3/P4 fix is validated by
this going quiet.
Two terms (SharedEcon::battleChecksumTerms, null-guarded on a live battle):
chkBattleItemId SavedBattleGame::_itemId - the next id this machine will mint.
Every `new BattleItem` advances it, transients included.
chkBattleCensus SUM over getItems() of FNV-1a(type) mixed with the item id and
its owner unit id. A sum, not a rolling hash: the _items order
is not replicated, so the term must not depend on it. FNV
rather than std::hash because host and peer need not be the
same build (Windows exe vs Linux AppImage).
Negative = "not stamped" = agree, mirroring the world checksum's GAP-4 fields, so
an older peer can never produce a false positive.
3a: attachWorldChecksum stamps both when a battle is live -> the existing
shared_checksum TestServer hook exposes them for free. verifyWorldChecksum
routes them to the battle comparison on their OWN path; they are deliberately
NOT folded into the world mismatch condition, whose repair is
sharedResyncStream.
3b: the host stamps both on the per-turn `next_turn` packet (NextTurnState::close,
the only packet guaranteed to cross once a turn); the client compares. On a
mismatch: one LOG_ERROR per episode with id + census + turn + side + item
count + this machine's live action/selection ("last action context" until P6
numbers intents), the in-battle warning banner at most once per
RESYNC_DEBOUNCE_MS, and battleDesyncSeen() latched for the harness. NEVER a
mid-battle resync - the world restream replaces the whole state stack, live
battle included - and never a modal over the battlescape.
battleDesyncSeen() is a session latch cleared only by resetResyncStats():
co-op re-arms _battleInit every turn, so that is NOT a battle-start hook.
psi temp-item counter leak (BattlescapeGame::psiButtonAction): both co-op
fallbacks minted a BattleItem that is never added to _items but still advanced the
REPLICATED counter, so every psi button press drifted chkBattleItemId by one on
the pressing machine alone. Fixed with the smaller of the two PRD options - a
local throwaway counter (`int transientItemId = -1`) instead of
getCurrentItemId(). No ctor overload, no engine API change, and the transient's id
stays -1 so it can never collide with a real item on either side. Resolving an
existing special weapon was not available: these fallbacks run precisely because
getSpecialWeapon() found none.
Harness: battle_state gains itemIdCounter / battleCensus / desyncSeen;
session.battle_checksum() + session.assert_battle_synced(host, client);
SavedBattleGame::getCurrentItemIdValue() is the const read behind them.
New tools/coop_test/test_battle_tripwire.py (+ test_weights entry): terms agree at
generation on battle_state and shared_checksum, scripted walks keep them agreeing,
a full turn cycle leaves the tripwire silent when they agree (and must have FIRED
if the battle drifted on its own), then an uneven battle_give - a test-only use of
an existing harness command, no divergence mechanism ships - is detected within
one turn with no resync requested and the battle still live and playable.
Adjacent, NOT fixed (found while writing the test):
- Options::skipNextTurnScreen + co-op client = double NextTurnState::close():
think() runs the host's `click_close` (_onClickClose -> close()) and then the
auto-close timer, so one state can popState() twice and take the battlescape
with it. The test therefore enables the option on the host only.
- TestServer dismiss_popup generic-pops NextTurnState, skipping close() - which is
where the host builds and sends `next_turn`. Any future turn-cycling test must
let the host's screen self-close or it ships no per-turn packet.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…olls (PRD-P3 1/3)
Three AUDIT-rng gaps that all end in the same place: the two machines' id spaces
drift apart and never come back.
GAP-1 mid-battle spawns (BattlescapeGame::spawnNewUnit / spawnNewItem, reached
from ExplosionBState::explode and spawnFromPrimedItems). Both machines rolled
RNG::percent(spawnChance) and, when they agreed, both CREATED the unit - plus
every built-in weapon initUnit mints for it - out of their own id counter at
their own moment. Now the host decides and ships `spawn_units`; a co-op peer
refuses to spawn at all except while replaying that manifest.
Seed replay is the mechanism (handoff B2: the host captures RNG::getSeed()
without consuming it, the peer RNG::setSeed()s it and runs the SAME function),
but seed replay alone is not sufficient, because the peer applies the manifest at
a different moment and anything re-derived from LOCAL state can differ. So the
manifest also carries every such input explicitly:
carrier_rule the blast normally destroys the carrier item before the peer
gets here, so the rule is named instead of an instance being
looked up - and nothing on this path constructs a BattleItem
(issue #74). spawnNewUnit/Item now take their RuleItem from the
manifest when replaying, so a null damage_item is fine.
faction derives from the carrier's owner, whose local lookup can fail
owner_id -> setPreviousOwner, resolved to a live unit or null
attacker_id -> the facing that isPositionValidForUnit checks
direction an RNG::generate(0,7) the peer must not make
itemLevel an RNG::generate(0,9) the peer must not make
unit_id +
item_ids what the host minted; the peer compares and LOG_ERRORs on drift
(the P2 tripwire catches it too, this just names the culprit)
GAP-4a hit pairing. TileEngine::hit is host-authoritative, so a peer parks the
BattleActionAttack it would have resolved and waits for the host's `hit_tile`,
which carried no attack identity at all - the pairing was FIFO position. The
first time the two machines disagreed about how many hits an action produced (a
CQB block, a pellet that terminated early) every later hit in the battle was
applied to the wrong attacker. `hit_tile` now carries an `attack_id` and each
parked entry carries the key it expects; an unmatched hit is logged at ERROR and
dropped rather than mis-attributed, and an absent field still means FIFO (older
peer).
That key is DERIVED from the attack (FNV-1a over exactly the fields hitCoop
consumes: type, attacker id, weapon id, damage-item id), not a host counter. A
counter was the first implementation and the new test proved it unusable: the
host sends hit_tile from inside its own TileEngine::hit, which runs before the
peer has even started the replay chain that parks the matching attack, so the
peer is permanently ONE BEHIND (measured: attack_id 0 arrived with 0 parked,
attack_id 1 with 1 parked, and so on for every hit of the battle) and two
separately-kept counters can never line up. A derived key does not care how far
apart the streams sit. Two hits of the same attack hash the same, which is
correct - their parked entries are interchangeable.
GAP-5 shotgun pellets. Projectile::calculateTrajectory pre-rolled conf->shots aim
points into _coopProjectiles*, but a shotgun fires one queued shot and then
traces getShotgunPellets()-1 MORE trajectories per shot, each of which pops the
same queue. The peer therefore ran dry mid-volley and rolled the rest off its own
stream: different endpoints, a different number of TileEngine::hit() calls, and
from there the GAP-4a mis-pairing above. The pre-roll now emits the pellets too,
interleaved in the exact order both machines consume them (shot, its pellets,
next shot, its pellets, ...), each rolled from a SAVED copy of the original aim
so the shot chain's deliberate walk of _targetVoxel is untouched - a non-shotgun
weapon rolls byte-identically to before. Behaviour note: behaviorType 1 pellets
are pre-rolled off the original aim rather than the first pellet's impact, which
is not known until the projectile has flown; the deviation magnitude differs
slightly, the replay makes both machines use the same value either way.
test: tools/coop_test/test_coop_outcome_gaps.py part 1 (+ test_weights). Neither
shotgun ammo nor a spawn-on-blast item exists in stock xcom1, so it generates a
throwaway ruleset into the harness user dirs and activates it on both machines
(the test_shared_missile_bombardment pattern): STR_RIFLE_CLIP gains 4 pellets,
STR_SMALL_ROCKET gains spawnUnit. A volley and then a rocket into empty floor -
deterministic, and it kills nobody, so no corpse is minted (corpse ids are P4's
gap). Asserts both machines minted the SAME new unit id at the same position,
assert_battle_synced holds, the tripwire stays quiet, and the peer's log contains
no unpaired-hit error.
Adjacent, NOT fixed:
- the hit_tile handler `new RuleDamageType` is never freed (leaked per hit,
pre-existing; the drop path does not make it worse).
- an autoshot shotgun's pellets consumed the NEXT shot's queued aim point on both
machines before this commit; they now consume their own.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ruct (PRD-P3 2/3)
Outcome AUTHORITY, the second class of AUDIT-rng gap: a decision that changes
battle state was taken independently on both machines, so the two could simply
disagree about what happened.
GAP-2 psi in PVE. `TileEngine::psiAttack` ran in full on both machines and
`psi_result` was only ever sent in PVP, so a mind control that succeeded on one
machine and failed on the other was PERMANENT - the per-turn `next_turn` bulk
repairs stats and tiles, never faction. psiAttack now early-returns false on a
co-op client in the non-PVP modes (mirroring hitUnit), and the host ships
`psi_result` with `pvp: false` on BOTH outcomes, carrying the victim's POST-state
(faction, morale, TU, mindControllerId, coop owner) rather than the inputs -
reduceByBravery, convertToFaction and recoverTimeUnits all read state only the
host has changed, so there is nothing for the peer to re-derive.
PVP/PVP2 are deliberately untouched, and are why the discriminator exists: there
`psi_result` is an INVERTED flip ("mine now" / "yours no more"), not a state copy,
and the executor may be either machine. Absent `pvp` = older peer = the legacy
flip, so nothing regresses.
GAP-4b melee. Both machines rolled meleeAttackCalculate for the same attack, so
one could land a hit the other missed - and then the two hit streams stopped
lining up, which is exactly what commit 1's attack_id keying now detects. The
SENDER decides, in MeleeAttackBState::init, and the boolean rides the existing
`melee_attack` packet; TileEngine::meleeAttack pops the parked answer.
Why the sender and why there: the roll normally happens in the ExplosionBState
that init() pushes, i.e. AFTER the packet has gone out, and a `melee_result`
follow-up would race the receiver's own ExplosionBState (which starts one frame
later, with no _coop_task_completed coverage on the melee path). Deciding it
before the send is the only placement with no race, and the sender is the only
machine running at that point - the same authority every other pre-roll on an
action already uses. Under the PRD-P5 invariant executor == host anyway.
BA_CQB deliberately does NOT touch that queue.
GAP-4b CQB. The close-quarters check is three more independent draws (sneak-up
percent, to-hit, redirect direction) that both machines made. The peer now skips
the block entirely - the redirected aim already rides the shot packet, because the
block rewrites _action.target before the send - and applies `cqb_blocked` /
`cqb_defender` so the defender pays the same TU and energy.
GAP-4b self-destruct. ExplosionBState::init rolled RNG::percent(specialChance)
for a BA_SELF_DESTRUCT on both machines, and it did so after the `selfDestruct`
packet had already been sent, so no follow-up could carry it. The roll moves to
BattleUnit::damage (host-only already), parks locally and ships as `triggered`.
GAP-3 Tile::ignite. setFire/setSmoke have carried the host guard and their
packets for a long time; ignite() wrote _fire/_smoke DIRECTLY, so a burning-floor
unit (UnitWalkBState/UnitFallBState) or a melee attack lit fires on the peer off
its own RNG::percent() roll and told nobody. Same guard, and the writes now route
through the two setters so set_fire_tile/set_smoke_tile carry them. _overlaps is
assigned before setSmoke because the smoke packet ships it.
AUDIT-guards top break: SavedBattleGame::convertUnit's mirrored PVE2 blocks read
_isActivePlayerSync and assigned OPPOSITE values, so they agreed only while
exactly one machine held the flag. Both are replaced by one seat-deterministic
rule - the respawn inherits the DYING unit's getCoop() - which both machines
compute identically with no reference to whose turn it is.
Sanctioned P1 findings, same functions:
- BattlescapeGame::melee_attack computed found_unit and never checked it, then
dereferenced null. Added the guard every sibling replay handler has.
- The melee replay double-charged TU: spendTU() was the LEFT operand of
`!spendTU(...) && !coop_action`, so a receiver spent the cost again on top of
the authoritative value the packet had just written (P1 measured 49 vs 29).
The guard now short-circuits.
GAP-1 follow-up: the spawn manifest gained `final_pos`. applyGravity reads the
floor the blast just destroyed, and tile destruction is itself host-authoritative
on its own packet, so the peer could drop a spawned unit a level differently
(caught by the new test: host z=0, client z=1). The landing tile is state, not a
decision - the host's wins.
test: test_coop_outcome_gaps.py part 2 - a replayed melee must leave the attacker
with IDENTICAL TU on both machines (the deterministic red/green for the
double-charge) with every hit still paired, and repeated mind controls must leave
the victim's faction AND owner identical on both machines.
Residual, documented not fixed:
- an AI multi-hit melee (_hitNumber > 0) parks only ONE decision, so its 2nd..Nth
hits still roll locally on each machine.
- psi/mana experience counters still diverge (AUDIT "minor", post-battle stats).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…PRD-P3 3/3) The last AUDIT-rng class: whether an item still EXISTS was decided per machine. GAP-8 proximity sweeps. `checkForProximityGrenadesCoop` (the peer's replay of the host's sweep) removed every non-grenade item whose own `fuseProximityEvent()` fired - and that call ends in `RNG::percent(getSpecialChance())` drawn from the peer's own stream, so the two machines swept different sets off the same 3x3. The peer stops removing anything there; the host collects what it removed and ships it as `removed_items` on the sweep packet. Presence of that field is the discriminator in the handler: a packet WITH it is a removal list, a packet WITHOUT it (the three existing trigger sends) still asks the peer to run its own scan for the explosion states and the glow bookkeeping. GAP-9 end-of-turn fuses. `BattleItem::fuseTimeEvent()` has the same roll, and a failed one re-arms (BFT_SET) or disables the fuse instead of detonating - so a client could dud a grenade the host exploded and then hold, forever, an item the host no longer has. `next_turn` repairs stats and tiles, never item existence. The client now makes NO fuse rolls; the host ships `exploded` / `removed` / `fuses` (id + corrected timer). `fuse_events` is its own message rather than a field on `next_turn` as first sketched: next_turn only crosses at the START of the player's turn, so a fuse decided when the PLAYER side ended would not reach the peer until a whole alien side had run. The blast CONSEQUENCES still arrive on the usual explosion packets (hit_unit / explode_items / set_fire_tile / destroy_tile); what never crossed was the disappearance of the item itself. GAP-7 `_smokeRNGs` deleted. The relay was unbalanced - pushed once per TileEngine::hit(), popped only down hitTile's smoke branch, and popped AGAIN by explode()'s hitTile with no matching push, so after any explosion the queue was feeding stale values to the wrong tiles - and vestigial: the value only ever reached setSmoke(), which is host-gated and ships set_smoke_tile. That packet is the peer's single source of smoke. Gone with it: `hit_tile`'s "smokeRNG" field and the dead `obj["damageType"]` read (the sender never wrote that field and the value was never used). GAP-10 script RNG. `randomChance` / `randomRange` draw from the global stream on whichever machine runs the script and nothing captures or ships the result, so modded battle behaviour built on them diverges silently. A host-decides channel for arbitrary script calls is out of scope; V1 is a single WARNING the first time either is called in a co-op battle, naming it as unsupported, so a modder sees it instead of chasing a phantom desync. test: test_coop_outcome_gaps.py part 3. The ruleset gives STR_ELECTRO_FLARE specialChance 50 and a proximity fuse: a NON-grenade item is REMOVED rather than detonated, so the fuse paths can fire repeatedly without a blast killing anyone (a kill mints a corpse, whose id is P4's gap), and 50 is what makes the outcome an actual coin flip - at the default 100 both machines "roll" RNG::percent(100) and trivially agree, which would make the whole assertion vacuous. Six primed flares, then a `battle_prox` sweep and a turn boundary; the set removed and the survivors' fuse timers must be identical on both machines. The existing test_coop_proximity_item_sweep.py covers the other direction (the peer must not remove what the host kept) and now also exercises the new packet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Inert scaffolding for the id-manifest. Nothing sends a manifest yet and no
behaviour changes: with no record and no guard ever opened, the factory hook
returns on its first line.
* BattleItem::setIdCoop(int) - the ONLY way an item's id is re-stamped after
it is minted. Restricted by comment to the manifest apply path.
* SharedEcon: the manifest store (std::map<{action,subject}, std::deque<int>>,
transient, never persisted), CoopSpawnRecord (host RAII recorder; opening
clears its key so a repeatable action cannot ship a previous run's tail),
CoopSubjectGuard (peer RAII consumer, PER CALL and stacked - a global
"active subject" would be clobbered by two deaths in flight, hole H2),
flushSpawnRecord / storeSpawnManifest / remapCorpseIds / noteMintedItem /
clearSpawnManifests.
* storeSpawnManifest re-slaves the local counter to max(_itemId, maxHostId+1),
the same rule SavedBattleGame's loader applies to a save's item ids, so an
adopted id can never be minted a second time.
* The four SavedBattleGame item factories call the hook. createItemForUnit
only does so once the unit has actually KEPT the item.
Deviation from the PRD sketch: the two guards take no `coopMod` argument.
connectionTCP::getCoopStatic()/::getHost() are static, so a pointer would carry
no information, and SavedBattleGame::convertUnit (a P4 site) has none to hand.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The id-manifest pilot. A death's corpses are a Tier-A spawn: the SET is
deterministic (the armor's corpse list, size^2 of them) so both machines create
the same items, but each mints its own ids - and once those disagree, id N
denotes a different instance on the two machines and every later id-keyed packet
(moveCoopInventory, coopResolveWeapon, the ammo matches in the shooting states)
lands on the wrong item.
* UnitDieBState::convertUnitToCorpse opens both scopes around the size^2
creation loop. Only one is ever live: the record on the host, the guard on
the peer.
* ~UnitDieBState flushes the record onto `after_unit_death` as `minted_ids` -
the first packet after the corpses exist. Nothing is written when the death
produced none (overkill, a carried body whose item is reused, a respawn).
* The peer's `after_unit_death` handler stores the manifest (re-slaving its
item-id counter) and REMAPS the corpses it already has. Remap, not
consume-on-create, is what fires here: the packet is gated on
`_coop_task_completed`, so it only lands once the peer's own death replay -
started by the earlier `unit_death` - has finished creating them (hole H3).
The guard covers the reverse order and is idempotent with it; whichever runs
first drops the manifest.
* Turn-boundary hygiene on both machines (NextTurnState::close, the peer's
`next_turn` handler): a manifest whose replay never happened does not
outlive its turn.
test_coop_inventory_item_theft.py: the census comparison is now strict and
un-filtered at BOTH the shot and the inventory move, and the fixed `time.sleep(6)`
before it is a `wait_quiesced()` symmetric-poll barrier (poll both machines until
their censuses hold still). The PRD's revert target - the `relevant_census()`
corpse filter - does not exist on this branch, so the strict assertion is written
directly instead. It also now asserts the item-id COUNTER, which a census
comparison cannot see.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The other two Tier-A sites, both consume-on-create (path a): unlike the corpse
case the manifest lands BEFORE the peer's replay creates the items, so no
remap-on-load is needed.
* Death traps: BattlescapeGame::checkForProximityGrenades records the
STR_DEATH_TRAP_<floorSpecialType> item it had to create and flushes it onto
both `checkForProximityGrenades` trigger sends; the peer's
checkForProximityGrenadesCoop creates its copy inside the guard. No field
when the trap item was already on the tile (a second unit stepping on the
same trap mints nothing) - old-peer behaviour, unchanged.
* convertUnit: SavedBattleGame::convertUnit records the respawn's fixed
built-in weapons, minted by initUnit(), and flushes them onto the existing
`convertUnit` packet, keyed on the DYING unit's id (which that packet already
carries).
Also in this commit, found by the new scenario test: the counter re-slave was
off by one. Re-slaving at manifest-ARRIVAL pushed the counter forward before the
peer's factory had minted its local id, so that mint landed one past the host's
and the two counters ended permanently one apart (host 89 / client 90 on a
convertUnit respawn) even though the census agreed. It now runs once the
manifest has been APPLIED - the guard's destructor or the end of remapCorpseIds.
test_coop_alien_launcher_item_loss.py keeps its full-census comparison and gains
the two tripwire terms (item-id counter + desyncSeen): its blast kills the firing
alien, so it is a corpse-manifest test whether it says so or not.
New test_coop_id_manifest.py drives all three sites in one battle and asserts
census + counter equality and a quiet tripwire after each: a death trap
(MCDPatches marks the plain floor tiles specialType 200 and defines a harmless
STR_DEATH_TRAP_200), a blast kill of a 2x2 Cyberdisc (four corpse items in one
size^2 loop) and a stun-rod knockout that converts a sectoid into a chryssalid
(a stun rod deliberately: a blast overkills a 30-HP unit and UnitDieBState then
skips the respawn AND the corpses).
Known gap, reported not fixed: on the peer, convertUnit normally runs from the
LOCAL death replay (UnitDieBState::think) before the host's `convertUnit` packet
arrives, and getAlreadyRespawned() then makes the packet-driven call a no-op -
so path (a) only fires for respawns the peer did not reach on its own. The PRD
scopes convertUnit to path (a); closing the rest needs the corpse-style
remap-on-load, which is a follow-up.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s (PRD-P4) test_coop_id_manifest.py killed its 2x2 Cyberdisc with a blaster bomb aimed at the unit's OWN tile - fired from inside a 2x2 unit, so the projectile frequently never left the muzzle and the target survived five shots in a row (two suite runs in a row). Both kills are stun-rod knockouts now, which cannot miss and cannot overkill; overkill is the outcome that mints no corpses and runs no respawn, so it would have made the assertions vacuous either way. Blast-killed corpses stay covered where they already are: the blaster shot in test_coop_alien_launcher_item_loss and test_coop_inventory_item_theft kills its firing alien, and both now assert the full census plus the item-id counter. The Cyberdisc corpse count is asserted exactly (4) instead of "at least some". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With `EnableCoopParallelTurns` on, a co-op PVE/PVE2 battle stops alternating sub-turns: both machines hold the player side at once with full UI, and `_isActivePlayerSync == getHost()` permanently - the executor invariant (PROTOCOL.md "Core invariant") that makes every existing send/RNG/reaction guard correct without a 90-site refactor. The client's own input is swallowed behind a default-off debug option until PRD-P6 lands `action_intent`; host-only acting is the shippable intermediate. Option off = classic. - option + handshake + persistence: the two Options.inc.h declarations P0 left inert are now registered (OPTION_OTHER); the HOST's value rides `enable_parallel_turns` on COOP_READY_HOST and campaign_start into connectionTCP::_enable_parallel_turns (missing key -> false = old-build degrade), persists as `coop_parallel_turns` at all four SavedGame sites. - `parallelTurnActive()` / `parallelInputBlocked()` predicates; PARALLEL TURNS toggle on the host window (one extra button row, window +18px). - battle-init role writes: the PVE client branch now setPlayerTurn(2) with _isActivePlayerSync FALSE; the host branch already was the invariant. The PVE2 first-init branches already satisfy it (they hand straight to the AI) so gamemode 4 needs no restriction. Dead commented PVP block deleted. - think(): parallel-aware paused banner, all persistent off-turn banners suppressed (they are showMessage(msg,-1) and would squat on the widget P6/P8 flash through), one ownership-only unit selector keyed on localSeat(), no `selected_unit` follow packet. - `playableUnitSelected` gets the ownership predicate in parallel - reading the executor flag there left every action button dead on the client. - camera: a replayed peer chain no longer yanks the local view. New BattleAction::coopReplay (set by makeReplayAction) gates UnitWalkBState's per-step setViewLevel and ProjectileFlyBState's projectile follow. Corrects the PRD's movePlayerTarget site, which only fires under panic/AI. - side boundary: the host's END TURN closes the whole side (no PlayerTurnYour). `endTurn` gains the RNG `seed` PlayerTurnYour carried, parallel-only; the bulk unit/tile resync still rides `next_turn` at the side's other end. - harness: real `parallelActive` plus `parallelEnabled` / `clientInputBlocked` on battle_state/parallel_state/get_coop, and a read-only `warning` readout (WarningMessage::getMessage) so banner suppression is assertable. - tools/coop_test/test_parallel_sharedturn.py + its weight. Not executed, verified inert instead (would change CLASSIC behaviour, which must stay byte-identical): the BattleUnit.cpp isSelectable / InventoryState show_inactive simplification, BattlescapeGame.cpp's unit-select getCurrentTurn()==2 term, and the setupCursor CT_NORMAL guard - popState() and cancelCurrentActionCoop() are still replay-path callers, so the guard stays. Adjacent, not fixed: `battle_action move` bypasses mapClick, so the harness cannot press the §4 gate through a map click; END TURN is used instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… (PRD-P6)
The first truly-parallel playable build. PRD-P5 made both machines hold the
player side at once but swallowed the client's input; P6 replaces that gate with
the thin-client loop from PROTOCOL.md - the client ships an `action_intent`, the
host validates/admits/executes, and the client DISPLAYS the resulting broadcast
exactly as it displays a host action in classic co-op. The PRD-P5 executor
invariant (`_isActivePlayerSync == getHost()`) is what makes that free: every
existing BState send site fires on the host, so no chain needed re-plumbing.
Option off = classic, untouched.
PRE-TASK - the receive gate is a DEPTH COUNTER (AUDIT-rng cautions)
- `connectionTCP::_coopTaskDepth` behind the same setCoopTaskCompleted(bool)
call shape (false = acquire, true = release, clamped at 0; "completed" is
depth 0). Fixes the lost-wakeup an InfoboxState opening and closing INSIDE a
shot caused: the modal's close used to write the gate back open while the
projectile was still in flight.
- Melee and psi had NO coverage at all - MeleeAttackBState/PsiAttackBState now
acquire in init() and release in a new deinit(), mirroring UnitTurnBState.
- Every holder that can have init() re-entered carries its own `_coopGateHeld`
flag (UnitWalkBState re-inits after the UnitFallBState it pushes in front of
itself pops; melee/psi after their ExplosionBState). A second acquire would
never be released and would jam the peer's queue for the rest of the battle.
ProjectileFlyBState/InfoboxState need no flag - `_initialized` / ctor+dtor
already pair them.
- Teardown (DebriefingState, the peer-drop path) hard-resets the depth.
- The three per-action consumeNow exemptions are unchanged and still required:
each names the packet that CLOSES the chain currently holding the gate.
HOST ARBITER (connectionTCP)
- `_actionSeq` / `_sideSeq` / `peerDisplayAckedSeq` (P7's, dormant, reset in the
same place so its uint32 backlog term can never underflow) / the one-slot
pending intent / `_sideCommitInProgress`.
- `canAdmitAction()` = live player side && !sideCommit && `_states.empty()` &&
gate depth 0.
- `action_intent` handler: stale `side_seq` / AI side / side commit -> deny
`turn_over`; unit missing, `isOut()`, wrong faction, wrong seat, unresolvable
weapon or unaffordable -> deny `invalid` + the matching warning key; not
admittable -> deny `busy`; else stamp `action_ack {req_id, action_seq}` and
execute.
- `action_ack` / `action_deny` clear the client's pending slot (on ACK RECEIPT,
per the protocol's 2026-08-01 correction); a stale answer whose req_id no
longer matches is ignored. 10 s watchdog on updateCoopTask.
- `side_seq` rides the `endTurn` packet - the one message that already crosses
at every side transition - and `_actionSeq` resets there with it.
HOST DISPATCHER
`BattlescapeGame::executeAction(BattleAction&, bool calculatePath)` factored out
of the local-input tails; the host's own clicks and an admitted intent share it
(mapClick's walk/shot/spray/psi branches, launchAction, moveUpDown, the kneel
button, handleNonTargetAction's prime/melee). Kinds with no BattleState of their
own mutate synchronously and therefore re-broadcast their classic replay packet
from here (kneel, active_grenade, medkit) - the UI handler that used to send it
is not on this path. BA_TURN is the intent's dispatch key only: UnitTurnBState
branches on `_action.type == BA_NONE` for door-opening, so the state is handed a
BA_NONE turn and secondaryAction's local tail keeps `_currentAction` verbatim.
CLIENT CAPTURE
Every confirm site routes through `coopRouteAction`, which on a parallel client
serializes the intent and executes NOTHING, and on the parallel host runs the
same admission check (deny -> busy flash) before the normal tail. Sites: mapClick
LMB/RMB (walk, shot, throw, spray, psi, mind probe), launchAction, moveUpDown,
handleNonTargetAction (prime/unprime/melee), the kneel button, MedikitState's
three presses and ActionMenuState's single-purpose medikit auto-use. The client
also stops sending the classic commands the intent replaced (`action_click`,
`active_grenade`), and BOTH machines stop sending the two pure UI-mirror packets
(`unit_action`, `psi_press`) in parallel mode - they exist only to drag the
peer's selection/cursor around, which parallel mode decouples (the PRD-P1 rule).
`coopActiveGranade` / `coopHealing` were the last two replay handlers still
writing the receiving player's selection and `_currentAction`; in parallel they
now run on a stack-local action with the healer's own medikit (the packet gained
`healer_id`/`weapon_id`/`weapon_type`/`hand`, additive).
PRD-P5's temporary `coopParallelDebugClientInput` option is deleted outright
(declaration, registration, harness setter). `parallelInputBlocked()` survives as
"this machine is a parallel CLIENT" and now gates only END TURN, which stays
host-only until PRD-P8's readiness tally.
UX + INTROSPECTION
Five new keys in bin/common/Language/en-US.yml + en-GB.yml after the fork block
(STR_COOP_PLAYER_BUSY / TURN_OVER / NOT_YOUR_SOLDIER / ACTION_REFUSED /
ACTION_TIMEOUT), flashed through BattlescapeState::warning - which PRD-P5 cleared
of the persistent off-turn banners precisely so these get through.
`parallel_state`/`battle_state` gained actionSeq, sideSeq, peerDisplayAckedSeq,
sideCommit, canAdmit, admitBlocked, pendingIntent{reqId,seat,kind}, pendingReqId,
pendingKind and taskDepth; new `battle_intent` command drives the real capture
door (and its `probe_step`/`dry` sub-modes), deliberately separate from
battle_action/battle_fire, which stay the raw local-execution lever.
TESTS
tools/coop_test/test_parallel_intents.py - all six acceptance scenarios: walk
round trip with convergence and no selection/camera/_currentAction hijack; deny
busy + flash + retry; deny invalid (unowned / no TU on the executor / already
down) with the right key and nothing executed; one intent of every kind (turn,
kneel, stand, shoot, throw, prime, medikit, psi, melee) with the census and the
PRD-P2 tripwire quiet after each; a both-machines-within-one-RTT race with no
double execution; and a client-intent walk past a hostile. Full suite green
(117/117; test_coop_outcome_gaps needed its usual retry).
Adjacent, NOT fixed (report only):
- `connectionTCP.cpp` `active_grenade` receive declares `bool fusetimer` and
passes it to an `int` parameter, so any fuse > 1 arrives as 1. Pre-existing;
it now also clips a client's primed fuse.
- ActionMenuState sends `action_click` for BA_HIT AND MeleeAttackBState sends
`melee_attack`, so a UI-driven melee makes the peer run TWO melee chains.
Pre-existing (the harness drives melee through battle_fire, which skips
ActionMenuState). Intent-driven melee produces exactly one.
- ActionMenuState's single-purpose medikit auto-use sends no `medkit` packet at
all in classic, so it never replicated. The intent path does send one.
- `medikitRemoveIfEmpty` runs only on the acting machine in classic, so a
consumable medikit that empties leaves the item censuses apart.
Discrepancy vs the PRD: prd-p6 says the `after_unit_death && _coopInitDeath`
consumeNow clause is dead because `_coopInitDeath` is never written true. It IS
written true (ProjectileFlyBState::init, cleared in deinit), so the clause is
live. Left exactly as it was, per "if code contradicts the PRD, stop and report".
PROTOCOL.md updated in place (it lives under the gitignored .agents/): the intent
carries `weapon_id`/`weapon_type` beside `hand` (hand alone cannot name the item
that acted, and the host must resolve the instance without ever constructing a
BattleItem - issue #74) and `ignore_spotted`; `ba_type` is always stamped; TU
validation skips zero-cost actions; `side_seq` is published on `endTurn`; the
deny->warning-key map and the three re-broadcast legacy packets are documented.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PRD-P6 answered every contention with "no": an input that arrived while ANY
chain was running was refused `busy` and the player clicked again. Most of what
a chain IS, though, is a walk animation nobody is waiting for. P7 splits
contention in two and bounds how far the executor may run ahead of the peer's
display.
As built
--------
* `BattlescapeGame::chainIsSkippable()` - true iff every queued state is a
UnitWalk/UnitTurn/UnitFallBState of a FACTION_PLAYER unit. A shot, explosion,
death, melee or psi state, the end-turn sentinel, an AI actor or an EMPTY
queue all answer false. UnitFallBState carries no action (it is constructed
from the parent alone), so its actors are read off the save's falling list.
* Pending-admit. An input the arbiter cannot take, whose blocking chain is
skippable, is DEFERRED rather than refused: one slot per seat (newest wins,
the replaced one gets `action_deny busy`), the fast-forward is armed, and the
slot is admitted - with its `action_ack` - at the drain. The executor's own
click takes the same path through the same intent serializer, flashing
locally instead of sending a deny, and replays with `coopReplay` OFF so its
camera still follows. Admission happens on the main-thread tick, never from
inside popState(): executing an action pushes onto the very queue the pop is
walking.
* Cancellation. A chain that stops being skippable while fast-forwarded -
reaction fire pushing a ProjectileFlyBState into a running walk - clears the
flag and refuses every pending slot `busy`. Hooked from the three state-push
entry points and from popState(), which is also where the flag lapses on
drain.
* Fast-forward effect: the interval-0 branch an OFF-SCREEN walk already takes.
UnitWalkBState::setNormalWalkSpeed (covering both its init and think call
sites), UnitTurnBState::init and UnitFallBState::init. Projectile, explosion,
death, melee and psi intervals are untouched.
* Client display fast-forward: when a packet sits deferred behind a closed
receive gate and what this machine is animating is pure locomotion, stop
waiting for it. `abortPath`'s teleport-correct fixes the endpoint either way.
* Display flow control: `action_end` (host) -> `action_done` (client) ->
`peerDisplayAckedSeq`, and canAdmitAction() gains
`(_actionSeq - peerDisplayAckedSeq) < 2`. A stale report from across a side
boundary is dropped rather than underflowing the uint32 term.
Wire delta vs PROTOCOL.md (updated in place; .agents/ is gitignored here)
-----------------------------------------------------------------------
The sketch stamped `action_seq` on every broadcast packet of a chain. Replaced
by a single per-chain `action_end {action_seq}` marker, because a stamp can
only say "chain N started" while the flow control needs "chain N has finished
being DISPLAYED" - and because three admitted kinds (kneel, prime, medikit)
push no BattleState at all, while a turn that turns nothing and a walk with no
path ship no packet whatsoever. Those chains would have been invisible to the
client, which would never report them displayed, wedging the arbiter for the
rest of the side. The marker is deliberately NOT whitelisted: being consumed at
gate depth 0 is exactly what makes it mean what it means. It also carries the
host's own actions' seq to the client, which no `action_ack` ever does.
Sanctioned rider
----------------
`active_grenade`'s receive read the fuse into a `bool` before handing it to
`coopActiveGranade(..., int fusetimer, ...)`, so every fuse above 1 arrived as
1 and the -1 an unprime/failed prime ships arrived as 1 too - the peer armed a
grenade the executor had just disarmed. Pre-existing in CLASSIC co-op and fixed
for classic as well; P6's re-broadcast had made it clip a parallel client's
intent-primed fuses on top.
Classic mode
------------
Byte-identical apart from that type fix: every other change is behind
`parallelTurnActive()`, and `setCoopFastForward()` refuses to arm outside it.
Harness
-------
* `battle_camera {unit|x,y,z, visible}` - UnitWalkBState already runs an
off-screen walk at interval 0, and BattleUnit::_visible is only raised for a
player unit somebody can SEE, so a test that means to observe a slow walk has
to arrange both. Display-only.
* `parallel_state` gains fastForward, chainSkippable, openChainSeq, displaySeq,
displayBacklog, pendBlocked, pendingAdmits.
* `test_parallel_skip.py` (5 scenarios) and `test_parallel_speed_skew.py`, plus
test_weights.json entries.
* test_parallel_intents' deny-busy scenario now holds the host with a SHOT: a
walk is deferred from P7 on, so the old slow-walk hold no longer reached the
deny at all. Its `place_near`/destination picks now require BOTH machines to
be able to path (a teleport that took on one machine only left a blocker in a
different place, so a tile the client liked was occupied on the host - the
intent was admitted and then quietly did nothing).
Adjacent, reported not fixed
----------------------------
* The identical `bool fusetimer` truncation in the `action_click` receive
(connectionTCP.cpp, classic-only path in parallel mode - the client
suppresses `action_click`).
* `abortPath` carries position and facing but no TU, so after the LAST walk of
a run the peer's TU is whatever its own partial animation had spent when the
abort cut it off. Pre-existing; a deliberately slow peer exposes it (P7's
client fast-forward narrows the window, it does not cause it).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A parallel player side has no owner, so it cannot be closed by one player's button press. PRD-P5 let the HOST close it unilaterally, which was fine while the client could not act at all; from PRD-P6 on the client is playing, and that press cuts the other player off mid-thought. END TURN becomes a latching per-seat READINESS toggle on both machines (`end_turn_ready` -> host tally -> `end_turn_tally` echo, PROTOCOL.md), and the executor commits the side from its main-thread tick once every seat is ready, the arbiter is idle and the peer's display backlog has drained. PRD-P5's direct close moved out of btnEndTurnClick wholesale; the classic branch is untouched and every change is behind parallelTurnActive(). Readiness has two halves. The EXPLICIT half is the press. The AUTO half - "this seat commands no live FACTION_PLAYER unit" - is DERIVED from the roster every tick rather than hooked to the three events the PRD named (death, mind-control setCoop flip, gift): re-deriving cannot miss a fourth way a roster changes, and makes "gaining a unit clears auto" free. An admitted action clears that seat's explicit ready, which is what aborts a commit an intent beat to the tick (E14); after the commit starts, everything in flight is denied `turn_over`. Readiness resets inside resetActionArbiter(), so it can never survive a side boundary. Reserve is localized (PRD-P8 5). `TU_COOP` / `kneel_reserved` stop being sent in parallel mode and are ignored on receipt, and the `selected_unit` piggyback is guarded classic-only, so the two players hold their own reserve. The client's mode already rode `action_intent` (P6); P6 swapped it into the save around the synchronous call, which missed UnitWalkBState's PER-STEP check - a client's walk was judged against the HOST's reserve for its whole length. It is now a chain-scoped override keyed on the ACTOR, so it covers the whole chain, cannot leak onto another unit, and leaves the host's own setting (and its UI) alone. UI: the END TURN button inverts while this machine is armed, and a small "END TURN 1/2" sits above it while any seat is; a peer's toggle flashes STR_COOP_PEER_WANTS_END_TURN / STR_COOP_PEER_CANCELED_END_TURN. Neither is on the _warning surface - that one fades, and a waiting player needs the count. test_parallel_endturn.py covers all eight acceptance scenarios. Two are deliberately forced rather than raced: E14 exercises intent-first and commit-first separately (the mid-commit window is a frame or two wide, and an intent that misses it legitimately runs on the next side), and the drain barrier uses a SHOT rather than a walk - PRD-P7's client display fast-forward compresses any locomotion-only chain, so a lone walk never produces a measurable backlog. Adjacent findings, not fixed here: - test_parallel_intents is flaky on the fixture, INDEPENDENT of this change: 5 runs on a pre-P8 build of this branch gave 1 hard failure and 2 retry-passes. Both failure modes are ones the test's own comments predict (the client's pathfinder liking a tile the host cannot reach; a driver boxed into the Skyranger). It wants the retry-tolerated list, or a sturdier driver pick. - A client's rx hold queue can spin on an `endPlayerTurn` it will never consume (`_coopEnd == 1` excludes it from the interrupt whitelist forever), visible as a five-figure rxRotates. Pre-existing; harmless but noisy. Wire deltas vs the frozen PROTOCOL.md are documented in .agents/prds/parallel/PROTOCOL.md (message shapes are unchanged; the additions are the derived auto half, the tick-driven echo, the tally adoption rules, and the commit's precondition order). That directory is gitignored in this repo, so it cannot ride the commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dead (PRD-P9)
Five drift seams the PRD-P9 soak found, all of them pre-existing in classic
co-op and all of them visible as a cross-machine census divergence.
COSTS. Prime, unprime and medi-kit mutate synchronously inside a UI handler, so
they push no BattleState - and the peer therefore had nothing that would charge
them. It mirrored the EFFECT (the fuse, the healed wounds) and never the price,
so the two copies of a soldier drifted by the action's TU on every single use:
measured 31 on the executor against 62 on the peer after one prime. Each of the
three now ships the actor's post-action `tu`/`energy` and the receiver applies
them, presence-gated so an older peer behaves exactly as before. Same shape as
the `abortPath` walk-end fix in the sibling commit.
RESURRECTION, three paths. A unit that died while a replayed chain was still
running came back to life on the peer - dead on the executor, standing on 0 HP
here, for the rest of the battle:
* UnitWalkBState::think's AbortCoopWalk branch wrote STATUS_STANDING
unconditionally, and the executor's `unit_death` (sent from
UnitDieBState::init, mid-chain) always arrives BEFORE the `abortPath` that
closes the walk.
* UnitDieBState::think's client branch cancelled the peer's own local death -
correct, the peer never decides who dies - by writing STATUS_STANDING over
a unit the executor's packet had already marked dead.
* BattlescapeGame::turnPlayerTargetAfter (the facing-correction packet) called
abortTurn(), which is a STATUS_STANDING write, on whatever unit it names.
All three now leave a unit that `isOut()` alone. Every one is inside a co-op-only
branch, so single player is byte-identical.
Verified by test_parallel_soak.py: the per-unit census (position, TU, health,
stun, fatal wounds, isOut) is equal after every side across 5 full turns.
…ts (PRD-P9)
The receive/arbiter half of PRD-P9's rider ledger, plus the receive side of the
cost replication whose senders are in the previous commit.
R1 `action_click`'s receive read `fusetimer` into a BOOL before handing it to an
int parameter, so every fuse above 1 arrived as 1 and the -1 of an unprime
arrived as 1 too - arming a grenade the sender had just disarmed. Exactly the
defect PRD-P7 fixed on `active_grenade`; this path is classic-only (a
parallel client suppresses `action_click` outright), so it is a classic
co-op fix.
R2 `abortPath` ships the walk's END STATE, not just where it stopped: `tu` and
`energy` are applied when present. The packet already teleport-corrected
position and facing, so a peer whose animation had been truncated (a slow
machine, an interrupted fast-forward) kept whatever ITS walk had spent -
measured 2 TU on the executor against 44 on the peer at a 1:300 speed skew.
R4 Deferred inputs now expire. PRD-P7's pending store had no bound of its own -
it relied on the chain in front draining - so a wedged chain could hold an
input past the CLIENT's 10 s watchdog and then run it for a seat that had
already given up, with the ack landing on a req_id that no longer matched.
8 s, deliberately inside the client's window, refused `busy`.
R7 A packet excluded by a condition the pump cannot itself change (the
`endPlayerTurn` term, whose `_coopEnd` only ever moves inside the very
handler it excludes) is now PARKED rather than rotated, and spliced back to
the FRONT of the hold queue when the exclusion lifts. Rotating re-examined it
on every tick for the rest of the session (five-figure `rxRotates` on an
otherwise idle battle). Front, not back, is strictly closer to FIFO than the
rotate it replaces. `rxParkSize()` exposes the depth.
3 Stuck-chain diagnostic: one WARNING per chain that stays open for over 120 s,
carrying every term canAdmitAction() reads (isBusy, gate depth, hold/park
depth, display backlog, pending admits, admitBlocked, side commit). There is
no distributed lock in this design, so it frees nothing - it says so once,
with the state that would otherwise have to be guessed.
Soak findings, receive side:
* `active_grenade` / `medkit` apply the actor's `tu`/`energy` when present -
`medkit` keyed on `healer_id`, NOT `actor_id`, which is the patient.
* `unit_death`'s `isTile` now defaults to TRUE when absent. The field was only
ever sent on `after_unit_death`, and asBool() on a missing key is false, so
every death unlinked the peer's unit from its tile before its own
UnitDieBState ran - convertUnitToCorpse's `dropItems && getTile()` test then
skipped itemDropInventory and the dead soldier's whole kit stayed on the
corpse here while it lay on the floor on the executor.
PROTOCOL.md updated in step (PRD-P9 additions section).
…(PRD-P9) test_parallel_soak.py - the test that plays the game rather than one mechanism. A seeded battle (`set_seed`, so a failure re-runs), 119 admitted actions across 5 full turns with real alien sides in between, driven from BOTH seats: walks with contention (a second seat clicking into a running walk, PRD-P7's deferral), turns, kneel/stand, shots, a medi-kit, a prime, an unprime and a thrown smoke grenade. After EVERY side it compares the two machines term by term: units position, TU, health, stun, fatal wounds, isOut items the STRICT id census - id, type, owner, fuse, slot, tile (PRD-P4) tiles the fire/smoke hazard census (new `battle_tiles`) drift itemIdCounter / battleCensus (PRD-P2's two terms) wire `desyncSeen` false on both machines throughout Three exclusions, each with its reason in the source: energy (only three packets carry it), a NON-player unit's TU (spent by AI that runs on the executor alone; reported by tu_report() instead) and `status` (an animation state that legitimately differs mid-frame - it rides the failure text). Phase D is rider R3, and the measurement is most of what it delivers. The display-backlog cap had never been observed to engage; forcing it took three attempts, all recorded in the source: a WALK cannot build a backlog (PRD-P7's fast-forward compresses it away, since `action_end` is itself a gated packet and so ALWAYS waits behind a lone walk), a SECOND SHOT cannot expose the cap (`isBusy()` is checked before the display term, so the answer stays "states" until the peer has caught up), and what works is one slow shot followed by stateless chains, which complete inside the frame they are admitted and leave the executor idle while `_actionSeq` climbs. Observed engaging with backlog 2, a client intent refused `busy` with nothing executed, and a clean recovery. It is reachable but not on demand - it needs the peer really animating the shot, which needs the shooter on ITS screen - so a run that cannot force it reports the measurement instead of failing, and still asserts the accounting is live. test_parallel_resume.py - PRD-P9 6. A mid-battle campaign save taken while the parallel side is live, both processes replaced, the pair resumed: the mode comes back (`coop_parallel_turns` survived), the executor invariant holds, the arbiter returns RESET rather than half-remembered (actionSeq / peerDisplayAckedSeq / pendingAdmits / the client's pending slot at 0, tally empty, side_seq agreed) and both seats can act again with the drift terms equal. R6 - test_parallel_intents' driver hardened rather than blanket-retried: `free_step_both` widens its radius before giving up (the fixture packs 14 soldiers into a 2x7 Skyranger, so a driver boxed in by its own squad has nothing at radius 2 and open ground two rings out), `place_near` insists on TWO shared exits before falling back to one (a driver with a single way out fails the moment a corpse lands on it), and the new `step_dest` re-places a driver that has become boxed in instead of turning a fixture accident into an intent failure. Harness introspection (test-only, TestServer.cpp): `battle_tiles` hazard census, `wounds`/`energy` on battle_state units, `rxPark`, an `energy` field on `battle_intent` (a soldier out of ENERGY refuses to walk exactly as one out of TU, which silently turned "drive a long walk" into "drive nothing"), a `battleFireSpeed` lever (battleXcomSpeed only covers walk animation, which P7 fast-forwards), and a dead duplicate `commitBlocked` assignment removed.
… (PRD-P9)
CHANGELOG gets the user-facing Added entry for the feature (how to turn it on,
what changes, what falls back) and the five co-op Fixed entries from the P9
hardening pass, written for a player rather than for the protocol.
The operator guide itself lands in `.agents/docs/parallel-battlescape-turns.md`,
which is outside the repo (`.agents/` is gitignored, like the PRDs it sits
beside): the option and its handshake fallback, the intent model in a paragraph,
the contention answers, the end-of-side rule, the twelve known limitations and
the diagnostic readouts. `.agents/prds/parallel/README.md` now carries a
"shipped through P9" status header and PROTOCOL.md a PRD-P9 additions section
listing every field this pass added to the wire.
GUARD SWEEP (PRD-P9 1). Re-ran the three audit greps at this HEAD and classified
every hit. NOTHING in product code was deleted, and the PRD's premise for that
task does not hold - reported rather than executed:
the AUDIT "UI cluster" (BattlescapeState warning gates :4616/4632/4659, draw
gates :3956/4054/4129/4390, launch button :6320, Map.cpp :1709/:1788,
InventoryState :369, the preview gates BattlescapeGame :3359/3436/3548) is
listed as "now-dead never-true". It is not. Those guards key on
`isYourTurn == 1` / `getCurrentTurn() == 1`, which is exactly the state a
CLASSIC co-op off-turn player holds - and classic mode is explicitly retained
(README "option off = byte-identical"). They are dead only in PARALLEL mode,
where both machines hold 2, which is what makes the full UI live on both
without touching them. Deleting them would change classic co-op behaviour,
which the same ground rule forbids.
accounting, all 62 `_isActivePlayerSync` hits + 34 turn-guard hits:
ORIGIN/executor send or RNG author (correct under `_isActivePlayerSync ==
getHost()`): BattlescapeGame :3351/:4034, MeleeAttackBState :289/:301,
Pathfinding :791, Projectile :202/:401/:428, ProjectileFlyBState :95/:589,
PsiAttackBState :147, UnitTurnBState :55, UnitWalkBState :89/:143
RECV/replay hydrate or suppression (correct, client-only): MeleeAttackBState
:74, ProjectileFlyBState :113/:309, Projectile :436, PsiAttackBState :71,
TileEngine :2550, UnitWalkBState :296/:397/:601
HOST/ambient authority: BattlescapeGame :1019 (falls), :1027 (non-player side)
PVP-only (gamemode 2/3, parallel excluded by construction): BattlescapeGame
:4090, BattlescapeState :1856/:1863/:1905/:1911, :3261-3274
CLASSIC-only, KEEP: the whole UI cluster above, BattlescapeState :1442
(off-turn banner, already `coopParallel == false`-fenced), :2149/:2209/:2221,
:3424/:3429/:3486/:3491/:3736, :3937/:3940, connectionTCP :1291/:1293
WRITE sites, P5-fenced: BattlescapeState :1885-1893 (client: turn 2,
executor FALSE), :1933, :3153, DebriefingState :330, GeoscapeState :1248,
connectionTCP :8941/:9516-9548/:12218/:13517
DIAG (test-only): TestServer :5085/:5090/:5364
VERIFIED-KEEP, load-bearing (PRD-P9 rider R9): BattleUnit::isSelectable's
`isYourTurn == 2` branch (:5341) - it IS the live ownership gate in
parallel mode; BattlescapeGame :3803 unit-select split; the setupCursor
CT_NORMAL guards
DEAD, deleted: one duplicate `resp["commitBlocked"]` in TestServer
(test-only). Also noted, not deleted: `BattleUnit::stopCoopWalk()` has no
callers anywhere.
… (PRD-P9) The rider-R3 phase fires auto bursts at whatever tile produces a chain that actually runs, and the skirmish fixture can ship as few as ONE hostile - so a stray hit ended the mission and made every remaining turn vacuous. The shooter is now teleported (on both machines) to a tile at least eight clear of every live hostile before the phase starts. Also: the census now fails with "the fixture's mission ENDED" instead of a three-minute hang and a bare socket error. The drain loop was popping the debriefing, then the geoscape, then the MAIN MENU - which quits the game. `KEEP_STATES` lists everything the soak must never dismiss.
…out) The soak's skirmish fixture came up on NEW BATTLE mission index 0 (STR_SMALL_SCOUT: lowQty/highQty 1, dQty 0), i.e. exactly ONE hostile. Any run that killed it ended the mission mid-soak and every later census was vacuous - the run reported the fixture fail-fast instead of testing anything. Pin the mission to index 1 (STR_MEDIUM_SCOUT, two ranks at lowQty 2 and 1 => >= 3 hostiles even on Beginner, same 40x40 footprint) by writing a one-key battle.cfg into each hermetic user dir, which is the game's own fixture file (NewBattleState::load). No `base` key, so NewBattleState still takes its usual initSave() path. A new floor assertion fails fast and loudly if the generated battle ever ships fewer than 3 hostiles again. CI weights: test_coop_id_manifest (122 s, measured), test_parallel_resume (104 s, measured) and test_parallel_soak (345 s, PRD-P9's measured 314-375 s band) had no entry, so plan_shards.ps1 was giving them the ~13 s median - badly skewing the shard split for the two long ones. NOT fixed here (test-only session, reported instead): * With >= 3 hostiles the soak no longer reaches 3 consecutive clean runs. Casualties during the ALIEN side diverge: the executor runs convertUnitToCorpse (kit to the ground, owner -1) while the peer keeps the dead unit's inventory on the body, and a player unit's reaction-fire TU cost does not replicate. Both persist >= 30 s with both machines idle (displayBacklog 0, rxHold 0, isBusy false), so neither is a settle race. * test_shared_month_run is flaky on this HEAD (FAIL/FAIL/PASS): a manufactured STR_LASER_PISTOL completing at month end reaches the host's world but not the client's. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The hardened >= 3-hostile soak (PRD-P9 closeout) found casualties during the ALIEN side diverging the two machines. Root-caused with packet-flow instrumentation on failing runs; five independent defects, all in the death/reaction seam, all pre-existing in CLASSIC co-op too. 1. TILE UNLINK vs the death replay. `after_unit_death` is sent after the executor's convertUnitToCorpse, so it always says "no tile", and it is exempt from the receive gate (`_coopInitDeath`) - so it routinely lands while the peer's own UnitDieBState is still queued behind an animation. The unlink then cost that replay everything reading getTile(): convertUnitToCorpse skipped itemDropInventory (the casualty's whole kit stayed on the body here and lay on the floor there - the reported "host has HC+ammo on GROUND, client keeps them on unit 9's BELT"), and UnitDieBState::init() popped the state outright for a non-PLAYER unit - no corpse and no drop at all (the reported missing STR_SECTOID_CORPSE). `next_turn`'s per-turn stamp carries the same unlink and lands at the harder moment: the alien side's last casualty dies a frame before the side closes. Both sites now defer to SharedEcon::corpseReplayPending(). 2. `after_unit_death` SENT FROM THE DESTRUCTOR. A BattleState is destroyed by cleanupDeleted(), which runs at a turn boundary - not when the state finishes - so the packet carrying the death's final unit state AND the PRD-P4 corpse id-manifest went out up to a whole side late (measured: 82 s). Moved to deinit(), which popState() calls the instant the state pops, exactly once. 3. DOUBLE DEATH vs the id-manifest. remapCorpseIds' "do the corpses exist here already?" test is a BT_CORPSE scan keyed on getUnit(), and an UNCONSCIOUS unit is ALREADY represented by such an item. A unit that is knocked out and later bleeds out ships two deaths, and the second one's manifest re-stamped the OLD body item; the peer then removed that body and minted its corpse one id further on. The manifest now parks while the replay is queued (consume-on-create takes it instead) and survives the turn boundary that used to drop it. 4. PANIC never replicated. handlePanickingUnit() is host-gated in both modes, but the peer ADOPTS STATUS_PANICKING from `next_turn` (sent before the host resolves anything) and kept it, with a full turn's TU, for the rest of the battle. New `panic_action` outcome packet (PROTOCOL.md updated) ships what UnitPanicBState itself writes - abortTurn, clearTimeUnits, +15 morale; the drops, the flee walk and a berserker's shots already had carriers. 5. REACTION-FIRE TU. CoopShoot pushed the peer's pre-shot UnitTurnBState with chargeTUs defaulted TRUE, on top of an `actor_tu` the executor read AFTER its own turn was charged. Reaction fire made it permanent: the executor turns for FREE inside ProjectileFlyBState::init, so every replayed reaction shot billed the peer a facing cost nobody paid. Plus: the id-manifest hands back the local id an adoption wastes (the peer minted it, bumped its counter past it, then adopted a LOWER host id - the counter stayed one ahead for the rest of the battle whenever the two machines created a Tier-A batch in a different order), and the drift tripwire skips a stamp that lands mid-death, when its two terms are legitimately one corpse apart. CLASSIC MODE: 1-5 are all shared between classic and parallel co-op and are fixed for both. The option-off byte-identity bar is behaviour vs the PRE-PROGRAM build; these are genuine pre-existing classic desyncs, so the change is deliberate and desirable. Full affected-area suite green. Test-only, same commit: * test_parallel_soak close_side no longer re-arms into the NEXT side. `allReady` is one frame wide (the executor commits on the tick that applies the second seat's readiness, and committing resets it), so the poll for it always ran its full 30 s timeout and the fallback closed a second side with no actions in it - every census after turn 1 was racing a battle advancing behind the test's back. * The soak re-rolls the battle when the generator hands back fewer than 3 LIVE hostiles (observed 3 of 5 arriving on 0 HP, reproduced on a build with nothing but logging added). NOT fixed here (reported, outside the death/reaction seam): the receive hold queue REORDERS. A packet that cannot be consumed is rotated to the back while the always-consume list behind it (hit_tile, unit_fire, set_smoke_tile...) is applied, so a `BattleScapeMove` blocked once ends up permanently behind its own follow-ups and the peer walks that unit from a stale position with no corrector. Seen only with a badly backed-up client (rxRotates > 200k, which the soak's rider-R3 phase manufactures); it surfaces as an alien position/energy diff. Fix shape: consume in place from the deque, skipping only messages whose subject unit already has an unconsumed earlier one, instead of rotating the head. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The last known battlescape-sync defect, found by PRD-P10's soak, was in the
receive pump rather than in any handler. `updateCoopTask()` rotated a packet it
could not consume yet to the BACK of `g_rxHold` and carried on, applying the
always-consume traffic behind it as the pass continued - so a unit's own
follow-ups could be applied while its earlier packet was still waiting. The
recorded trace (the numbers are the actor's TU, i.e. one unit):
host sent Move(47) abortPath(37) Move(37) abortPath(27)
peer applied abortPath(37) Move(37) abortPath(27) ... Move(47)
- the first move landing two seconds late, the peer then walking that unit out
of a stale position with nothing left to correct it. Needs a badly backed-up
client (rxRotates > 200k) and cost about one soak run in six before P10.
INVARIANTS THE PUMP HAD TO KEEP (audited before designing; all preserved):
1. the gate itself: consume at depth 0 (`coopTaskCompleted()`) or on the
always-consume whitelist, and nothing else;
2. the three per-action exemptions (`abortPath` while `_coopWalkInit`,
`unit_death` / `after_unit_death` while `_coopInitDeath`) - each CLOSES the
chain that holds the gate, so waiting for depth 0 deadlocks them;
3. `action_end` stays gated (PRD-P7): it must not jump the line, because the
client reports a chain displayed exactly when it consumes that marker;
4. `g_rxPark` (PRD-P9 R7): the `endPlayerTurn` exclusion is parked, not
retried, and re-enters at the FRONT when the exclusion lifts;
5. no starvation: the pass must terminate, and the pass-consumed-nothing exit
must still stop the pump busy-waiting;
6. the queue survives a mid-pass session teardown without resurrecting
packets from a session that is over.
AS BUILT
Nothing is rotated. The pass walks `g_rxHold` front to back, consuming in
place; a packet it cannot consume goes to a per-pass `deferred` deque and the
whole deque is spliced back onto the FRONT when the pass ends, so the queue is
exactly what it was minus what was applied. A packet that is only skipped keeps
its position, which means it gets first refusal every pass instead of coming
round last - the term the old pump got wrong when the gate opened part-way
through a pass.
On top of that, per-subject ordering: the unit a packet is about is extracted by
`coopPacketSubject()` (a whitelist keyed on the state string - the wire spells
the same field `id`, `unit_id` and `actor_id`, and a bare `id` means a base, a
craft or a vote elsewhere), and no packet may be consumed while an earlier,
still-unconsumed packet names the same unit. Two carve-outs, both measured:
* CHAIN CLOSERS are exempt (invariant 2). Holding one is not a delay but a
deadlock: anything queued between a walk's `BattleScapeMove` and its
`abortPath` - a mid-walk reaction hit, say - would block the only packet
that can reopen the gate. Without this carve-out the liveness floor below
fired twice in a five-turn soak, i.e. two ten-second stalls.
* `unit_fire` is NOT in the subject table. It is the only always-consume
packet that names a unit and is not a closer, so it is the only one this
rule could actually delay - and delaying it is worse than applying it early.
The peer burns its own units (`BattleUnit::prepareHealth` subtracts fire
damage and decrements `_fire` every turn tick) and on a peer `_fire` is
written by nothing but this packet, so one held across a turn boundary
burned the host's unit and not the peer's: measured as a one-item census
drift (a corpse the host had and the peer did not) in a five-turn soak.
LIVENESS FLOOR: if a whole tick consumes nothing while per-subject blocking is
holding something back, and that repeats for 600 ticks (~10 s), the next tick
runs with blocking disabled, so this pump can never be stuck longer than the old
one. `rxLegacyPasses` counts it and is expected to stay 0; the new test asserts
that on both machines.
WHAT STILL ROTATES: nothing. `g_rxRotateCount` is kept under its old name and
now counts GATE HOLDS - packets the gate refused, which keep their queue
position - so the existing readouts still mean "packets the gate would not let
through". `g_rxSkipBlocked` is new and counts the ordering holds. `g_rxPark`
still leaves and re-enters the queue, unchanged (invariant 4).
TEST INTROSPECTION (test-only, both in the shipped Release build because the
harness drives it): an applied-packet ring (`parallel_state {trace: true}` ->
`[{seq, state, unit}]`, 256 deep) so a test can assert application ORDER rather
than end state, and `rx_inject`, which puts packets straight into the hold
queue. `rx_inject {awaitGate: true}` ARMS a batch that the peer's own main loop
lands on the first tick whose receive gate is shut - the window this rule is
about is a few frames wide, and a round trip loses that race every other run.
RED/GREEN: with per-subject blocking removed, an always-consume packet injected
behind an earlier packet for the same unit is applied FIRST
(`['unit_fire', 'hit_unit']`); with it, the order is preserved
(`['hit_unit', 'unit_fire']`) - reproduced on the first attempt in both
directions. That pair is no longer what the shipped test injects, because
`unit_fire` left the subject table for the reason above; the test now injects a
three-packet burst of gate-held no-ops and asserts all three are applied in the
order they landed, none starved, none overtaken.
CLASSIC: the pump is shared, so this fixes a classic-reachable ordering hazard
too. Correct ordering IS the pre-backlog behaviour - with an empty or shallow
hold queue the two pumps are indistinguishable - so the full-suite byte-identity
bar is met in behaviour, not merely in the option being off.
Adjacent, NOT fixed here (scope): the peer applies fire damage locally from a
host-supplied `_fire`, including its own `RNG::generate` roll, which is a Tier-B
authority seam of exactly the kind the P0-P4 arc exists to remove.
Tests: new tools/coop_test/test_parallel_rx_order.py (walk-level order, injected
burst order, liveness floor quiet, convergence, tripwire quiet). Soak: 3
consecutive clean runs. Affected-area regression green except
test_parallel_skip, which fails IDENTICALLY at HEAD (90f6417) with the same
message and the same state dump - pre-existing, not from this change; the soak
also failed 2/2 at HEAD on this machine while the final build ran 3/3 clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…1 closeout)
Scenarios 1 and 3 need the client's `action_intent` to reach the arbiter while
the host's walk chain is still draining. That window was whatever the fixture
roll handed out, so the test failed about one run in three on a loaded machine -
never on a product assertion, always on a window that never opened.
Three fixture causes, all found by making the test report what it measured
rather than assume it:
* ENERGY. `PI.top_up` restores TU only, and energy is what runs out first: a
soldier with 200 TU and no energy is admitted, drains in ~1 s and does not
move. That is the "1.0 s baseline" exactly. Local `top_up` now tops up both,
the way test_parallel_soak.py already did (TestServer's `energy` field
exists for this, PRD-P9).
* CASUALTIES. The walker is marched past hostiles repeatedly and (scenario 4)
over a primed grenade, so it gets knocked out - `status` 6 - and a casualty
never walks again, no matter how the destination is re-rolled. `live_driver`
detects it and switches to another soldier, threaded through scenarios 1-4.
* DIRECTION. A walk that closes on an alien draws reaction fire, which pushes
a ProjectileFlyBState into the chain and correctly cancels the fast-forward -
the deferral under test is then refused rather than admitted (four wasted
rolls in one run). `long_walk_target` now picks the furthest reachable tile
that also leads AWAY from the nearest hostile.
The window itself is now measured, not hoped for: scenario 1 times an
uncontended run of the same walk and re-rolls until it covers WALK_MIN_TILES and
lasts WALK_FLOOR_S, escalating two dials cheapest-first (widen the path, then
slow the frame). Scenarios 3 and 4 arm at the pair it found. TILES, not seconds,
is the dominant axis: the fast-forward pins the interval to 0, so one tile left
over is a ~50 ms flag no socket poll will see.
Assertion contract unchanged and slightly strengthened. `+2` on action_seq is no
longer taken as proof of a deferral (an intent arriving after the drain reaches
the same count), so `watch_defer` samples fastForward, pendingAdmits and the
drain in ONE poll loop and all three must hold; the non-transient
`lastDenyReason` latch, cleared before each attempt, now backs the fading
warning-widget reads in scenarios 1-3. Every retry path covers a fixture
accident only - a walk that went nowhere, a KO, a cancelled skip - and each
prints why.
Also in scenario 3: the interruption's aim point is resolved BEFORE the walk
starts. A fast-forwarded walk drains in well under a second, so the
`battle_state` dump that used to sit between arming and `battle_fire` was itself
enough to miss the window, admitting the pending intent instead of cancelling
it. Includes the pre-existing deny-latch hardening of that scenario's flash read.
Validation: 6 consecutive runs, all exit 0 (119/142/139/117/153/127 s).
Regressions: test_parallel_intents.py and test_parallel_speed_skew.py both exit 0.
Test-only; no engine change.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two playtest notes on the PRD-P8 end-turn UI. The two orange peer-toggle flashes go entirely. Both sites were in connectionTCP::onTCPMessage: the host's `end_turn_ready` receipt and the client's `end_turn_tally` application. Neither carried state - the whole body of the client-side one was a diff loop feeding flashBattleWarning, so it goes with it, and with it the now-unread `seats` local. The seat bit is now assigned unconditionally on the host (the old `if (ready != want)` guard existed only to gate the flash). The two STR_COOP_PEER_* keys are dropped from en-US/en-GB rather than left dead. The "END TURN r/n" tally is painted `_indicatorGreen` - the `squadsightUnits` interface element, i.e. the exact index blinkVisibleUnitButtons() fills the numbered enemy indicators with for their green state (54 in xcom1, 86 in xcom2). Read off the member rather than written as a literal, so a mod re-colouring the indicators re-colours this too. Position, size and visibility are untouched; the element the widget is add()ed with carries colour -1 in the battlescape interface, so nothing was overridden. Adjacent, NOT fixed here: test_parallel_endturn.py scenario 4+6 (the display drain barrier) is red, and it is red at HEAD too - a baseline run of the unmodified binary failed with a different mode (`not_top_state`/`states` instead of `no_side`). Its premise does not hold: `battleXcomSpeed` only paces UnitWalk/UnitTurn/UnitFall, which PRD-P7's client fast-forward compresses to interval 0 anyway, and ProjectileFlyBState pins its own interval at 1000/60 and never reads the option - so the "deliberately slow client" is a no-op and the barrier window is one network round trip. `action_done` is emitted from the client's receive path, not from the display finishing, so parking the client's battlescape under a modal does not widen it either (tried). Observing the barrier needs a TestServer lever that holds `action_done`. test_parallel_sharedturn.py passes; its banner-assertion comment loses its reference to the removed flashes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Manual parallel-turns play (skirmish + ABORT) showed the two players'
debriefings disagreeing: the host counted 2 alien kills, the client 1.
DebriefingState::prepareDebriefing() runs on BOTH machines and builds the
score page from the LOCAL save - the host's debriefing packet carries only
the soldier-stats and diary pages, and it is applied AFTER prepareDebriefing.
Its alien-kill row is
oldFaction == FACTION_HOSTILE && bunit->killedBy() == FACTION_PLAYER
and killedBy (with murdererId) was the one part of a death that never crossed
the wire: each machine derived it in its own checkForCasualties. The peer runs
that pass over the same victim only while it is REPLAYING the attack, so
ordinary shot kills agreed and the gap hid. It never replays an alien-side
reaction shot as a local attack chain, so a reaction-fire kill left the alien
on the BattleUnit ctor default `_killedBy = its own faction` - no kill, no
points, on the client only. Repro on a 3-alien skirmish: same alien, host
killedBy=0 murdererId=10, client killedBy=1 murdererId=0; tally 3 vs 2.
Fixed at the source, not the display: killedBy is SAVED (BattleUnit::save
writes it unconditionally), so this is persisted state, not a label - shipping
the host's computed tallies on the debriefing packet would have papered over a
save that still disagrees. UnitDieBState::coopWriteKillAttribution() stamps
killedBy + murdererId on `unit_death` (while the death is still displaying) and
on `after_unit_death` (the definitive re-stamp); the peer adopts them and never
re-derives. Presence-gated both ways - absent means an older peer, which keeps
its local derivation. The peer's own kill diaries are host-replaced at the
debriefing (kills.clear() then refill), so the extra murderer resolution the
stamped murdererId enables cannot double-count.
Rider, same additive shape: psi_result (pvp: false) gained `energy`. A
successful mind control runs recoverTimeUnits(), which restores BOTH tu and
energy; only tu was shipped, so the peer's copy of a mind-controlled victim
kept whatever energy it had when the attack landed.
Harness: `debrief_state` reports the score page's rows, points and total
(DebriefingState::harnessStats()), which is what makes the user-visible symptom
directly assertable.
test_coop_debrief_sync.py drives, for both endings (ABORT and WIN), a
host-executed kill, a client-intent kill and at least one alien-side reaction
kill, then asserts identical per-alien attribution AND identical debriefing
rows/points/totals on both machines. It fails rather than passing vacuously if
the fixture never produces an alien-side death. Stock STR_SMALL_SCOUT holds one
alien, so the test generates a throwaway deployment ruleset (the
test_coop_outcome_gaps pattern).
Adjacent, not fixed here: a skirmish debriefing returns to the MAIN MENU, so
session.drain_to_geoscape() walks off the end of the stack and pops the menu
(the process exits) - campaign-only helper, noted in the new test. And
test_parallel_soak drifted once on unit status after an alien side
(host DEAD/UNCONSCIOUS vs peer STANDING at 0 HP, the pre-existing peer
death-replay race); it was clean on re-run and nothing here touches status.
Reported from manual parallel-turns play: after ABORTING a NEW BATTLE > COOP mission the host was handed a LOBBY offering RESUME GAME, and pressing it dropped them on the GEOSCAPE. Winning does the same. Root cause. A co-op skirmish world arrives through LoadGameState on BOTH machines, so every skirmish stack is [GeoscapeState, BattlescapeState] - there is a dead geoscape buried under the battle. When the mission ends both players get a DebriefingState over it, and whoever presses OK first leaves correctly (monthsPassed == -1 -> GoToMainMenuState, which drops the SavedGame, issue #82). That exit disconnects, and the disconnect looked like a DROP to the player still reading their debriefing: host - CoopState(20) "<client> has left the server" plus, in disconnectTCP's teardown, the LobbyMenu re-open meant for a drop while the host sits on the NEW BATTLE setup screen. LobbyMenu's ctor saw the buried geoscape with the session still locked, latched _resumeToGame, and offered RESUME GAME, which pops the debriefing away and lands on that geoscape. client - CoopState(21) "Server connection lost" over its debriefing. Both are the issue #79 bug class ("one player's exit affects the other") with a skirmish end screen instead of a finished campaign. Fix. connectionTCP::skirmishMissionOver() - the skirmish twin of campaignEnded() (lobbyMode 0 + no live battle + coopMissionEnd). While it holds, a peer leaving is silent on both sides and the lobby is never re-opened; each player's own debriefing OK is the exit and it goes through the GoToMainMenuState chokepoint. Campaign missions keep the freeze/wait route, a drop DURING a skirmish battle keeps its dialogs and rejoin, and a drop BEFORE the battle still re-opens the lobby. Stack after the fix, both endings and both close orders: the second player keeps [GeoscapeState, DebriefingState] with no lobby and no coop popup, and both machines finish on [MainMenuState] with hasSave false. Tests. New tools/coop_test/test_skirmish_end_main_menu.py drives one skirmish per ending with the debriefings closed in a different order each time (abort/client-first = the reported repro, win/host-first = the mirror). Red-run with the guard forced false reproduces the reported stack ['GeoscapeState', 'DebriefingState', 'LobbyMenu', 'CoopState']. Green: boot_check, test_skirmish_flow, test_skirmish_battle_turn_control, test_skirmish_rejoin_battle, test_resume_game_in_battle, test_vote_abort_battle, test_coop_debrief_sync, test_lobby_dialogs, test_reconnect_dialog, test_rejoin_flow, test_session_hardening, test_lobby_gating, test_coop_resume_battle_control, test_parallel_skip, test_coop_outcome_gaps. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d lever
test_parallel_endturn.py scenarios 4+6 (the end-turn display drain barrier)
have been red since ~P9. It was a TEST defect, not product: the "deliberately
slow client display backlog" premise is false.
- battleXcomSpeed paces only UnitWalk/UnitTurn/UnitFall, and PRD-P7's client
display fast-forward pins exactly those to interval 0 whenever a gated
packet is waiting - and `action_end` always is.
- ProjectileFlyBState fixes its own interval and never reads the option, so
the shot the scenario used as a workaround does not depend on the lever it
was setting either.
- Freezing the client's battlescape under a modal does not help: background
replay keeps draining via handleStateCoop and `action_done` is emitted from
the gated drain in updateCoopTask.
So the real barrier window is one network round trip (~200 ms) and no fixture
arrangement can widen it. The missing piece was observability, not a fixture.
Lever: TestServer `hold_action_done {hold: true|false}`. While held the CLIENT
parks its `action_done` reports instead of shipping them; releasing emits the
newest parked seq (peerDisplayAckedSeq is deliberately not advanced while held,
so one emit subsumes the whole backlog). No wire change - same packet, same
single P7 emit point, it just leaves when the test says so.
Product touch is two statics plus a five-line early return at
connectionTCP::coopEmitActionDone() (default off, only the test server writes
it). `parallel_state` gained `holdActionDone` and `heldActionDones`; the latter
counts reports parked since the hold was engaged, which is what proves the
client really finished DISPLAYING rather than merely lagging - a scenario
resting on a count of 0 would be vacuous.
Scenario 4+6 rewritten around it: drain to backlog 0 -> hold -> the host runs
ONE chain (a plain walk now; the shot was only ever a workaround for the
missing hold) -> both seats ready -> the commit sits on `display_backlog` with
canAdmit true for a fixed 12 s window -> release -> the side closes exactly
once and sideSeq bumps. One chain is the point: the arbiter refuses to admit at
a backlog of 2, so a single outstanding chain is the only state in which the
display report is provably the last thing between two ready seats and the side
close. Every existing assertion is kept; the hold is released in a finally that
cannot mask a real failure, and the scenario asserts the lever is disengaged
before the later scenarios run.
Validation: test_parallel_endturn.py 4/4 consecutive green (35 held samples per
run, 1 report parked, side closing 4-7 s after release); test_parallel_speed_skew,
test_parallel_skip, test_parallel_intents green; boot_check clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The battle tripwire watched two ITEM terms. Neither can see the drift players actually report - a unit that is dead on one machine and standing on the other - until the corpse is minted, which is a whole side too late, and never sees a unit that simply ends the side on a different tile. Adds a third term, `chkBattleUnits`: an order-independent sum of FNV-1a over getUnits() of id, faction, LIVENESS and position. Stamped on `next_turn` and on the world checksum next to the other two, negative = "not stamped" = agree, so an older peer cannot raise a false positive. FIELD SELECTION is the whole design: * LIVENESS, not the raw UnitStatus: 0 = on its feet / 1 = DEAD / 2 = UNCONSCIOUS / 3 = IGNORE_ME. Every animation phase (WALKING, TURNING, AIMING, COLLAPSING, PANICKING) collapses to 0, because the peer is a display that lags the executor and the two are never on the same frame. This is BattleUnit::isOut() refined by which out-state it is - dead versus merely stunned is a real difference, a stunned unit wakes up. * EXCLUDED, each because it differs legitimately at the compare instant: TU and energy (both machines regenerate TU in their own prepareNewTurn, which straddles this stamp; a non-player unit's TU is a documented pre-existing seam), health / stun / fatal wounds / morale / mana (same straddle - fatal-wound bleed and stun recovery run independently), the raw status and the facing. What is left is the intersection with the set test_parallel_soak ASSERTS after every side. The `next_turn` compare MOVES ahead of that handler's bulk unit apply. The packet rewrites every unit's position and status from the host's snapshot, so a compare made after it agrees every time - the term would have been silent by construction. The item terms do not care (nothing in that handler touches an item), so all three move together. Comparability: `next_turn` carries no subject, so the PRD-P11 receive pump lets it overtake a per-unit chain it could not consume in the same pass - by design, the snapshot is a repair. While it has, this machine is one walk or one death behind the peer through the protocol working as intended. New `rxPassDeferred()` reports that, and the unit term stands down for that stamp; the item terms keep comparing, because an id minted on one machine only is not something a late chain can heal. Measured in a soak: an alien three tiles and 12 TU apart at the stamp, identical again by the time the side settled. Reporting rides the existing path: the log line now NAMES which term(s) diverged and, for the unit term, dumps this machine's per-unit id:faction/liveness/rawstatus@x,y,z, so the two machines' logs diff to the unit in one step - a sum says THAT the unit sets differ, never which unit. Harness: `battleUnitsChecksum` on battle_state, session.battle_checksum returns the triple. assert_battle_synced READS the unit term and reports it but does not assert it - it is a settling quantity and some inputs differ legitimately for a whole side (test_coop_outcome_gaps documents a spawned unit sitting a z-level apart after a blast until next_turn repairs it). It is asserted where it IS an invariant: the in-game tripwire at the turn boundary, and the soak's own per-unit census at full quiescence. Red/green in test_battle_tripwire section 5, on a TEST-ONLY one-sided status write (`battle_intent` gains a `status` setter): the unit term moves on one machine and both item terms do not - the exclusivity proof - the tripwire then fires, its log line names `units` and carries the per-unit dump, and the same `next_turn` REPAIRS the skew, which is only possible because the compare ran ahead of the overwrite. DEAD rather than UNCONSCIOUS, because the engine revives its own unconscious units at every turn boundary (SavedBattleGame::endTurn -> reviveUnconsciousUnits -> abortTurn) and an UNCONSCIOUS skew is undone by the very turn cycle meant to stamp it. PROTOCOL.md updated in the agent-docs repo. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Root cause of the residual "UNIT CENSUS DRIFT after the alien side" the
PRD-P9 soak kept failing on (reproduced locally, ~1 run in 2).
The PRD-P11 receive pump blocks per SUBJECT: a packet whose unit already
has an earlier packet deferred in this pass waits with it. Three packets
are exempt as CHAIN CLOSERS - abortPath, unit_death, after_unit_death -
because each ends the chain holding the receive gate, so making one wait
for a mid-chain packet of the same unit deadlocks the gate.
That exemption keys on `_coopWalkInit` / `_coopInitDeath`, which are
GLOBAL "a replay chain is running here" flags, NOT per unit. So while ANY
unit's walk replay was running, an `abortPath` for a DIFFERENT unit
counted as a closer, skipped the ordering rule and overtook that unit's
own still-queued `BattleScapeMove`. Measured in a failing soak (four
inversions in one five-turn run, zero after this change):
rx abortPath id=1000003 here=25,11,1 -> 25,11,1 tu=6
rx replay walk id=1000003 start=25,11,1 end=16,13,0 tu=6
walk deinit id=1000003 at=25,12,1 tu=2
The teleport-correction lands first; `movePlayerTarget` then puts the unit
back on the packet's START tile and re-paths LOCALLY, and with its closer
already spent nothing corrects the result - so the peer keeps whatever its
own truncated path walked and spent. The host had that alien standing
still on 6 TU; the peer took a step to 25,12,1 on 2 TU, and the side ended
with the two machines a tile and 4 TU apart.
The death pair inverts the same way and is worse: `unit_death` carries the
victim's status AT THE START of the death (STANDING), so applying it after
`after_unit_death` puts a corpse back on its feet on 0 HP - which is the
"host DEAD/UNCONSCIOUS, peer STANDING at 0 HP" half of the same report,
and the fourth member of the family PRD-P10 fixed three of.
Fix, in the P10 shape (receiver-side guard, executor authoritative): a
closer may still jump a MID-chain blocker - that is the exemption the
carve-out was written for and it stays - but it may not jump the OPENER of
the chain it closes. The wire is FIFO, so an opener still queued ahead of
its closer means this machine has not started that chain at all and there
is nothing yet to close; no new deadlock is reachable. Implemented by
recording which packet kind seeded each per-subject block and pairing
closer -> opener (`coopChainOpener`). `unit_death` has no entry: it closes
the SHOT chain, whose opener `ProjectileFlyBState` is keyed on the shooter
while `unit_death` is keyed on the victim, so the subject rule can never
pair them.
Soak on the final build: 5 of 6 clean, including three consecutive
(pre-fix: three attempts, none clean). The one failure was a different
seam - a player unit a z-LEVEL apart mid-player-side, the same
gravity-follows-host-authoritative-terrain difference
test_coop_outcome_gaps already documents and deliberately does not assert;
the soak asserts z mid-turn, where only `next_turn` repairs it. Left
alone: adjacent, pre-existing and not this fix's subject.
PROTOCOL.md updated in the agent-docs repo.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…allout) PR #139 deleted the dead CoopMenu bundle; PrimeGrenadeState.cpp had been getting BattleUnit.h transitively through it and the branch's prime-cost replication (tu/energy on the actor) stopped compiling: C2027 use of undefined type 'OpenXcom::BattleUnit' at the getTimeUnits/getEnergy reads. Include it directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…l turns)
Developer feature request: show who is acting and why input is blocked,
instead of looking like a bug. New STR_COOP_WAIT_FOR_PLAYER_ACTION map
banner (Please wait for {0} action to finish) on its own full-width
centered Text in the map strip above the toolbar (warning color, high
contrast), driven per frame by BattlescapeState::updateCoopWaitBanner():
local isBusy() + the owner seat of the running chain, so it works on
both machines with zero new wire traffic. Own-seat actions suppressed.
Owner resolution latches once per busy window (_coopBusyOwnerSeat) via
BattlescapeGame::getPrimaryBusyActor(), which skips consequence states
(UnitDieBState/UnitFallBState/ExplosionBState pushed to the queue front
mid-chain) so a kill cannot re-attribute the banner to the victim's
seat. Latch drops on idle and outside a live parallel battle.
The busy action_deny no longer flashes the toolbar warning widget - it
arms a 30-frame _coopWaitDenyTicks click-sync window for the banner (the
mirror-gap case); the deny wire shape and _clientLastDenyWarning latch
are unchanged, and all other deny keys keep the toolbar flash. Naming is
seat-indexed (seatName) with a documented 2-player-only
getCurrentClientName() fallback for skirmish battles, where the seat
roster is never populated. _coopWaitDenyTicks resets at battle init.
Harness: battle_state gains coopWaitBanner + lastDenyWarning; new
test_coop_wait_banner.py (5 scenarios: host banner + clear, host own
suppressed, client banner during host replay with owner-latch stability,
client own suppressed); test_parallel_intents busy scenario now asserts
the latched deny key instead of the timing-sensitive flash.
Review evidence: serial build 0 errors; boot_check green; wait-banner
suite x3 consecutive all-pass; intents suite green; Latin-1 encodings
preserved (connectionTCP.cpp 19 non-ASCII + BOM, .h 5, TestServer.cpp 4,
BattlescapeState.cpp 4, BattlescapeGame.cpp 12, no lone LFs); banner
visually verified on both machines via harness screenshots. PROTOCOL.md
UX note landed in the agent-docs repo (67fa8c0) - wire unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Unreleased/Added: the parallel-turns wait banner, per-action desync detection with one-click reporting, the next-launch crash reporter, and first-class mod support (script-RNG lockstep). Unreleased/Fixed: the desync tail the initiative eliminated, summarized for players. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closes the named validation gap: the SHARED economy campaign and parallel battlescape turns (EnableCoopParallelTurns) had never been exercised together by any suite entry or soak. The parallel_* tests all use the SKIRMISH fixture or the SEPARATE-campaign soak; the shared_* tests all run the CLASSIC (alternating-sub-turn) battle. test_shared_parallel_campaign is the one place they meet. Headline finding (asserted, not assumed): parallel turns DO activate in a SHARED battle. A SHARED co-op mission is gamemode 1 (PVE) and the host's EnableCoopParallelTurns propagates across the COOP_READY_HOST handshake, so parallelTurnActive() returns true; battle_state.parallelActive is true on both machines and the PRD-P5 executor invariant holds (activeSync host=true / client=false). If it had NOT activated the test STOPs with the observed gamemode rather than forcing it. Drives the full geoscape -> battle -> debrief -> return cycle: a seeded terror site flown from the shared craft (host SEED, client SEED+1, pinned right before map generation), both seats acting in parallel (host walk + shot executor-local, client walk + shot routed as intents), per-action sync-check clean across all ten promoted ALARM buckets, item census equal and the drift tripwire silent after every action, the client wait banner observed, a voted ABORT scoring an identical debrief on both machines, and a clean return to one identical SHARED world with the replica zero-disk and no desync bundle / crash marker / dump. Deliberate de-flake: actions are driven within the FIRST player side and the mission aborted, so the test never enters the P8 side-close 'Waiting for HostPlayer' handshake that is the campaign soak's documented flake (that turn-cycle path is already covered by test_parallel_soak --profile campaign). Walks retry with re-placement to absorb the interrupt-stalls a dense terror map produces (a stalled walk still runs a complete, sync-compared chain). Seed-pin makes it deterministic; x5 consecutive green. shared_fixture.bring_up/SharedSession gain optional per-instance host_options/ client_options (default None = unchanged) so a SHARED test can bring the campaign up with the host on a feature the client must not set itself. No product code touched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…escape-turns-5ad7b6
The P2 battle drift tripwire (SharedEcon::attachBattleChecksum / verifyBattleChecksum) stamped and compared its chkBattleItemId / chkBattleCensus / chkBattleUnits terms in every co-op mode. In PvP (gamemodes 2/3) the two machines run a role-aware sim that diverges BY DESIGN - per #151's contract PvP sits outside all parallel + I0 sync machinery - so the chkBattleUnits term fired a false "CO-OP DESYNC DETECTED" banner and wrote a diagnostic bundle on both machines on the first next_turn (units peer != local, legitimately). Gate the battle-term machinery OFF for PvP, symmetric on both sides: - attachBattleChecksum: do not stamp chkBattle* in gm2/3. An unstamped peer reads back as the -1 "agree" sentinel in verifyBattleChecksum, so mixed old/new-version sessions stay compatible. - verifyBattleChecksum: do not compare or run captureDesyncReport in gm2/3 (belt-and-braces on the receive side). Scoped strictly to the battle terms. The SHARED-economy world checksum (chkFunds/chkBases/... in attachWorldChecksum) is a different mechanism that PvP campaigns still use, and is left untouched. The I0 per-action sync-check capture is already inert in PvP and needs no gate: its client emitters and host ring recorder gate on parallelTurnActive() (gamemodes 1/4 only), and syncCheckCompare returns early on a missing "h" map. Found via origin/main's new PvP test test_skirmish_debrief_disconnect (#162), which triggers the false bundle. This fix removes the bundle but does NOT by itself make that test pass - its disconnect-notice timeout has a separate cause addressed in a follow-up commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…not clean exits (reconciles 056b500 with #162) The post-merge conflict: #162 (test_skirmish_debrief_disconnect) wants the host to raise a "<player> has left the server" notice when the peer DROPS while the host is reading a skirmish debriefing; 056b500 (test_skirmish_end_main_menu) forbids that notice when the peer LEAVES CLEANLY (pressed OK on its own debrief) while the other player is still on theirs. At the notice gate the two are indistinguishable - a clean OK-exit and an abrupt crash both arrive as an identical socket close (SDLNet_TCP_Recv <= 0); GoToMainMenuState and disconnectTCP send nothing, so there was no signal to tell "drop" from "clean exit". 056b500's blanket skirmishMissionOver() suppression fixed the clean case by silencing BOTH. Make it decidable by having the LEAVER announce a clean departure: - Wire: additive `{"state":"coop_leaving"}`, sent from DebriefingState::btnOkClick on the skirmish OK-exit (monthsPassed == -1, getCoopStatic()) BEFORE the teardown closes the socket - the one clean chokepoint for that exit (GoToMainMenuState carries no coop code). Queued ahead of the teardown; TCP in-order delivery lands it before the FIN. Whitelisted in the parallel receive gate so it is never parked. - Receive: the handler latches connectionTCP::_peerLeftCleanly. Reset at every disconnectTCP teardown (after the gate has read it) AND on a fresh client attach, so a clean leave in one session can never suppress a real crash notice in the next. - Gate (onConnect == -2): allow_cutscene && !campaignEnded() && (!skirmishMissionOver() || (debriefOpen() && !_peerLeftCleanly)). The new debriefOpen() helper (monthsPassed == -1 + a DebriefingState on the stack) is narrower than skirmishMissionOver(), which stays true through the post-OK menu transition too - so a drop while results are on screen shows the notice, a peer that left after this machine already dismissed its debrief stays silent (056b500's transition window). Old-peer compat: an old leaver sends no coop_leaving, so a new host shows the notice even on its clean exit - acceptable version-mixing degradation (documented in PROTOCOL.md), never a false SUPPRESSION of a real drop. Both regression tests pass unmodified: test_skirmish_debrief_disconnect (#162, abrupt -> notice) and test_skirmish_end_main_menu (056b500, clean -> silent, single dismiss reaches the menu). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
NonPolynomialTim
enabled auto-merge (squash)
August 17, 2026 05:56
NonPolynomialTim
disabled auto-merge
August 17, 2026 06:02
NonPolynomialTim
marked this pull request as draft
August 17, 2026 06:03
Collaborator
Author
|
Last thing to fix before merging is the latest comment from Ari on the authoritative issue, which I'll do tomorrow |
This was
linked to
issues
Aug 17, 2026
Collaborator
Author
|
Leaving a note for myself that I should also go through the issue list and validate that all existing battlescape bugs/desyncs are fixed with the new architecture and link them all so they get closed when this merges |
Collaborator
I've been looking through the code, and I can really see how much work you've put into it. It's clear you've been working on the parallel turns feature every day. Aside from that one bug, I didn't notice any desyncs or other issues. Are you also planning to release it tomorrow? :) |
…ement
The per-shard 25-minute CI wall-clock timeout is the wrong bound - the suite
only grows, and in run 31999655291 it spuriously killed 2 of 4 shards ("has
timed out after 25 minutes" on shards 2 and 4, which carried test_coop_debrief_sync
at 529.8s and test_parallel_soak at 305.8s). Replace the shard wall with per-TEST
budget enforcement so the suite can grow while a single pathological test is still
bounded.
What changed:
- New tools/coop_test/slow_test_exceptions.json: single source of truth for
budgets. default 180s/test; hard_kill_multiplier 2.0; max_budget_s 900 (hard cap,
no unlimited entries - both runners error on load if an exception exceeds it).
7 seeded exceptions with observed CI durations in each "reason".
- tools/ci/run_coop_suite.ps1 (the CI shard runner) and tools/coop_test/run_parallel.py
(the local K-lane runner) both read that JSON and enforce it:
* a test that FINISHES over budget FAILS the suite even if it passed
("BUDGET EXCEEDED: <t> took Xs > Bs budget - re-engineer the test or add a
justified exception");
* a test still running at 2x its budget is hard-killed with its game subtree and
failed ("BUDGET HARD-KILL ... killed as a hung test"), so a hang can never
wedge a shard now that the wall is gone;
* real failures still retry once (flake tolerance); hangs are NOT retried, so the
worst case stays bounded.
run_coop_suite.ps1 previously had no per-test timeout at all - the 25-min step wall
was its only hang bound - so this adds a Start-Process/WaitForExit(ms)/taskkill-tree
ceiling there.
- .github/workflows/ci-main.yml and ci-validate.yml: removed the 25-min step timeout
on the "Coop test suite" step; raised the coop-shard job timeout 30 -> 60 as a
coarse runaway backstop only (per-test budgets are the primary bound). Leaving the
30-min job cap would have re-capped the shard and defeated the removal.
pull_request_target note (delayed effect): the PR merge gate is ci-validate.yml,
which runs on pull_request_target - GitHub always takes that workflow from the BASE
branch (main), so this branch's edit to ci-validate.yml does NOT change its own PR
gate; it takes effect only for PRs opened after this merges to main. ci-main.yml's
coop-shard runs only on push to main, so its edit likewise takes effect on the next
push to main (post-merge). The runner scripts + JSON, by contrast, are checked out
from the PR head and take effect immediately.
Validated with fake sleeping tests (no game launched, machine reserved for a timing
session): pass-under-budget, budget-exceeded-but-completed, hang-hard-killed-at-2x
(killed at 2s not 30s), real-failure-retried, and over-cap rejection - green in both
run_parallel.py (22/22 unit checks) and run_coop_suite.ps1 (end-to-end against a
scratch tree). No product C++ touched; Python/PowerShell/YAML only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… stops overloading its shard Root cause of the shard-2 25-min wall kill in run 31999655291: the heaviest suite entry was absent from test_weights.json, so plan_shards.ps1 weighted it at the median (~28s) and packed ~40 tests into its shard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
test_parallel_soak's strict cross-machine item census (assert_census) could sample while the client was mid-drain on a boundary death: the host had already minted the corpse and spilled its inventory to the floor while the client was still one death behind (corpse replay in flight, or the neutral->player advance deferred). CI signature: "ITEM CENSUS DRIFT ... strict id census differs", host items on STR_GROUND + corpse vs client items still in BELT/RIGHT_HAND. settle_display waited only on displayBacklog/taskCompleted/rxHold/isBusy - isBusy is the EXECUTOR's animation flag and does not cover the peer's corpse-replication windows. The product's own sync-check already gates on exactly this window (SharedEcon.cpp:5817: corpseReplayPendingAny() || corpseRemapPendingAny() || _coopInitDeath). Changes: - TestServer.cpp: expose that same predicate as an additive `corpsePending` bool on the EXISTING battle_state block (reuses the _coopInitDeath source the block already reads). Read-only; no compare/tripwire semantics change. - test_parallel_soak.py: extend settle_display to also wait, on BOTH machines, for corpsePending==False AND turnAdvanceDeferred==0 before the census. Bounded; on timeout it PROCEEDS to the census (poll result ignored) - a persistent divergence drains the predicate and stays different, so a real drift still fires; only the transient straddle is waited out, never masked. Detection unchanged: sync-check / tripwire compare semantics untouched. Validated: build clean (serial, MP overrides) + boot_check; repro_boundary_death (corpse-mint window logic holds with the field present); test_parallel_soak --profile baseline (seeds 20260819/22/23/24 real + 17/18/19 faithful-instrumented) and --profile speed-skew, zero census failures; test_sync_check (strict), test_battle_tripwire (PVE red-capability intact), test_shared_battle, test_shared_parallel_campaign all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Eliminate the fixed-port model in the coop test harness; both socket kinds
now bind OS-assigned ephemeral ports, killing the collision/linger class (CI
31999655291 shard 1: test_pvp_duplicate_bases "test server not reachable on
:49900" was a prior scenario's instance still holding the fixed port).
TestServer (test-only):
- probeEphemeralPort() binds loopback port 0, reads it back via getsockname,
releases it, so the game reports the actual port instead of the harness
guessing. Winsock guarded with NOMINMAX/WIN32_LEAN_AND_MEAN, ws2_32 linked
via pragma; Windows-only, POSIX uses plain BSD sockets.
- Control socket: OXC_TEST_PORT=0 -> probe + bind, then write the actual port to
<userfolder>/testserver_port.txt (stale file removed before bind; atomic
temp+rename). The listening log line is the fallback report.
- host_tcp / host_udp / host_menu_host: port "0" -> probe the right socket kind,
return the actual bound port in the EXISTING response (resp["port"]).
- join_udp: localport "0" -> ephemeral client bind, reported as resp["localport"].
Only the 5 existing port-consuming commands were touched - no new command
branch (the Session-F dispatcher trap).
Harness:
- GameClient spawns every instance with OXC_TEST_PORT=0 and reads the reported
control port from the port file in connect() (the 60s budget is pure boot
budget now, no port-guessing). The positional port is an inert label;
user_dir=None keeps the fixed path for repro tooling that attaches to a
running instance.
- cmd() bridges the coop port transparently: a host_* command is rewritten to
ask for "0" and the returned port is stashed under the original literal, which
the paired join_* reuses as an in-process rendezvous key. The whole suite
migrates with no per-test edits - the literals are keys, not bound ports.
- Removed the dead lane-shift machinery (_PORT_SHIFT_CMDS, PORT_BLOCK); kept the
per-slot machine lock and s{slot}_ user-dir prefixes (still shared state).
- run_parallel: K is no longer capped by port bands (now a CPU/RAM guard).
Validated: build 0 err + boot_check; test_pvp_duplicate_bases x5; test_shared_battle,
test_parallel_intents, test_shared_refresh, test_shared_parallel_campaign,
test_udp_bringup, test_skirmish_end_main_menu, test_sync_check; a K=4 run of 12
light tests (12/12, 2.66x, no port bands). The game still honours a fixed
OXC_TEST_PORT for manual debugging.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Since 5e5ce50 the harness rewrites a coop host_tcp port literal to an OS-assigned ephemeral one, and LobbyMenu::waitingText() renders the REAL bound port. The Bug 5 predicate still required the stale literal "47903" in detailsText, so it could never match -> deterministic 15s TimeoutError. Capture the host_tcp response (resp["port"] carries the actual bound port, public and self-contained) and assert that instead of the literal; the "ClientPlayer" half is unchanged. Swept the suite for sibling port-literal text/predicate matches: none (test_shared_lobby_details.py only checks for the presence of "on port"; PORT constants elsewhere are inert rendezvous keys). test_session_hardening green 3x; test_shared_refresh unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
NonPolynomialTim
added a commit
that referenced
this pull request
Aug 17, 2026
…ement (#170) The per-shard 25-minute CI wall-clock timeout is the wrong bound - the suite only grows, and in run 31999655291 it spuriously killed 2 of 4 shards ("has timed out after 25 minutes" on shards 2 and 4, which carried test_coop_debrief_sync at 529.8s and test_parallel_soak at 305.8s). Replace the shard wall with per-TEST budget enforcement so the suite can grow while a single pathological test is still bounded. What changed: - New tools/coop_test/slow_test_exceptions.json: single source of truth for budgets. default 180s/test; hard_kill_multiplier 2.0; max_budget_s 900 (hard cap, no unlimited entries - both runners error on load if an exception exceeds it). 7 seeded exceptions with observed CI durations in each "reason". - tools/ci/run_coop_suite.ps1 (the CI shard runner) and tools/coop_test/run_parallel.py (the local K-lane runner) both read that JSON and enforce it: * a test that FINISHES over budget FAILS the suite even if it passed ("BUDGET EXCEEDED: <t> took Xs > Bs budget - re-engineer the test or add a justified exception"); * a test still running at 2x its budget is hard-killed with its game subtree and failed ("BUDGET HARD-KILL ... killed as a hung test"), so a hang can never wedge a shard now that the wall is gone; * real failures still retry once (flake tolerance); hangs are NOT retried, so the worst case stays bounded. run_coop_suite.ps1 previously had no per-test timeout at all - the 25-min step wall was its only hang bound - so this adds a Start-Process/WaitForExit(ms)/taskkill-tree ceiling there. - .github/workflows/ci-main.yml and ci-validate.yml: removed the 25-min step timeout on the "Coop test suite" step; raised the coop-shard job timeout 30 -> 60 as a coarse runaway backstop only (per-test budgets are the primary bound). Leaving the 30-min job cap would have re-capped the shard and defeated the removal. pull_request_target note (delayed effect): the PR merge gate is ci-validate.yml, which runs on pull_request_target - GitHub always takes that workflow from the BASE branch (main), so this branch's edit to ci-validate.yml does NOT change its own PR gate; it takes effect only for PRs opened after this merges to main. ci-main.yml's coop-shard runs only on push to main, so its edit likewise takes effect on the next push to main (post-merge). The runner scripts + JSON, by contrast, are checked out from the PR head and take effect immediately. Validated with fake sleeping tests (no game launched, machine reserved for a timing session): pass-under-budget, budget-exceeded-but-completed, hang-hard-killed-at-2x (killed at 2s not 30s), real-failure-retried, and over-cap rejection - green in both run_parallel.py (22/22 unit checks) and run_coop_suite.ps1 (end-to-end against a scratch tree). No product C++ touched; Python/PowerShell/YAML only. Cherry-picked from PR #166 (commit 835be08) so this CI change can land on main independently while PR #166 is stabilized: per the pull_request_target note above, the ci-validate.yml wall removal only takes effect for the merge gate once it is on the base branch (main). The PR #166 hunk to tools/coop_test/run_parallel.py (a branch-only local K-lane runner that does not exist on main and is not run by the CI coop-shard gate) is intentionally omitted here; the four CI-effective files -- both workflows, run_coop_suite.ps1, and the new slow_test_exceptions.json -- are applied verbatim from 835be08. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…escape-turns-5ad7b6
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ses tuMax Additive harness-introspection fields; existing consumers unaffected. - probe_step (battle_intent): each vouched candidate now carries `pathLen` (Pathfinding::calculate step count from the unit's current tile) and `floorSafe` (dest tile has a floor under it / no z-fall). A Chebyshev- adjacent tile whose DIRECT step is terrain-blocked resolves to a 6-29 step detour; a TU-clamped unit then stops partway or falls a z-level, which the walk/race position asserts misread as a double execution. pathLen==1 && floorSafe selects the clean single steps those asserts assume. - battle_state units: `tuMax` (getBaseStats()->tu). setTimeUnits clamps to _stats.tu (~66 for a rookie), so the harness's literal top-up of 200 silently became the clamp; exposing the ceiling lets top_up() be honest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…el intents
Fixes the two CI flake signatures in test_parallel_intents, both proven to be
test-fixture artifacts (not product bugs) by the prior instrumented run:
(1) scenario_race "the action ran more than once": the step-picker vouched a
Chebyshev-adjacent dest whose direct step is terrain-blocked, so
Pathfinding produced a legitimate 6-29 step detour; the TU-clamped unit
stopped partway / fell a z-level and `ph in (start, dest)` misread it.
(2) scenario_walk "client never displayed its OWN action": `landed = pos(host)`
captured an INTERMEDIATE detour tile one frame into the walk, then waited
for the client to match that stale snapshot forever.
- free_step_both / step_dest filter to pathLen==1 AND floor-safe on BOTH
machines (new single_steps_of / single_common), so every walk dest is a
clean single step. steps_of/common_steps stay raw - test_parallel_skip and
test_parallel_soak use them to find the FURTHEST reachable tile on purpose.
- scenario_walk: drop the mid-walk snapshot; settle both machines, then assert
settled-final host==client AND final != start.
- scenario_race: keep the no-drift / `ph in (start,dest)` asserts (valid again
with single-step dests) + an explicit action_seq-advance admission assert.
- top_up reads the real tuMax instead of the literal 200 (honest contract).
- scenario_busy 2b: assert the retry is ADMITTED (action_seq advances), not
that the unit physically moves - a walk admitted next to a live hostile can
be reaction-interrupted at its start tile (the case scenario 6 already
tolerates). Latent flake, unmasked once (2) stopped the run from usually
dying at scenario 1 before it reached the retry.
seed 1234: race misread reproduced deterministically pre-fix (iteration 6
every run), FAILING_ITERATIONS=[] post-fix. test_parallel_intents x5 green
(159-170s, budget 360s). Regressions green: endturn, soak baseline,
sync_check, shared_battle (classic - no delta), shared_parallel_campaign.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…172) flakes Both run non-gating until de-flaked; tracked in their issues. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…on slow window) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
2 participants
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.
Closes #152.
Closes #40.
Summary
Parallel battlescape turns: both players act during the same X-Com side (host-authoritative simulation, intent forwarding, thin-client display), plus the full per-action desync-detection net that proves it safe, one-click desync reporting, a next-launch crash reporter, and first-class mod support.
Player-facing highlights (full list in CHANGELOG Unreleased):
Validation
Test plan
🤖 Generated with Claude Code