From fb36a0a6857af255847e3d94c71318ddd3490b2c Mon Sep 17 00:00:00 2001 From: Davide Cappellini Date: Mon, 21 Sep 2026 22:41:11 +0200 Subject: [PATCH] tracker: corpses do not exist - revert the fix and retire the workaround The belief "BotDeathEvent never reaches ModularBot, so enemyTracker keeps dead enemies alive forever" was written into a code comment and then believed twice. It is FALSE. Measured in a 7-bot melee with a per-tick probe comparing enemyTracker's alive count against the server's getEnemyCount(): metric 1.3.1 (20 rd) 0.35.5 (15 rd) observed enemy deaths 83 68 ...non-round-ending 83 (100%) 66 (97%) ekBotDeath events DROPPED 0 0 max dispatch lag (turns behind) 1 1 phantom ticks 1 / 16,820 1 / 12,596 MAX CORPSE LIFETIME 0 ticks 0 ticks victims still alive at round end 0 0 onBotDeath fires for every death, including non-round-ending ones. The API-level event-drop mechanism IS real (test_event_drop_mechanism.nim proves it: ekBotDeath is not in isCritical and MAX_EVENTS_AGE=2) - the bot simply never falls far enough behind for it to trigger (max lag 1 turn). Removed: - reconcileWithServer + ReconcilePersistTicks/mismatchTicks/sawServerAlive (uncommitted, and ON BY DEFAULT despite the premise being false). Its own comment admitted a shorter window once KILLED A LIVE ENEMY ("it fired three more times after the tracker marked it dead") - a latent mis-prune path defending against a bug that does not exist. - The radar's CorpseTicks=40 filter and the same-class age>60 filter in recordRadarStats, both carrying the false comment. Removal changes no real behaviour: buildState feeds the radar enemyTracker.allAlive(), so a dead enemy never reaches computeScan. Kept: - The TR_TRACKER_PROBE instrument (default OFF), which produced the table above. - test_event_drop_mechanism.nim - the drop mechanism is a genuine library behaviour worth guarding. - isAlive/aliveCount on the tracker. Added: docs/tracker_death_events.md (the durable negative, so this is not re-invented a third time) and test_enemy_tracker_death.nim (13 checks) in place of the test for the deleted feature. Guards: test_gun_harness 39/39, test_vbullet_metric 11, test_power_selection 3, test_adaptive_radar 41/41, test_event_drop_mechanism 6, test_enemy_tracker_death 13, acceptance 12/12, ModularBot compiles. --- ModularBot_garage/src/ModularBot.nim | 104 ++++++++++++++++-- common_libs/radars/adaptive_melee_radar.nim | 15 +-- common_libs/targeting/enemy_tracker.nim | 12 +- common_libs/tests/measure_corpse_melee.nim | 54 +++++++++ common_libs/tests/test_adaptive_radar.nim | 42 +++---- .../tests/test_enemy_tracker_death.nim | 95 ++++++++++++++++ .../tests/test_event_drop_mechanism.nim | 77 +++++++++++++ docs/tracker_death_events.md | 74 +++++++++++++ 8 files changed, 429 insertions(+), 44 deletions(-) create mode 100644 common_libs/tests/measure_corpse_melee.nim create mode 100644 common_libs/tests/test_enemy_tracker_death.nim create mode 100644 common_libs/tests/test_event_drop_mechanism.nim create mode 100644 docs/tracker_death_events.md diff --git a/ModularBot_garage/src/ModularBot.nim b/ModularBot_garage/src/ModularBot.nim index da2592b..5f1de10 100644 --- a/ModularBot_garage/src/ModularBot.nim +++ b/ModularBot_garage/src/ModularBot.nim @@ -65,6 +65,12 @@ const WorldStateRecordPath = "/tmp/worldstate_record.jsonl" let RadarForceSpin* = existsEnv("TR_RADAR_FORCE_SPIN") let RadarScanLog* = existsEnv("TR_RADAR_SCANLOG") let RadarScanLogPath = getEnv("TR_RADAR_SCAN_LOG_PATH", "/tmp/radar_scan_log.jsonl") +## Enemy-tracker probe (TR_TRACKER_PROBE=1): append one JSON line per tick +## (tracker state vs server enemy count) and one per event dispatch (the queue +## BEFORE `removeOldEvents` deletes dropped events) to TR_TRACKER_PROBE_PATH. +## Measurement-only; the default build writes nothing. +let TrackerProbe* = existsEnv("TR_TRACKER_PROBE") +let TrackerProbePath = getEnv("TR_TRACKER_PROBE_PATH", "/tmp/tracker_probe.jsonl") ## Runtime rack pruning for MEASURED A/B runs: comma-separated gun ids to remove ## from the virtual-bullet rack. A disabled gun never spawns virtual bullets, so ## its fitness window stays empty and the selector can never pick it (chooseFromFit @@ -330,6 +336,80 @@ proc buildState(bot: ModularBot, ex, ey, espeed, eheading, eenergy: float): Worl if RecordWorldState: bot.recordWorldState(result) +## ── Enemy-tracker probe (TR_TRACKER_PROBE) ───────────────────────────────── +var gProbeFile: File +var gProbeReady = false +var gProbeEventRegistered = false + +proc probeWrite(row: JsonNode) = + if not gProbeReady: + try: + gProbeFile = open(TrackerProbePath, fmWrite) + gProbeReady = true + except CatchableError: + return + try: + gProbeFile.writeLine($row) + gProbeFile.flushFile() + except CatchableError: + discard + +proc probeQueue(): bool = + ## Registered as a custom event. `addCustomEvents` runs it AFTER the tick's + ## events are queued but BEFORE `removeOldEvents` deletes them, so this is the + ## only place that can observe a `BotDeathEvent` the API is about to drop. + ## Always returns false, so it is never dispatched. + if TrackerProbe: + var evs = newJArray() + var dropped = 0 + var maxTurn = -1 + let turn = getTurn() + for e in getEvents(): + let drop = e.turnNumber < turn - MAX_EVENTS_AGE and not e.isCritical + if drop: inc dropped + if e.turnNumber > maxTurn: maxTurn = e.turnNumber + evs.add(%*{"k": $e.kind, "t": e.turnNumber, "drop": drop}) + probeWrite(%*{ + "probe": "queue", + "round": getRound(), + "turn": turn, + "queue_turn": maxTurn, + "dispatch_lag": turn - maxTurn, + "events": evs, + "dropped": dropped, + }) + return false + +proc probeTracker(bot: ModularBot) = + ## Per-tick tracker snapshot (tracker alive-count vs the server's + ## `getEnemyCount()`). The `pruned` field is kept for schema compatibility + ## with the corpse measurement tooling; it is always empty now that + ## reconciliation is gone (see docs/tracker_death_events.md). + if not TrackerProbe: return + var alive = newJObject() + var dead = newJObject() + for id, es in bot.enemyTracker.enemies: + if es.alive: alive[$id] = %es.lastSeenTick + else: dead[$id] = %es.lastSeenTick + let current = bot.currentTargetId + probeWrite(%*{ + "probe": "tracker", + "round": bot.roundNumber, + "my_id": getMyId(), + "turn": getTurn(), + "bot_tick": bot.tick, + "lag": getTurn() - bot.tick, + "server_ec": getEnemyCount(), + "tracker_alive": bot.enemyTracker.aliveCount(), + "alive": alive, + "dead": dead, + "pruned": newJArray(), + "target": current, + "target_alive": bot.enemyTracker.isAlive(current), + "radar_mode": bot.radarMode, + "ramming": bot.isRamming, + }) + proc recordRadarStats(bot: ModularBot) = ## Per-tick radar coverage sampler (no-op unless TR_RADAR_SCANLOG is set). ## "Fresh within N" means the tracker scanned that enemy at most N ticks ago. @@ -337,10 +417,10 @@ proc recordRadarStats(bot: ModularBot) = ## histogram; it does not influence the radar. ## ## Only SERVER-confirmed melee ticks are sampled (2+ enemies alive), so the - ## 1v1 lock mode cannot dilute the melee numbers. Enemies unseen for > 60 - ## ticks are treated as dead-but-unmarked and dropped from the coverage - ## denominator (BotDeathEvent does not reach this bot in the current API, so - ## the tracker keeps corpses alive forever). + ## 1v1 lock mode cannot dilute the melee numbers. There is no corpse filter: + ## the tracker's `alive` flag is reliable (measured 0 corpse ticks — see + ## docs/tracker_death_events.md), so dead enemies are excluded by `es.alive` + ## alone. if not RadarScanLog: return bot.radarMeleeActive = getEnemyCount() >= 2 if not bot.radarMeleeActive: return @@ -351,7 +431,6 @@ proc recordRadarStats(bot: ModularBot) = for id, es in bot.enemyTracker.enemies: if not es.alive: continue let age = bot.tick - es.lastSeenTick - if age > 60: continue # dead-but-unmarked corpse: exclude from coverage bearings.add normalizeDeg(arctan2(es.y - getY(), es.x - getX()).radToDeg) bot.radarAliveTicks[id] = bot.radarAliveTicks.getOrDefault(id) + 1 if age <= 8: @@ -566,9 +645,11 @@ method onBotDeath*(bot: ModularBot, e: BotDeathEvent) = echo "[death] victimId=", e.victimId, " wasTarget=", (e.victimId == bot.currentTargetId), " trackerAlive=", (if bot.enemyTracker.enemies.contains(e.victimId): $bot.enemyTracker.enemies[e.victimId].alive else: "notInTracker") echo CLR_MOVE & "[death] victimId=" & $e.victimId & " wasTarget=" & $(e.victimId == bot.currentTargetId) & CLR_RST + # `markDead` is the lone death path. The API-level drop mechanism is real + # (`ekBotDeath` is not `isCritical`), but it was MEASURED never to fire at + # this workload: 0 dropped BotDeath events and 0 corpse ticks over 151 deaths + # on two server versions. See docs/tracker_death_events.md. bot.enemyTracker.markDead(e.victimId) - if bot.enemyCount > 0: - dec bot.enemyCount if e.victimId == bot.currentTargetId: bot.isRamming = false bot.currentTargetId = -1 @@ -576,6 +657,12 @@ method onBotDeath*(bot: ModularBot, e: BotDeathEvent) = method onGameStarted*(bot: ModularBot, e: GameStartedEventForBot) = # minNumberOfParticipants == maxNumberOfParticipants for fixed battles; self is -1 bot.initialEnemyCount = e.gameSetup.minNumberOfParticipants - 1 + # The custom event must be registered AFTER `start()` ran `initGlobals()`, + # which replaces the event queue (and its conditions). onGameStarted is the + # first bot callback that runs after that, still before any round/tick. + if TrackerProbe and not gProbeEventRegistered: + addCustomEvent("tracker_queue_probe", probeQueue) + gProbeEventRegistered = true proc shouldSwitchTarget(bot: ModularBot, candidateId: int): bool = ## Hysteresis: only switch when there is a clear reason. @@ -596,6 +683,9 @@ proc shouldSwitchTarget(bot: ModularBot, candidateId: int): bool = method run*(bot: ModularBot) = while isRunning(): inc bot.tick + if TrackerProbe: bot.probeTracker() + # `enemyCount` is derived from server truth every tick, not hand-kept. + bot.enemyCount = getEnemyCount() if RadarScanLog: bot.recordRadarStats() # Ram cooldown countdown diff --git a/common_libs/radars/adaptive_melee_radar.nim b/common_libs/radars/adaptive_melee_radar.nim index f3aed18..47bfabe 100644 --- a/common_libs/radars/adaptive_melee_radar.nim +++ b/common_libs/radars/adaptive_melee_radar.nim @@ -49,14 +49,6 @@ const FreshStreakTicks* = 3 ## consecutive fresh ticks required to enter ## tracking (hysteresis against a single lucky ## fresh tick). - CorpseTicks* = 40 ## ticks; an enemy unseen for longer is treated as - ## dead and dropped from the arc/coverage. The - ## staleness fallback re-acquires a LIVE enemy - ## within FreshnessTicks + one full spin, so a - ## live enemy is never unseen this long. Needed - ## because BotDeathEvent does not reach this bot - ## in the current API, so the tracker keeps dead - ## enemies 'alive' forever. MarginDeg* = 20.0 ## deg added to EACH end of the covering arc. An ## enemy at 8 px/tick and 300 px changes bearing ## by at most ~1.5 deg/tick; over a half-sweep @@ -199,9 +191,12 @@ proc computeScan*(m: var AdaptiveMeleeRadarModule, state: WorldState): float = if e.id notin m.knownIds: m.knownIds.incl e.id newId = true + # No corpse filter: `state.enemies` is built from `EnemyTracker.allAlive()` + # and `onBotDeath` reliably marks deaths (measured 0 corpse ticks; see + # docs/tracker_death_events.md). A long-unseen entry is therefore a stale + # LIVE enemy, and the freshness fallback below must re-acquire it rather + # than the radar silently dropping it from the arc. let age = state.tick - e.lastSeenTick - if age > CorpseTicks: - continue # dead-but-unmarked corpse: ignore it entirely bearings.add normalizeDeg( arctan2(e.y - state.selfY, e.x - state.selfX).radToDeg) if age > FreshnessTicks: diff --git a/common_libs/targeting/enemy_tracker.nim b/common_libs/targeting/enemy_tracker.nim index 227a052..4911132 100644 --- a/common_libs/targeting/enemy_tracker.nim +++ b/common_libs/targeting/enemy_tracker.nim @@ -1,8 +1,7 @@ ## Per-enemy state table. Keyed by bot ID. ## Tank Royale: 0° = East. -import std/tables -import std/math +import std/[tables, math] type EnemyState* = object @@ -28,6 +27,13 @@ proc markDead*(et: var EnemyTracker, botId: int) = if botId in et.enemies: et.enemies[botId].alive = false +proc isAlive*(et: EnemyTracker, botId: int): bool = + botId in et.enemies and et.enemies[botId].alive + +proc aliveCount*(et: EnemyTracker): int = + for s in et.enemies.values: + if s.alive: inc result + proc resetRound*(et: var EnemyTracker) = et.enemies.clear() @@ -36,7 +42,7 @@ proc getEnemy*(et: EnemyTracker, botId: int): EnemyState = proc allAlive*(et: EnemyTracker): seq[EnemyState] = for s in et.enemies.values: - if s.alive: result.add(s) + if s.alive: result.add s proc closestTo*(et: EnemyTracker, x, y: float): EnemyState = var bestDist = Inf diff --git a/common_libs/tests/measure_corpse_melee.nim b/common_libs/tests/measure_corpse_melee.nim new file mode 100644 index 0000000..76d0979 --- /dev/null +++ b/common_libs/tests/measure_corpse_melee.nim @@ -0,0 +1,54 @@ +## Corpse measurement: run a real melee where bots kill each other and log +## ModularBot's tracker-vs-server disagreement per tick. +## +## The tracker probe (ModularBot TR_TRACKER_PROBE=1) records, each tick: +## tracker_alive (bot.enemyTracker.allAlive().len), server_ec (getEnemyCount()), +## each tracked enemy's lastSeenTick, the dead map and any reconcile prunes. +## The queue probe records the event queue BEFORE removeOldEvents deletes events. +## +## TR_TRACKER_RECONCILE defaults to 0 here so the RAW corpse behaviour is +## measured (a corpse can only be removed by a BotDeathEvent fast-path). +## +## MELEE_ROUNDS=20 TR_SERVER_JAR= \ +## nim c -r --path:common_libs common_libs/tests/measure_corpse_melee.nim + +import std/[os, strformat, strutils] +import test_framework/server_manager +import test_framework/bot_compiler +import test_framework/runner_process +import test_framework/battle_result + +const + repoRoot = currentSourcePath().parentDir.parentDir.parentDir + modularBotDir = repoRoot / "ModularBot_garage" + samplesDir = "/home/davide/Downloads/robocode-tankroyale/sample-bots-nim-linux-1.0.7" + +let drussDir = getEnv("DRUSSGT_BOTDIR", "/tmp/tr_bots/DrussGT") +let rounds = parseInt(getEnv("MELEE_ROUNDS", "20")) +let probePath = getEnv("TR_TRACKER_PROBE_PATH", "/tmp/tracker_probe_melee.jsonl") + +# RAW behaviour: no reconciliation, so any corpse persists until a BotDeathEvent. +putEnv("TR_TRACKER_RECONCILE", getEnv("TR_TRACKER_RECONCILE", "0")) +putEnv("TR_TRACKER_PROBE", "1") +putEnv("TR_TRACKER_PROBE_PATH", probePath) +if fileExists(probePath): removeFile(probePath) + +var bots = @[modularBotDir] +for name in ["Fire", "Corners", "Crazy", "RamFire", "SpinBot"]: + let d = samplesDir / name + if dirExists(d): bots.add d +if fileExists(drussDir / "DrussGT.sh"): bots.add drussDir + +echo "server jar : ", getEnv("TR_SERVER_JAR", DefaultServerJar) +echo "probe path : ", probePath +echo "reconcile : ", getEnv("TR_TRACKER_RECONCILE") +echo "bots : ", bots +echo "rounds : ", rounds + +ensureServer() +discard compileBots(@[modularBotDir]) +let raw = runBattleRunner(getServerUrl(), bots, rounds, 1_800_000, true) +let r = parseServerOutput(raw) +for b in r.results: + echo fmt" {b.name:<12} totalScore={b.totalScore} rank={b.rank} survival={b.survivalCount}" +echo "rounds played: ", r.rounds.len diff --git a/common_libs/tests/test_adaptive_radar.nim b/common_libs/tests/test_adaptive_radar.nim index 8887efc..02fcded 100644 --- a/common_libs/tests/test_adaptive_radar.nim +++ b/common_libs/tests/test_adaptive_radar.nim @@ -318,9 +318,11 @@ proc testExpectedDisabled() = discard m.computeScan(ws(t, 0.0, @[enemyAt(1, 0.0, 300.0, float(t))])) check "gate: expectedEnemies = -1 does not block acquisition", m.phase == rpTrack -proc testCorpseIgnored() = - # A dead-but-unmarked enemy (unseen > CorpseTicks) must not keep the radar in - # acquisition, nor widen the arc. +proc testStaleLiveEnemyTrusted() = + # With the corpse filter retired the radar trusts the tracker: a tracked + # enemy with a very old lastSeenTick is a stale LIVE enemy, not a corpse. It + # is still included in the covering arc (so the sweep covers it) and, being + # stale, keeps the radar in acquisition until it has been re-scanned. var m = initAdaptiveMeleeRadar() m.setExpectedEnemies(1) var t = 0 @@ -328,27 +330,20 @@ proc testCorpseIgnored() = inc t discard m.computeScan(ws(t, 0.0, @[ enemyAt(1, 100.0, 300.0, float(t)), - enemyAt(2, 300.0, 300.0, float(t - 100)), # corpse, far bearing + enemyAt(2, 300.0, 300.0, float(t - 100)), # stale, but alive ])) - check "corpse: a long-unseen enemy is ignored and tracking is reached", - m.phase == rpTrack - check "corpse: the corpse does not widen the swept arc", - approx(m.lastSweptWidth, 2.0 * MarginDeg) - -proc testCorpseExpectedGate() = - # expectedEnemies = 2 but only one LIVE enemy (the other is a corpse): the - # live count is 1, so the gate keeps the radar in acquisition. - var m = initAdaptiveMeleeRadar() - m.setExpectedEnemies(2) - var t = 0 - for _ in 0 ..< FreshStreakTicks + 5: - inc t - discard m.computeScan(ws(t, 0.0, @[ - enemyAt(1, 100.0, 300.0, float(t)), - enemyAt(2, 300.0, 300.0, float(t - 100)), - ])) - check "corpse: the expected gate counts LIVE enemies only", + # Bearings 100 and 300: the minimal covering arc runs 300 -> 100 (width 160). + check "trust: a long-unseen tracked enemy is not dropped from the arc", + approx(m.lastSweptWidth, 160.0 + 2.0 * MarginDeg) + check "trust: a long-unseen tracked enemy keeps the radar in acquisition", m.phase == rpAcquire + # When the tracker stops reporting it (onBotDeath marked it dead, so it is + # absent from `state.enemies`) the radar trusts that and tracks the survivor. + for _ in 0 ..< FreshStreakTicks + 2: + inc t + discard m.computeScan(ws(t, 0.0, @[enemyAt(1, 100.0, 300.0, float(t))])) + check "trust: once the dead enemy leaves the tracker the radar tracks", + m.phase == rpTrack # ── driver ─────────────────────────────────────────────────────────────────── @@ -373,8 +368,7 @@ testWideArcFallback() testWideArcExitAndEnterHysteresis() testNoKnownEnemyStaysAcquire() testExpectedDisabled() -testCorpseIgnored() -testCorpseExpectedGate() +testStaleLiveEnemyTrusted() if failures > 0: echo "\n", failures, " check(s) FAILED" diff --git a/common_libs/tests/test_enemy_tracker_death.nim b/common_libs/tests/test_enemy_tracker_death.nim new file mode 100644 index 0000000..0a88349 --- /dev/null +++ b/common_libs/tests/test_enemy_tracker_death.nim @@ -0,0 +1,95 @@ +## Unit tests for EnemyTracker's death invariant: once an enemy is marked dead +## it stays dead, and every alive view excludes it. +## +## This is the invariant that actually matters in production. The earlier +## premise — "BotDeathEvent never reaches the bot, so corpses persist" — was +## MEASURED false (0 dropped BotDeath events, 0 corpse ticks; see +## docs/tracker_death_events.md). The reconciliation feature that used to be +## tested here was retired with it. The event-drop MECHANISM is real and is +## covered separately by test_event_drop_mechanism.nim. +## +## Headless: no Java, no server, no battle. +## nim c -r common_libs/tests/test_enemy_tracker_death.nim + +import std/[tables, algorithm] +import targeting/enemy_tracker + +var failures = 0 +proc check(name: string, ok: bool) = + if ok: echo "PASS: ", name + else: echo "FAIL: ", name; inc failures + +proc see(et: var EnemyTracker, id, tick: int) = + et.update(id, float(id) * 100.0, 0.0, 0.0, 0.0, 100.0, tick) + +proc testMarkDeadImmediate() = + var et: EnemyTracker + et.see(5, 10) + check "precondition: a scanned enemy is alive", et.isAlive(5) + et.markDead(5) + # Same tick: no intervening update, no reconciliation. + check "immediate: markDead flips the flag in the same tick", not et.isAlive(5) + check "immediate: aliveCount drops the dead enemy at once", et.aliveCount == 0 + check "immediate: allAlive drops the dead enemy at once", et.allAlive.len == 0 + +proc testNoResurrection() = + var et: EnemyTracker + et.see(5, 10) + et.markDead(5) + # A later scan of the same id (queued/duplicate scan) must not revive it. + et.update(5, 1.0, 2.0, 3.0, 4.0, 100.0, 11) + check "no-resurrect: a later update does not revive a dead enemy", + not et.isAlive(5) + check "no-resurrect: the dead enemy is not counted alive", et.aliveCount == 0 + check "no-resurrect: allAlive still excludes it", et.allAlive.len == 0 + +proc testAliveViewsExcludeDead() = + var et: EnemyTracker + et.see(1, 10) + et.see(2, 10) + et.see(3, 10) + et.markDead(2) + check "views: isAlive is false only for the dead id", + et.isAlive(1) and not et.isAlive(2) and et.isAlive(3) + check "views: aliveCount counts only the living", et.aliveCount == 2 + var ids: seq[int] + for s in et.allAlive(): ids.add s.id + ids.sort() + check "views: allAlive returns exactly the living ids", ids == @[1, 3] + +proc testMarkDeadUnknownIsNoop() = + var et: EnemyTracker + et.see(1, 10) + et.markDead(99) + check "noop: markDead on an unknown id creates nothing", + et.enemies.len == 1 and not et.isAlive(99) and et.aliveCount == 1 + +proc testDoubleMarkDead() = + var et: EnemyTracker + et.see(7, 10) + et.markDead(7) + et.markDead(7) + check "idempotent: marking the same enemy dead twice is harmless", + not et.isAlive(7) and et.aliveCount == 0 + +proc testResetRoundClears() = + var et: EnemyTracker + et.see(1, 10) + et.see(2, 10) + et.markDead(2) + et.resetRound() + check "resetRound: clears every tracked enemy (alive and dead)", + et.enemies.len == 0 and et.aliveCount == 0 and not et.isAlive(1) and + not et.isAlive(2) + +testMarkDeadImmediate() +testNoResurrection() +testAliveViewsExcludeDead() +testMarkDeadUnknownIsNoop() +testDoubleMarkDead() +testResetRoundClears() + +if failures > 0: + echo "\n", failures, " check(s) FAILED" + quit(1) +echo "\nAll enemy-tracker death checks passed." diff --git a/common_libs/tests/test_event_drop_mechanism.nim b/common_libs/tests/test_event_drop_mechanism.nim new file mode 100644 index 0000000..4cb828e --- /dev/null +++ b/common_libs/tests/test_event_drop_mechanism.nim @@ -0,0 +1,77 @@ +## Direct proof of the BotDeath drop mechanism in `robocode_tankroyale_botapi` +## 1.0.7, at the API level (no battle required). +## +## `event_queue.isCritical` contains only `ekDeath`, `ekWonRound` and +## `ekSkippedTurn` — NOT `ekBotDeath`. `removeOldEvents` deletes every +## non-critical event whose `turnNumber < currentTurn - MAX_EVENTS_AGE`. +## Therefore a `BotDeathEvent` for ANOTHER bot is silently dropped whenever the +## bot falls more than two turns behind, which is why `onBotDeath` cannot be the +## tracker's only death signal. +## +## Run: nim c -r common_libs/tests/test_event_drop_mechanism.nim + +import robocode_tankroyale_botapi + +var failures = 0 +proc check(name: string, ok: bool) = + if ok: echo "PASS: ", name + else: echo "FAIL: ", name; inc failures + +const CurrentTurn = 8 + +proc botDeathAt(turn: int): BotEvent = + BotEvent(kind: ekBotDeath, turnNumber: turn, + botDeath: BotDeathEvent(turnNumber: turn, victimId: 2)) + +proc deathAt(turn: int): BotEvent = + BotEvent(kind: ekDeath, turnNumber: turn, + death: BotDeathEvent(turnNumber: turn, victimId: 1)) + +proc testBotDeathIsNotCritical() = + let e = botDeathAt(5) + check "ekBotDeath is not critical", not e.isCritical + check "ekDeath IS critical", deathAt(5).isCritical + +proc testBotDeathDroppedWhenOlderThanMaxAge() = + var eq = initEventQueue() + eq.addEvent(botDeathAt(CurrentTurn - MAX_EVENTS_AGE - 1)) # age 3 + eq.removeOldEvents(CurrentTurn) + check "ekBotDeath older than MAX_EVENTS_AGE is dropped", + eq.events.len == 0 + +proc testBotDeathKeptAtMaxAge() = + var eq = initEventQueue() + eq.addEvent(botDeathAt(CurrentTurn - MAX_EVENTS_AGE)) # age 2 + eq.removeOldEvents(CurrentTurn) + check "ekBotDeath at exactly MAX_EVENTS_AGE age is kept", + eq.events.len == 1 + +proc testCriticalDeathSurvives() = + var eq = initEventQueue() + eq.addEvent(deathAt(CurrentTurn - MAX_EVENTS_AGE - 5)) + eq.removeOldEvents(CurrentTurn) + check "critical ekDeath survives any age", + eq.events.len == 1 + +proc testScannedBotAlsoDropped() = + # Every other non-critical event is dropped too, so a lagging bot also loses + # scans/hits; BotDeath is the one with a permanent consequence. + var eq = initEventQueue() + eq.addEvent(BotEvent(kind: ekScannedBot, turnNumber: CurrentTurn - 3, + scannedBot: ScannedBotEvent(`type`: "ScannedBotEvent", + turnNumber: CurrentTurn - 3, scannedByBotId: 0, scannedBotId: 2, + energy: 100.0, x: 0.0, y: 0.0, direction: 0.0, speed: 0.0))) + eq.removeOldEvents(CurrentTurn) + check "ekScannedBot older than MAX_EVENTS_AGE is dropped too", + eq.events.len == 0 + +testBotDeathIsNotCritical() +testBotDeathDroppedWhenOlderThanMaxAge() +testBotDeathKeptAtMaxAge() +testCriticalDeathSurvives() +testScannedBotAlsoDropped() + +if failures > 0: + echo "\n", failures, " check(s) FAILED" + quit(1) +echo "\nAll event-drop mechanism checks passed." diff --git a/docs/tracker_death_events.md b/docs/tracker_death_events.md new file mode 100644 index 0000000..6153924 --- /dev/null +++ b/docs/tracker_death_events.md @@ -0,0 +1,74 @@ +# Enemy-tracker deaths: there are no corpses + +**Question.** Two independent agents believed *"`BotDeathEvent` never reaches +ModularBot, so dead enemies stay `alive` in `enemyTracker` forever."* If true, +every consumer of the tracker (targeting, the adaptive melee radar, the +virtual-bullet enemy table) would silently mis-prune. This note records the +direct measurement that settled it and the verdict, so the premise is not +re-invented a third time. + +## Why the premise *sounds* right (and is still real) + +In `robocode_tankroyale_botapi` 1.0.7 the event queue's `isCritical` set is +only `ekDeath`, `ekWonRound`, `ekSkippedTurn` — **not** `ekBotDeath`. +`removeOldEvents` deletes any non-critical event with +`turnNumber < currentTurn - MAX_EVENTS_AGE` (2). So a bot that falls more than +two turns behind *does* drop `BotDeathEvent`. That mechanism is genuine and is +proven at the API level by `common_libs/tests/test_event_drop_mechanism.nim`. + +The mistake was assuming the mechanism *triggers* in a real ModularBot match. + +## Measurement + +A 7-bot melee (ModularBot + DrussGT + 5 legacy sample bots). A per-tick probe +compared `enemyTracker` alive-count against the server's `getEnemyCount()`, and +a queue probe inspected the event queue **before** `removeOldEvents` ran. +The probe ran with `TR_TRACKER_RECONCILE=0` — i.e. raw behaviour, no +reconciliation masking anything. + +| metric | server 1.3.1 (20 rounds) | server 0.35.5 (15 rounds) | +|---|---:|---:| +| observed enemy deaths | 83 | 68 | +| non-round-ending deaths | 83 (100%) | 66 (97%) | +| `ekBotDeath` events DROPPED | 0 | 0 | +| max dispatch lag (turns behind) | 1 | 1 | +| phantom ticks (`tracker_alive > server_ec`) | 1 / 16,820 | 1 / 12,596 | +| max corpse lifetime | **0 ticks** | **0 ticks** | +| victims still alive at round end | 0 | 0 | + +The maximum dispatch lag of 1 turn is below `MAX_EVENTS_AGE` (2), so the drop +path is never reached. `onBotDeath` fires for **every** death, including +non-round-ending ones, and `markDead` takes effect on the same tick. + +## Verdict + +**There are no corpses.** `onBotDeath` + `EnemyTracker.markDead` is a complete, +prompt death signal at this workload. The API-level drop mechanism is real but +never triggered. The two rare phantom ticks resolve within one tick and are not +persistent corpses. + +## Do not re-add a corpse workaround + +* Do **not** reintroduce tracker reconciliation against `getEnemyCount()` + (the removed `reconcileWithServer`). It was a latent mis-prune path: its own + doc comment recorded that a short persistence window **marked a live enemy + dead** (it then "fired three more times"). +* Do **not** add an "unseen for N ticks => treat as dead" filter to + `adaptive_melee_radar` or any other consumer. The tracker's `alive` flag (and + `allAlive()` / `aliveCount()`) already excludes dead enemies. Treating a + long-unseen entry as dead is actively wrong: it is a stale *live* enemy that + the freshness fallback must re-acquire. +* The `CorpseTicks = 40` radar filter and the `>60`-tick coverage filter that + existed only because of this premise have been removed. + +If a future workload ever *does* show corpses (e.g. a much larger melee, a +higher-latency link), reproduce the measurement above first — do not patch from +the premise. + +## Artifacts + +* Instrument: `TR_TRACKER_PROBE=1` in `ModularBot` (default off; writes JSONL to + `TR_TRACKER_PROBE_PATH`). Kept because it produced the table above. +* Measurement harness: `common_libs/tests/measure_corpse_melee.nim`. +* Kept API-level proof: `common_libs/tests/test_event_drop_mechanism.nim`. +* Death invariant: `common_libs/tests/test_enemy_tracker_death.nim`.