From c9b67530dbf0281000068566572b379c875acbb9 Mon Sep 17 00:00:00 2001 From: Davide Cappellini Date: Sat, 26 Sep 2026 18:58:26 +0200 Subject: [PATCH] j141 fix: the per-gun real-shot accounting was still array[17] after rack id 17 landed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit REGRESSION: the live bot stopped firing entirely and moved degenerately whenever a rack admitted rack id 17 (the ADE+SBC BITBRAIN gun added in e9302bc). ROOT CAUSE: ModularBot.nim declared the per-gun accounting arrays (gunRealShots / gunRealHits / gunRealShotsByMode / gunRealHitsByMode / gunSelectionCount) as array[17, int] — the rack size BEFORE id 17 existed. The instant the selector picked gun 17, the accounting indexed one past the end: * debug build -> IndexDefect out of run(): the bot stops, 0 shots; * -d:release (shipped) -> silent out-of-bounds write onto the adjacent lastPowerLogKey: string header, so the bot kept moving but never fired and never reported a shot. Measured with the owner's exact out/.env, 1v1 SittingDuck: 0dc5552 (pre-rename): BitBrain(id16) selected 356+157 ticks, realShots 24+9, realHits 23+9, ModularBot wins 180/360 HEAD (e9302bc..) : selected 0, vShots 0, realShots 0, realHits 0 debug : IndexDefect on the first tick that selects id 17 -d:release : same 0/0/0, bot scores 22 and dies FIX: derive every gun-indexed width from the rack instead of a literal. gun_harness/selector exports NumRackGuns* = len(RackGunNames) (18); ModularBot uses it for the five accounting arrays, the per-round reset loops, the gun_stats.jsonl dump loop, the table and initTracker(). Shipped defaults unchanged: clean env is still the onlyPattern rack and movement is still strafe. GUARD: common_libs/tests/test_rack_stat_width.nim (24 checks) — rack table shape, a source scan proving no gun-indexed width/loop bound is narrower than NumRackGuns, an in-process accounting replay that would have overflowed array[17], and the legacy-namespace checks (id 16 via TR_RACK_BITBRAIN, id 17 never admitted while legacy). It reports 7 failures on the pre-fix ModularBot.nim and passes after. Optional --live section proves a rack admitting only the newest id fires and lands hits. Parity: test_env_report 25, test_rack_membership 49, test_lead_gain_ registration 13, test_lead_gain_legacy 24, test_bitbrain_net 44, test_gun_harness 39, test_tfil_commit_env 30, test_bitbrain 56, test_tm_pattern_registration 20 — all unchanged, 0 failures. Co-Authored-By: Claude Opus 4.8 (1M context) --- ModularBot_garage/src/ModularBot.nim | 28 ++- common_libs/gun_harness/selector.nim | 8 + common_libs/tests/test_rack_stat_width.nim | 249 +++++++++++++++++++++ 3 files changed, 274 insertions(+), 11 deletions(-) create mode 100644 common_libs/tests/test_rack_stat_width.nim diff --git a/ModularBot_garage/src/ModularBot.nim b/ModularBot_garage/src/ModularBot.nim index 03bf2bb..b6688c7 100644 --- a/ModularBot_garage/src/ModularBot.nim +++ b/ModularBot_garage/src/ModularBot.nim @@ -286,18 +286,24 @@ type roundNumber: int realShotsFired: int realHits: int - gunRealShots: array[17, int] - gunRealHits: array[17, int] + gunRealShots: array[NumRackGuns, int] + gunRealHits: array[NumRackGuns, int] # Same real-shot accounting split by the rack in force at fire time, so a # later data-driven pass can rank guns per mode (1v1 vs melee). - gunRealShotsByMode: array[vb.RackMode, array[17, int]] - gunRealHitsByMode: array[vb.RackMode, array[17, int]] + # These four arrays and `gunSelectionCount` are indexed by SELECTED GUN ID, + # so their width is `NumRackGuns`, not a literal. A literal `array[17, int]` + # survived the addition of rack id 17 (BITBRAIN) and indexed one past the end + # on the first tick that gun was selected — an IndexDefect in a debug build + # and silent corruption of the adjacent `lastPowerLogKey` string in the + # shipped `-d:release` binary, i.e. a bot that aims, moves and never fires. + gunRealShotsByMode: array[vb.RackMode, array[NumRackGuns, int]] + gunRealHitsByMode: array[vb.RackMode, array[NumRackGuns, int]] pendingFires: seq[PendingShot] ## FIFO of fired shots awaiting onBulletFired bulletId stamp bulletGun: Table[int, int] ## bulletId -> gun id, filled on onBulletFired, drained on resolution bulletMode: Table[int, vb.RackMode] ## bulletId -> rack at fire time bulletShot: Table[int, PendingShot] ## bulletId -> shot metadata (Task A shot log) pendingHitBullets: HashSet[int] ## hit bulletIds seen before their onBulletFired stamp - gunSelectionCount: array[17, int] + gunSelectionCount: array[NumRackGuns, int] lastPowerLogKey: string ## change detector for the TR_POWER_LOG line lastKnownTargetId: int ## persists through death, used for round-end stats # Radar measurement instrumentation (only touched when RadarScanLog is set). @@ -790,7 +796,7 @@ method onRoundEnded*(bot: ModularBot, e: RoundEndedEventForBot) = let fit = bot.tracker.fitnessFor(targetId) var gunsArr = newJArray() - for gid in 0..<17: + for gid in 0.. IndexDefect out of `run()`: the bot stops dead, 0 shots; +## * `-d:release` -> a silent out-of-bounds write that lands on the adjacent +## `lastPowerLogKey: string` header, so the bot kept moving +## but never fired and never reported a shot. +## +## The fix derives every gun-indexed width from `NumRackGuns` (== `len(RackGunNames)`). +## This test is the guard that would have caught it: +## +## A. rack table — `NumRackGuns` is 18, 18 DISTINCT names, and the +## membership table and the legacy alias target are in range; +## B. source shape — `ModularBot.nim` contains NO literal width/loop bound +## narrower than `NumRackGuns` for the gun-indexed +## accounting, and sizes them with `NumRackGuns`; +## C. accounting — an in-process replay of the real shape (`array[NumRackGuns]` +## indexed by every admitted id) never goes out of range, +## and gun 17 IS accounted; +## D. live (opt-in) — with a rack that admits ONLY the newest id, the bot +## selects it, fires and lands hits (skipped without JARs). +## +## Run: nim c -r common_libs/tests/test_rack_stat_width.nim +## Live: TR_SERVER_JAR=... TR_BATTLE_RUNNER=... nim c -r common_libs/tests/test_rack_stat_width.nim + +import std/[os, strutils, json, tables] +import gun_harness/selector +import gun_harness/virtual_bullets +import test_framework/bot_compiler +import test_framework/server_manager +import test_framework/runner_process + +const + repoRoot = currentSourcePath().parentDir.parentDir.parentDir + botSource = repoRoot / "ModularBot_garage" / "src" / "ModularBot.nim" + adversaryDir = repoRoot / "common_libs" / "test_framework" / "adversaries" / "SittingDuck" + statsPath = "/tmp/rack_stat_width_stats.jsonl" + PatternId = 5 + NewestId = NumRackGuns - 1 ## 17, the id that regressed + LegacyLeadGainId = 16 + +var failures = 0 +proc check(name: string, ok: bool) = + if ok: echo "PASS: ", name + else: echo "FAIL: ", name; inc failures + +# ── A. rack table ──────────────────────────────────────────────────────────── + +proc testRackTable() = + check "A1: NumRackGuns is derived from RackGunNames", + NumRackGuns == RackGunNames.len and NumRackGuns == 18 + check "A2: rack membership table has one entry per gun", + DefaultRackMembership.len == NumRackGuns + var seen = initTable[string, bool]() + var allDistinct = true + for n in RackGunNames: + if n in seen: allDistinct = false + seen[n] = true + check "A3: all rack gun names are distinct", allDistinct + check "A4: the newest id is in range for every per-gun table", + NewestId < NumRackGuns and LegacyLeadGainId < NumRackGuns + check "A5: the legacy alias target is a real gun id", + RackLegacyAlias[0][1] < NumRackGuns + +# ── B. the live bot's source is not width-locked to a literal ─────────────── + +proc testSourceShape() = + check "B0: ModularBot.nim exists", fileExists(botSource) + if not fileExists(botSource): return + let src = readFile(botSource) + var lines = src.splitLines() + + # Every array declared as a per-gun accounting array, with its literal width. + let gunArrays = ["gunRealShots", "gunRealHits", "gunRealShotsByMode", + "gunRealHitsByMode", "gunSelectionCount"] + for field in gunArrays: + var found = false + for line in lines: + let st = line.strip() + if not (st.startsWith(field & ":")): continue + found = true + # outer width for a per-mode array, else the single array width + var inner = st[(st.find("array[") + "array[".len) .. ^1] + if st.find("array[") != st.rfind("array["): inner = inner[(inner.find("array[") + "array[".len) .. ^1] + let width = inner[0 ..< inner.find(',')].strip() + check "B1: " & field & " is sized with NumRackGuns (got '" & width & "')", + width == "NumRackGuns" + break + check "B2: " & field & " is still declared", found + + # No hard-coded per-gun width/loop bound narrower than the rack may survive. + # Scoped to the gun-indexed accounting only: other arrays in the file (e.g. the + # 12-bucket covering-arc histogram) are legitimately narrower. + var offenders: seq[string] + for i, line in lines: + let code = line.split('#')[0] + var guarded = false + for field in gunArrays: + if field in code: guarded = true + if not guarded: continue + for token in ["array[", "0..<", "..<", "initTracker("]: + let at = code.find(token) + if at < 0: continue + var rest = code[at + token.len .. ^1].strip(chars = {' ', '('}) + var digits = newStringOfCap(4) + for ch in rest: + if ch in {'0' .. '9'}: digits.add ch + else: break + if digits.len == 0: continue + if digits.len < rest.len and rest[digits.len] in {',', ')', '.'}: continue + if parseInt(digits) < NumRackGuns: + offenders.add "line " & $(i + 1) & ": " & line.strip() + check "B3: no per-gun width or loop bound is hard-coded below the rack size", + offenders.len == 0 + for o in offenders: echo " offender: ", o + var literal17: seq[string] = @[] + for i, line in lines: + if line.split('#')[0].find("array[17,") >= 0 or line.split('#')[0].find("0..<17") >= 0: + literal17.add "line " & $(i + 1) & ": " & line.strip() + check "B4: the old literal 17 is gone from the accounting (comments exempt)", + literal17.len == 0 + for o in literal17: echo " offender: ", o + check "B5: the accounting arrays are adjacent to a documented width note", + "NumRackGuns" in src + +# ── C. the accounting shape, in process ────────────────────────────────────── + +proc testAccounting() = + # Exactly the declaration the live bot now uses. + var gunSelectionCount: array[NumRackGuns, int] + var gunRealShots: array[NumRackGuns, int] + var gunRealHits: array[NumRackGuns, int] + + # The rack that regressed: ONLY the newest gun admitted, in both modes. + var membership = DefaultRackMembership + for i in 0..= 17 + + # The legacy namespace: TR_RACK_BITBRAIN with TR_BITBRAIN_NET unset addresses + # LEADGAIN (16), and id 17 is NOT admitted. + delEnv("TR_BITBRAIN_NET") + for n in RackGunNames: delEnv("TR_RACK_" & n) + putEnv("TR_RACK_BITBRAIN", "both") + let legacy = loadRackMembership() + check "C4: legacy TR_RACK_BITBRAIN admits LEADGAIN (16)", + legacy[LegacyLeadGainId] == rmBoth + check "C5: legacy namespace never admits id 17", + legacy[NewestId] == rmOff + delEnv("TR_RACK_BITBRAIN") + +# ── D. live proof, only with the JARs ──────────────────────────────────────── + +proc lastRound(path: string): JsonNode = + result = nil + for line in lines(path): + let s = line.strip() + if s.len == 0: continue + let node = parseJson(s) + if node.hasKey("guns"): result = node + +proc live() = + if getEnv("TR_SERVER_JAR", "").len == 0 or getEnv("TR_BATTLE_RUNNER", "").len == 0: + echo "SKIP: live section (TR_SERVER_JAR / TR_BATTLE_RUNNER not set)" + return + # Rebuild the bot with a rack that admits ONLY the newest id — the exact + # configuration that used to index one past the end. + # The bot reads `/out/.env`, so the live check has to write there. + # It backs the owner's file up to `/.env.j141-backup` FIRST and restores + # it in a `finally`, so even a hard crash leaves a recoverable copy. + let dotenv = repoRoot / "ModularBot_garage" / "out" / ".env" + let backup = repoRoot / "ModularBot_garage" / "out" / ".env.j141-backup" + let hadDotenv = fileExists(dotenv) + if hadDotenv: copyFile(dotenv, backup) + defer: + if hadDotenv: copyFile(backup, dotenv) + else: removeFile(dotenv) + removeFile(backup) + var linesOut: seq[string] + for l in readFile(backup).splitLines(): + if l.strip().startsWith("GUN_STATS_PATH"): linesOut.add "GUN_STATS_PATH=" & statsPath + elif l.strip().startsWith("TR_RACK_BITBRAIN"): linesOut.add "TR_RACK_BITBRAIN=both" + else: linesOut.add l + writeFile(dotenv, linesOut.join("\n")) + echo "NOTE: live section wrote a temporary out/.env (backup at out/.env.j141-backup)." + + echo "Running one live round (rack: only rack id ", NewestId, ")..." + let compiled = compileBots(@[repoRoot / "ModularBot_garage", adversaryDir]) + echo "compiled: ", compiled + ensureServer() + discard runBattleRunner(getServerUrl(), + @[repoRoot / "ModularBot_garage", adversaryDir], 1, 300_000, true) + removeFile(statsPath) + if not fileExists(statsPath): + check "D1: the bot wrote its per-round gun stats", false + return + let r = lastRound(statsPath) + check "D1: the bot wrote its per-round gun stats", r != nil + if r == nil: return + let guns = r["guns"] + check "D2: the stats dump has one row per rack gun", + guns.len == NumRackGuns + var selected = 0 + var shots = 0 + var hits = 0 + for g in guns: + selected += g["selected"].getInt() + shots += g["realShots"].getInt() + hits += g["realHits"].getInt() + echo " selected=", selected, " realShots=", shots, " realHits=", hits + check "D3: a gun IS selected (the selector reaches the newest id)", selected > 0 + check "D4: shots ARE fired", shots > 0 + check "D5: hits ARE landed", hits > 0 + +when isMainModule: + testRackTable() + testSourceShape() + testAccounting() + if paramCount() > 0 and paramStr(1) == "--live": live() + else: echo "SKIP: live section (pass --live with the JAR env vars)" + echo "" + if failures == 0: echo "ALL CHECKS PASSED" + else: + echo failures, " CHECK(S) FAILED" + quit(1)