#45 scoreboard: reclaim the binary's DEATHS field, add a heartbeat, sweep the false KB claims
Follow-up to 4fa7eee, driven by an adversarial review pass and three rig cycles.
Each item below is a real defect that pass found, not polish.
DEATHS now uses the binary's own column. PilotList::Execute @0x4cabd0 draws
`fild [edi+0x27c]` (KILLS) and `fild [edi+0x280]` (DEATHS); +0x280 has exactly
two runtime writers image-wide plus the ctor zero. Our port had declared it
`pad_0x280`, never written, and displayed the ENGINE's Player::deathCount
(+0x200) instead -- which is the respawn-handshake identity, seeded -2, and is
why it needed a clamp to pass as a count. It is now BTPlayer::deathTally (our
offset 0x274, offsetof-locked), incremented beside ++deathCount, read by the
gauge, and replicated. deathCount is left to the engine's handshake. So this
half moves TOWARD the binary; it also closes the plan's Headline-1.
(`deathTally`, NOT `deaths`/`deathCount` -- those would shadow the base.)
The mirror was not self-healing: update records are UNRELIABLE by construction
(Entity::UpdateMessage clears ReliableFlag, ENTITY3.h:112; the relay's UDP path
also drops stale/reordered datagrams), and the pair was dirtied only on an
event. One lost datagram left every peer stale until that pilot's next kill or
death -- forever for the last kill of a round. Added a 2s heartbeat that
re-dirties the record, which also bounds how long the binary's phantom
partner-increment stays visible. The timer is a function static deliberately: a
data member would change sizeof(BTPlayer) and break the offset locks.
Also fixed: the SBMIRROR row AND its change-detect both still read deathCount (a
constant -2 here), so the "log only the edge" guard could never be false and
every row printed deaths=-2. That is what made the first rig runs look like
DEATHS was broken when a WRITE/READ trace proved the transport correct. Row
count per node fell from ~20 to 3, one per real change.
Guarded the record against per-bit layouts: update_model is a BIT INDEX and
Entity::WriteUpdateRecord switches on it (ENTITY.cpp:329-352) -- the DamageZone
bit emits a variable-length packed stream whose length it computes itself, so
appending two ints and re-stamping recordLength over that would corrupt it.
Deleted BTPlayerCountObservedDeath -- definition, call site, extern and friend
together, since /FORCE hides stragglers. It never executed (its call site sat
inside the once-per-death transition, which a replicant never enters) and could
not have worked (0 of 8800 corpus DMG rows target a replicant, which is why every
DEATH inst=R row reads killer=0:0). Under replication it would have been a
second writer of a replicated counter.
New forensics: NOCREDIT names the failing link when a kill credit is skipped (it
used to be completely silent -- the counter simply never moved), and PLAYER_LINK
records whether the one-shot link resolved. Both retire the NULL-playerLink
theory: every rig shows `PLAYER_LINK inst=R resolved=1` and no NOCREDIT rows.
PLAYER_DEAD now logs both counters (deaths=handshake, tally=scoreboard).
KB sweep of the claims that hid this bug for so long:
* context/gauges-hud.md's "RESOLVED -- deaths tally per node from locally
observed events" was FALSE; corrected with the measured evidence.
* docs/GAUGE_COMPOSITE.md + btl4gau3.cpp "the dead pad_0x280" -- never dead,
merely unwritten.
* btplayer.hpp attributed VehicleDeadMessageHandler to @004c012c; it is
@004c05c4 (absent from the decomp export -- the #60 gap).
* docs/RESPAWN_REARM_PLAN.md's "#45 SUBSUMED" -- the PLAYER_DEAD symptom was
subsumed, the scoreboard defect was not.
* The corpus figure in the record banner now states its method so it is
reproducible.
Rig-verified (2-node loopback, several cycles): owner 3:1 kills 2/tally 1 read
`kills=2 deaths=1` on the peer; owner 2:1 kills 1/tally 2 read `kills=1 deaths=2`
on the peer. Respawn unaffected, no crash, no GLITCH rows. Still awaiting live
multi-pod verification by a human; all pods must run the same build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
4fa7eee54f
commit
a52207d779
+143
-45
@@ -231,6 +231,10 @@ void BTPlayerLayoutSelfCheck() // never called -- a compile-time lock only
|
||||
static_assert(offsetof(BTPlayer, killCount) == 0x270,
|
||||
"BTPlayer::killCount moved -- the scoreboard bridges (BTPilotKills) and the "
|
||||
"binary's 0x27c-era offsets must be re-checked");
|
||||
static_assert(offsetof(BTPlayer, deathTally) == 0x274,
|
||||
"BTPlayer::deathTally moved -- it is the binary's DEATHS column (@0x280) that "
|
||||
"Gitea #45 put back into service; re-check GetDeaths/BTPilotDeaths and the "
|
||||
"update-record mirror");
|
||||
static_assert(sizeof(BTPlayer) == 0x28c,
|
||||
"BTPlayer size changed -- re-measure the offsets above before trusting any "
|
||||
"raw-offset code that touches this class");
|
||||
@@ -254,6 +258,9 @@ void BTPlayerLayoutSelfCheck() // never called -- a compile-time lock only
|
||||
|
||||
static const char *SelfDestructName = "self destruct"; // &DAT_00524b38
|
||||
static const Scalar RespawnDelay = 5.0f; // death -> drop-zone hunt (@004c0830)
|
||||
// #45: how often a master re-dirties its update record so the replicated
|
||||
// KILLS/DEATHS pair reconverges after a dropped (unreliable) update datagram.
|
||||
static const Scalar ScoreboardHeartbeat = 2.0f;
|
||||
static const Scalar TicksPerSecond = 1.0f; // (see note in PlayerSimulation)
|
||||
|
||||
//
|
||||
@@ -485,11 +492,24 @@ void
|
||||
++deathCount; // this+0x200
|
||||
message->deathCount = deathCount; // message+0x38
|
||||
|
||||
//
|
||||
// #45: the SCOREBOARD deaths tally is the binary's own +0x280, not the engine's
|
||||
// deathCount above. deathCount stays what the engine made it -- the respawn
|
||||
// handshake identity (seeded -2, matched at PLAYER.cpp:230) -- so it must not be
|
||||
// the number we display or replicate. This is the one the PilotList draws and
|
||||
// the one that rides the update record to every other pod.
|
||||
//
|
||||
++deathTally; // this+0x280
|
||||
|
||||
// MP MATCH FORENSICS (matchlog.hpp): the owning player's death
|
||||
// notification (fires exactly once per death, deduped above).
|
||||
if (BTMatchLogActive())
|
||||
BTMatchLog("PLAYER_DEAD", "player=%d:%d deaths=%d",
|
||||
BTMatchHostOf(GetEntityID()), (int)GetEntityID(), deathCount);
|
||||
// #45: log BOTH counters. `deaths` is the engine's respawn-handshake
|
||||
// deathCount (seeded -2); `tally` is the binary's +0x280 scoreboard column
|
||||
// that the gauge draws and the update record replicates. Having them side
|
||||
// by side is what makes a divergence between them visible at all.
|
||||
BTMatchLog("PLAYER_DEAD", "player=%d:%d deaths=%d tally=%d",
|
||||
BTMatchHostOf(GetEntityID()), (int)GetEntityID(), deathCount, deathTally);
|
||||
// TODO(bring-up): scenarioRole is the scoring role looked up from the mission's
|
||||
// role registry (btplayer.cpp:815, currently deferred), so it is NULL for the
|
||||
// minimal dev missions. Only debit a life when a role is present.
|
||||
@@ -919,6 +939,36 @@ void
|
||||
|
||||
Player::PlayerSimulation(time_slice); // FUN_0042e168
|
||||
|
||||
//
|
||||
// SCOREBOARD HEARTBEAT (#45). The KILLS/DEATHS mirror rides the update record,
|
||||
// and update records are UNRELIABLE BY CONSTRUCTION: `Entity::UpdateMessage`
|
||||
// clears `ReliableFlag` (ENTITY3.h:112) and the relay's UDP path additionally
|
||||
// drops stale/reordered datagrams behind a per-sender sequence gate. The pair is
|
||||
// only dirtied on a score/death EVENT, so one lost datagram would leave every
|
||||
// peer's copy of this pilot's columns stale until their NEXT kill or death --
|
||||
// which, for the last kill of a round, is forever. Re-dirty the record at a low
|
||||
// fixed rate so the pair reconverges within a beat. This is also what bounds the
|
||||
// visibility of the binary's phantom partner increment (the wrong-column slip we
|
||||
// still reproduce faithfully): the owner's authoritative value overwrites it
|
||||
// within one beat instead of whenever the pilot next scores.
|
||||
//
|
||||
// Cheap: a Player has no 60Hz stream of its own -- it emits a record only when
|
||||
// dirtied -- and the record is 68 bytes. The timer is a function static ON
|
||||
// PURPOSE: a new data member would change `sizeof(BTPlayer)` and break the offset
|
||||
// locks. A node pilots exactly one master player, so a shared timer is exact
|
||||
// here; with several it would simply beat in turn.
|
||||
//
|
||||
if (GetInstance() != Entity::ReplicantInstance)
|
||||
{
|
||||
static Scalar s_scoreboard_beat = 0.0f;
|
||||
s_scoreboard_beat += time_slice;
|
||||
if (s_scoreboard_beat >= ScoreboardHeartbeat)
|
||||
{
|
||||
s_scoreboard_beat = 0.0f;
|
||||
ForceUpdate(); // updateModel |= DefaultUpdateModelFlag
|
||||
}
|
||||
}
|
||||
|
||||
if (
|
||||
(lastPerformance - lastConsoleUpdate) / TicksPerSecond >= CONSOLE_UPDATE_INTERVAL // _DAT_004c08fc
|
||||
|| application->GetApplicationState() == Application::EndingMission // app+0x88 == 6
|
||||
@@ -1452,7 +1502,7 @@ BTPlayer::BTPlayer(
|
||||
DEBUG_STREAM << "[layout] sizeof(BTPlayer)=" << (int)sizeof(BTPlayer)
|
||||
<< " (0x" << std::hex << (int)sizeof(BTPlayer) << std::dec << ")"
|
||||
<< " killCount@0x" << std::hex << (int)offsetof(BTPlayer, killCount)
|
||||
<< " pad_0x280@0x" << (int)offsetof(BTPlayer, pad_0x280)
|
||||
<< " deathTally@0x" << (int)offsetof(BTPlayer, deathTally)
|
||||
<< " objectiveMech@0x" << (int)offsetof(BTPlayer, objectiveMech)
|
||||
<< " deathPending@0x" << (int)offsetof(BTPlayer, deathPending)
|
||||
<< std::dec << " <-- the raw write targets byte 0x284\n"
|
||||
@@ -1581,7 +1631,7 @@ BTPlayer::BTPlayer(
|
||||
}
|
||||
|
||||
killCount = 0; // this[0x9f]
|
||||
pad_0x280 = 0; // this[0xa0]
|
||||
deathTally = 0; // this[0xa0] (#45: the binary's DEATHS column @0x280)
|
||||
objectiveMech = 0; // this[0xa1]
|
||||
lastPerformance = lastConsoleUpdate = lastUpdate; // this[0xa2],[0xa3] = this[4]
|
||||
|
||||
@@ -1654,6 +1704,10 @@ int BTPilotDeaths(void *pilot)
|
||||
{
|
||||
if (pilot == 0)
|
||||
return 0;
|
||||
// #45: GetDeaths() is now the binary's own +0x280 tally, which starts at 0 and
|
||||
// only ever counts up, so the clamp no longer has a -2 engine seed to hide (it
|
||||
// used to read Player::deathCount, the respawn handshake identity). Kept as a
|
||||
// cheap floor in case a record ever arrives before the owner's first write.
|
||||
int deaths = ((BTPlayer *)pilot)->GetDeaths();
|
||||
return (deaths < 0) ? 0 : deaths;
|
||||
}
|
||||
@@ -1675,7 +1729,7 @@ int BTPilotDeaths(void *pilot)
|
||||
// for OUR compiled object (printed by the ctor, 2026-07-25):
|
||||
//
|
||||
// sizeof(BTPlayer) = 652 (0x28c)
|
||||
// killCount @ 0x270 pad_0x280 @ 0x274
|
||||
// killCount @ 0x270 deathTally @ 0x274
|
||||
// objectiveMech @ 0x278 deathPending @ 0x284 <-- what 0x284 really is
|
||||
//
|
||||
// So every target designation wrote a Mech pointer into **`deathPending`**, the
|
||||
@@ -1764,27 +1818,14 @@ BitMap *BTPilotNameBitmap(void *pilot)
|
||||
return mission->GetSmallNameBitmap(index);
|
||||
}
|
||||
|
||||
// OBSERVED-DEATH tally (MP DEATHS fix): each node maintains every player's
|
||||
// score from LOCALLY OBSERVED events -- exactly how the KILLS credit already
|
||||
// works (the victim's ScoreMessage handler ++s the SHOOTER's local Player
|
||||
// copy). This is the symmetric half: when a REPLICANT mech dies here (the
|
||||
// once-per-death transition runs on every node), tally the death onto its
|
||||
// owning player's LOCAL copy. The owner's own node counts through its real
|
||||
// VehicleDead(-1) path, so the caller gates this to replicant mechs -- each
|
||||
// node then counts every pilot self-consistently. Normalize the -2/-1
|
||||
// pre-acquire seed first so death #1 reads 1.
|
||||
void BTPlayerCountObservedDeath(void *player)
|
||||
{
|
||||
if (player == 0)
|
||||
return;
|
||||
BTPlayer *p = (BTPlayer *)player;
|
||||
if (p->deathCount < 0)
|
||||
p->deathCount = 0;
|
||||
++p->deathCount;
|
||||
if (getenv("BT_SCORE_LOG"))
|
||||
DEBUG_STREAM << "[score] observed death -> pilot " << player
|
||||
<< " deaths=" << p->deathCount << "\n" << std::flush;
|
||||
}
|
||||
// OBSERVED-DEATH tally: DELETED 2026-07-25 (Gitea #45) together with its call site
|
||||
// (mech4.cpp) and its `friend` (btplayer.hpp) -- all three at once, because /FORCE
|
||||
// would otherwise hide a straggler. It never executed (its call site sat inside the
|
||||
// once-per-death TRANSITION, which a replicant never enters) and it could not have
|
||||
// worked (a replicant victim carries no attribution: 0 of 8800 corpus DMG rows target
|
||||
// a replicant). DEATHS is now replicated from the owning pod instead. Do not
|
||||
// resurrect it: under replication it would be a SECOND writer of a replicated counter,
|
||||
// and it incremented `Player::deathCount`, which the scoreboard no longer even reads.
|
||||
|
||||
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
// WriteUpdateRecord / ReadUpdateRecord (Gitea #45)
|
||||
@@ -1813,10 +1854,27 @@ void
|
||||
{
|
||||
Player::WriteUpdateRecord(message, update_model);
|
||||
|
||||
//
|
||||
// Append the counters ONLY to the default-model record. `update_model` is a
|
||||
// BIT INDEX, and Entity::WriteUpdateRecord switches on it (ENTITY.cpp:329-352):
|
||||
// the DamageZone bit emits a variable-length packed stream of DamageZone
|
||||
// records whose length it computes itself, so appending two ints and
|
||||
// re-stamping recordLength over THAT would corrupt the record. A pilot only
|
||||
// ever sets the default bit today (plain `ForceUpdate()`), which is why the
|
||||
// engine's own Player::WriteUpdateRecord gets away with being bit-agnostic --
|
||||
// but a future `ForceUpdate(mask)` on a player would make this silently wrong,
|
||||
// and skipping here degrades safely: the reader's recordLength guard simply
|
||||
// leaves the counters alone.
|
||||
//
|
||||
if (update_model != Simulation::DefaultUpdateModelBit)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
UpdateRecord *rec = (UpdateRecord *)message;
|
||||
rec->recordLength = sizeof(UpdateRecord);
|
||||
rec->killTally = killCount;
|
||||
rec->deathTally = deathCount;
|
||||
rec->killTally = killCount; // @0x27c
|
||||
rec->deathTally = deathTally; // @0x280 -- NOT the engine's deathCount
|
||||
}
|
||||
|
||||
void
|
||||
@@ -1830,34 +1888,47 @@ void
|
||||
// MIXED-VERSION GUARD: a peer on a build without this extension writes a
|
||||
// plain Player-sized record, and reading the two fields off the end would
|
||||
// pick up whatever record was packed after it in the update stream and
|
||||
// display it as a kill count. Trust the length the writer stamped.
|
||||
// display it as a kill count. Trust the length the writer stamped. (The
|
||||
// stream itself stays in sync either way -- both sides advance by the
|
||||
// WRITER's recordLength, ENTITY.cpp:390 / SIMULATE.cpp:323.)
|
||||
//
|
||||
if (rec->recordLength < sizeof(UpdateRecord))
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
//
|
||||
// Only ever mirror onto a REPLICANT. The owning master holds the sole
|
||||
// authoritative copy of both counters -- it is the only node the engine ever
|
||||
// increments them on -- so a record must never be allowed to write back over
|
||||
// it. ReadUpdateRecord is a replicant-side path today, but this is the
|
||||
// invariant the whole design rests on, so state it rather than assume it.
|
||||
//
|
||||
if (GetInstance() != Entity::ReplicantInstance)
|
||||
{
|
||||
return;
|
||||
}
|
||||
int was_kills = killCount;
|
||||
int was_deaths = deathCount;
|
||||
int was_deaths = deathTally;
|
||||
|
||||
killCount = rec->killTally;
|
||||
deathCount = rec->deathTally;
|
||||
deathTally = rec->deathTally;
|
||||
|
||||
//
|
||||
// Log only the EDGE -- this runs at update rate, so an unconditional line
|
||||
// would drown the log (and the matchlog) in per-frame noise.
|
||||
//
|
||||
if (killCount != was_kills || deathCount != was_deaths)
|
||||
if (killCount != was_kills || deathTally != was_deaths)
|
||||
{
|
||||
if (BTMatchLogActive())
|
||||
BTMatchLog("SBMIRROR",
|
||||
"player=%d:%d kills=%d deaths=%d wasKills=%d wasDeaths=%d",
|
||||
BTMatchHostOf(GetEntityID()), (int)GetEntityID(),
|
||||
killCount, deathCount, was_kills, was_deaths);
|
||||
killCount, deathTally, was_kills, was_deaths);
|
||||
if (getenv("BT_SCORE_LOG"))
|
||||
DEBUG_STREAM << "[score] scoreboard mirror: pilot "
|
||||
<< (int)GetEntityID() << " kills " << was_kills << "->"
|
||||
<< killCount << " deaths " << was_deaths << "->" << deathCount
|
||||
<< killCount << " deaths " << was_deaths << "->" << deathTally
|
||||
<< "\n" << std::flush;
|
||||
}
|
||||
}
|
||||
@@ -1960,17 +2031,21 @@ void BTPostKillScore(Entity *victim, Scalar damage) // Step 7: KILL (+ MP deat
|
||||
{
|
||||
return;
|
||||
}
|
||||
// OBSERVED-KILL credit (MP KILLS fix, 2026-07-13; the symmetric twin of
|
||||
// BTPlayerCountObservedDeath): every node maintains LOCAL score copies,
|
||||
// so the kill must land on the KILLER's player-link object -- the same
|
||||
// object the PilotList gauge reads (the roster resolves pilots via the
|
||||
// mechs' GetPlayerLink()). The old GetMissionPlayer() dispatch credited
|
||||
// a copy the scoreboard never displays (kills stuck at 0 while deaths --
|
||||
// which already flow through the player link -- counted; user-reported).
|
||||
// Resolving the killer from the victim's lastInflictingID also credits
|
||||
// the REMOTE player's local copy when THEY score ("target's kills
|
||||
// display 0") -- each node witnesses the death transition and tallies
|
||||
// its own copies self-consistently.
|
||||
// KILL credit. Runs on the VICTIM's node -- the only node whose mech carries a
|
||||
// populated `lastInflictingID` (damage is applied master-side only: 0 of 8800
|
||||
// corpus DMG rows target a replicant, which is why every `DEATH inst=R` row reads
|
||||
// `killer=0:0`). The credit is dispatched to the killer's Player, which HERE is a
|
||||
// REPLICANT, so `Entity::Dispatch` reroutes it to the owning host
|
||||
// (ENTITY.cpp:244-251) and `++killCount` lands on the killer's OWN machine.
|
||||
//
|
||||
// CORRECTED 2026-07-25 (Gitea #45). The previous banner claimed "every node
|
||||
// maintains LOCAL score copies ... each node witnesses the death transition and
|
||||
// tallies its own copies self-consistently". That was FALSE and it is what hid
|
||||
// this bug: the reroute means exactly ONE node increments, no other node can
|
||||
// observe the killer at all, and nothing carried the value back out -- so every
|
||||
// remote pilot's KILLS read 0 on every other pod, all mission (measured: 125 of
|
||||
// 125 `SCORE type=2` rows credit the logging node's own player). The counters are
|
||||
// now replicated owner->replicant instead; see Read/WriteUpdateRecord above.
|
||||
BTPlayer *killer_player = 0;
|
||||
if (application->GetHostManager() != 0)
|
||||
{
|
||||
@@ -1999,6 +2074,29 @@ void BTPostKillScore(Entity *victim, Scalar damage) // Step 7: KILL (+ MP deat
|
||||
<< " deaths=" << killer_player->GetDeaths()
|
||||
<< " score=" << (Scalar)killer_player->GetScore() << std::endl;
|
||||
}
|
||||
else if (BTMatchLogActive())
|
||||
{
|
||||
//
|
||||
// #45 observability: the credit was SKIPPED. This used to be completely
|
||||
// silent, which is why the bug survived so long -- the counter simply never
|
||||
// moved and no log said why. Record which link in the chain broke so a
|
||||
// single matchlog convicts it: a missing killer entity, an unregistered
|
||||
// mech, a self-kill, or the NULL playerLink hazard.
|
||||
//
|
||||
Entity *k = (application->GetHostManager() != 0)
|
||||
? application->GetHostManager()->GetEntityPointer(
|
||||
((Mech *)victim)->lastInflictingID)
|
||||
: 0;
|
||||
extern int BTIsRegisteredMech(Entity *e);
|
||||
BTMatchLog("NOCREDIT",
|
||||
"victim=%d:%d killerID=%d:%d found=%d self=%d registered=%d link=%d",
|
||||
BTMatchHostOf(victim->GetEntityID()), (int)victim->GetEntityID(),
|
||||
BTMatchHostOf(((Mech *)victim)->lastInflictingID),
|
||||
(int)((Mech *)victim)->lastInflictingID,
|
||||
(int)(k != 0), (int)(k == victim),
|
||||
(int)(k != 0 && BTIsRegisteredMech(k)),
|
||||
(int)(k != 0 && ((Mech *)k)->GetPlayerLink() != 0));
|
||||
}
|
||||
|
||||
// MP DEATH: credit a death to the VICTIM's own player. NULL for the solo
|
||||
// BT_SPAWN_ENEMY dummy (GetPlayerLink()==0) -> skipped, so DEATHS stays 0 in
|
||||
|
||||
Reference in New Issue
Block a user