From d02c272801b2e616a69efa6272b8ec8df9bce73b Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Thu, 20 Aug 2026 18:04:07 -0400 Subject: [PATCH] fix(result): wait for the final round's board, not for one that never comes MegaMek writes a board when a phase ends and only for five of them - BoardView.gamePhaseChange tests oldPhase against deployment, movement, targeting, firing, physical - so no victory render is ever written and the old wait for one newer than the marker could only pass by accident. Co-Authored-By: Claude Opus 5 (1M context) --- TODO.md | 21 ++++++++++++++ container/lib/collect.sh | 47 ++++++++++++++++++++++--------- tests/shell/test-results-watch.sh | 41 +++++++++++++++++---------- 3 files changed, 80 insertions(+), 29 deletions(-) diff --git a/TODO.md b/TODO.md index 036bb56..7522eee 100644 --- a/TODO.md +++ b/TODO.md @@ -173,6 +173,27 @@ the finish is supposed to leave behind. only what did not land. Everything that is *not* complete at victory - the latency table, the client's logs, the profiles - still goes at exit, because that is when the JVMs stop writing them. +- [ ] **Kill attribution is not computed anywhere yet.** "Kills" today would + mean "enemy units that stopped existing", which counts ammo explosions, + falls and a machine that walked into a chasm. Two sources could say who + actually did it, and neither is wired up. MegaMek tracks it per unit + (`Entity.getKillerId`), which `UnitReport` does not emit - one field, and + the engine's own answer, recorded when it happened. Or it can be derived + from `raw/game_actions_N.tsv`, which carries attacker-to-target attack + rows and per-phase unit state, and is now kept for every match. + + What is undecided is where the deriving lives. In the container it would + have to run at victory, on two cores, over a file that grows with the + match - a long one is megabytes of TSV and the parse is not free at the + moment a player is waiting to be handed on. In headquarters it can run + off S3, be re-run when the logic improves, and never delay anybody; but + that is a parser in a second language against a format upstream owns. + The field and the derivation are also not exclusive: the field is + evidence and the derivation is a claim, and a record could carry both. + + Deliberately parked until matches are on-lexicon, because what the + numbers are *for* decides where they should be computed. + - [x] **Artifacts are filed by what they are.** `launch/` is what the match was given - the staged `scenario.mms` and `identity.json`, both sent at init, so a match that never reaches victory still says what it was and who was diff --git a/container/lib/collect.sh b/container/lib/collect.sh index da67ca3..cd880a4 100644 --- a/container/lib/collect.sh +++ b/container/lib/collect.sh @@ -253,28 +253,43 @@ mm_logdir() { # it. A screenshot taken around some interesting moment mid-match would land # beside it; today the last frame is the only one we take. # -# Version sort, not lexical: round_10 sorts before round_2 in a plain sort, so -# a lexical "last" is the last one alphabetically and not the last one played. +# Version sort. MegaMek zero-pads both numbers - round_%03d_%03d_ - so a +# plain sort agrees with it today and this is belt and braces; it stops being +# so the moment a match passes round 999 or upstream changes the format. # # Sent before the result, which is why it is called separately rather than left # to the image sweep: headquarters draws a card as soon as it can read a # result, and Bluesky fetches that card once, when the post is made. A result # that lands first is a window where both get a card with no board on it. collect_board() { - local dir last dest waited limit marker + local dir last dest waited limit round want dir="$(mm_logdir)/gameSummaries/board" [ -d "$dir" ] || return 0 - # The victory board is rendered on the same phase change that writes the - # marker, so it is usually a fraction of a second away. Without the wait the - # card is the round before the one that decided the match. Bounded: nothing - # else sends until this returns. - marker="$ARENA_STATE/game-over" + # Wait for the final round's board, not for "a render newer than the marker". + # + # MegaMek writes a board summary when a phase *ends*, and only for five of + # them - BoardView.gamePhaseChange takes oldPhase and tests isDeployment, + # isMovement, isTargeting, isFiring, isPhysical. VICTORY is never an old + # phase there, so no victory render is ever written and the last picture of + # any match is the phase that preceded the end. The old test here waited for + # something newer than `game-over`, which MegaMek does not produce: it could + # only pass by accident, when the client happened to process the last phase + # change after the host had already seen VICTORY, and otherwise burned its + # whole budget before falling through to the same file it would have picked + # anyway. + # + # What is worth waiting for is real, though. That last render is a 2000x1260 + # PNG being written by another JVM, and it can still be in flight when the + # host writes the marker. So this waits for a render *of the final round*, + # which `result.json` names, and stops as soon as one exists. + round="$(result_round)" limit="${ARENA_CARD_WAIT:-2}" waited=0 - if [ -f "$marker" ]; then + if [ "$round" -gt 0 ] 2>/dev/null; then + want="$(printf 'round_%03d_' "$round")" while [ "$waited" -lt "$((limit * 4))" ]; do - [ -n "$(find "$dir" -type f -name '*.png' -newer "$marker" 2>/dev/null | head -n1)" ] && break + [ -n "$(find "$dir" -type f -name "$want*.png" 2>/dev/null | head -n1)" ] && break sleep 0.25 waited=$((waited + 1)) done @@ -285,10 +300,14 @@ collect_board() { dest="$ARENA_STATE/share-card.png" [ -f "$SENT_DIR/share-card.png" ] && return 0 cp "$last" "$dest" || return 0 - # Logged before the send, not after: which render became the board is worth - # knowing whether or not the upload landed, and it is the one thing about - # this that a bad version sort would get quietly wrong. - log "share card: $(basename "$last")" + # Logged before the send, not after, and it says whether the card is the + # final round's. Settling for an earlier one is not a failure - a match can + # end on a phase whose board was already drawn - but it is the thing a bad + # wait or a bad sort would get quietly wrong, so it is said out loud. + case "$(basename "$last")" in + "$(printf 'round_%03d_' "${round:-0}")"*) log "share card: $(basename "$last")" ;; + *) log "share card: $(basename "$last") (round $round had none)" ;; + esac # Same name in the older layout, under `shots/` rather than this folder. upload_once_as "$dest" socials share-card.png screenshots share-card.png || return 1 } diff --git a/tests/shell/test-results-watch.sh b/tests/shell/test-results-watch.sh index 3e75808..16b11a7 100755 --- a/tests/shell/test-results-watch.sh +++ b/tests/shell/test-results-watch.sh @@ -32,7 +32,7 @@ echo '{"ending": "victory", "startedAt": "2026-08-20T10:00:00Z", "endedAt": "2026-08-20T10:31:00Z", "scenario": {"name": "Fight for Farhaven", "source": "library", "library": "TrainingScenarios/1-FirstRun.mms"}, - "round": 7, "players": []}' > "$STATE/result.json" + "round": 10, "players": []}' > "$STATE/result.json" echo '{"matchId": "test-match", "players": []}' > "$STATE/identity.json" # What init/30-assets.sh staged: one camo per slot, indexed by slot. The page @@ -43,8 +43,12 @@ printf 'fake-png' > "$TMP/run/camo/1-TraineeA.png" printf 'TraineeA\t1-TraineeA.png\n' > "$TMP/run/camo/index.tsv" LOGS="$TMP/logs" mkdir -p "$LOGS/gameSummaries/board/game-uuid" "$LOGS/gameSummaries/minimap/game-uuid" -touch "$LOGS/gameSummaries/board/game-uuid/round_10_9_TARGETING.png" \ - "$LOGS/gameSummaries/board/game-uuid/round_1_9_TARGETING.png" \ +# The names MegaMek writes: round_%03d_%03d_, the phase being the one +# that just ended. It never writes one for VICTORY - BoardView.gamePhaseChange +# tests oldPhase against deployment/movement/targeting/firing/physical only - +# so the last board of any match is the phase before the end. +touch "$LOGS/gameSummaries/board/game-uuid/round_010_020_PHYSICAL.png" \ + "$LOGS/gameSummaries/board/game-uuid/round_001_009_TARGETING.png" \ "$LOGS/gameSummaries/minimap/game-uuid/round_1_9_TARGETING.png" \ "$LOGS/gameSummaries/minimap/game-uuid/game-uuid.gif" @@ -124,7 +128,7 @@ check "the merge keeps the scenario" bash -c \ # hundred megabytes a match and exactly one of them is ever read, so only the # victory board goes up; the per-phase minimaps are the GIF's own frames. check "leaves the per-phase boards behind" bash -c \ - "! grep -q 'board-round_1_9_TARGETING.png' <<<\"\$1\"" _ "$out" + "! grep -q 'board-round_001_009_TARGETING.png' <<<\"\$1\"" _ "$out" check "leaves the per-phase minimaps behind" bash -c \ "! grep -q 'minimap-round_1_9_TARGETING.png' <<<\"\$1\"" _ "$out" check "offers the summary GIF" grep -q "minimap-game-uuid.gif" <<<"$out" @@ -148,7 +152,7 @@ check "leaves the path ranker behind" bash -c \ # Version-sorted, because round_10 is later than round_2 and a plain sort says # the opposite. check "offers the share card" grep -q "share-card.png" <<<"$out" -check "the share card is the last round" grep -q "share card: round_10_" <<<"$out" +check "the share card is the final round" grep -q "share card: round_010_020_PHYSICAL" <<<"$out" # And the card goes first: headquarters draws a card as soon as it can read a # result, so the other order publishes a page whose card has no board on it. @@ -156,25 +160,32 @@ check "sends the share card before the result" bash -c \ '[ "$(grep -n "share-card.png" <<<"$1" | head -n1 | cut -d: -f1)" \ -lt "$(grep -n "result-final.json" <<<"$1" | head -n1 | cut -d: -f1)" ]' _ "$out" -# --- a final render that lands after the marker ------------------------------- -# The victory board can be a fraction of a second behind the marker. Waiting -# is what keeps the card off the round before the one that decided the match. -rm -f "$STATE/game-over" "$STATE/share-card.png" +# --- the final round's board, still being written ------------------------------ +# That last render is a 2000x1260 PNG written by another JVM and can still be +# in flight when the host writes the marker, so the wait is for the board of +# the round `result.json` names - not, as it used to be, for a render newer +# than the marker, which MegaMek never produces. +rm -f "$STATE/game-over" "$STATE/share-card.png" \ + "$LOGS/gameSummaries/board/game-uuid/round_010_020_PHYSICAL.png" rm -rf "$STATE/sent" ( sleep 1.5 - touch "$LOGS/gameSummaries/board/game-uuid/round_11_9_VICTORY.png" ) & + touch "$LOGS/gameSummaries/board/game-uuid/round_010_020_PHYSICAL.png" ) & late=$! -out="$(run_watcher)" +out="$(CARD_WAIT=4 run_watcher)" wait "$late" 2>/dev/null -check "waits for the render the marker implies" grep -q "share card: round_11_9_VICTORY" <<<"$out" +check "waits for the final round's board" \ + grep -q "share card: round_010_020_PHYSICAL" <<<"$out" -# Bounded, though: a client that never draws one must not hold the result up. +# Bounded, though: a match whose final round drew no board at all must not hold +# the result up, and what it settles for is said out loud rather than passed +# off as the picture that was asked for. rm -f "$STATE/game-over" "$STATE/share-card.png" \ - "$LOGS/gameSummaries/board/game-uuid/round_11_9_VICTORY.png" + "$LOGS/gameSummaries/board/game-uuid/round_010_020_PHYSICAL.png" rm -rf "$STATE/sent" out="$(CARD_WAIT=1 run_watcher)" -check "gives up waiting and sends anyway" grep -q "share card: round_10_" <<<"$out" +check "gives up waiting and sends anyway" grep -q "share card: round_001_009_TARGETING" <<<"$out" +check "and says the round had none" grep -q "(round 10 had none)" <<<"$out" # --- what a previous pass already got through --------------------------------- # The marker file is the whole of the contract between this and finalize: an -- 2.51.2