From bd11cd80da65819d415c4f6ac8ca0c6c75e51aff Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Wed, 13 May 2026 18:52:40 +0200 Subject: [PATCH] ui/phase-27: root-cause aggregation of duplicate (race, className) rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Legacy reports list the same `(race, className)` pair across several roster rows; the engine likewise creates one ShipGroup per arrival. Both the legacy parser and `TransformBattle` were keyed on shipClass without summing — only the last row / group's counts survived, so a protocol's destroy count appeared to exceed the recorded initial roster. The UI worked around this with phantom-frame logic. Both parser and engine now SUM `Number`/`NumberLeft` across rows / groups sharing the same class; the phantom-frame workaround is gone. KNNTS041 turn 41 planet #7 reconciles: `Nails:pup` 1168 initial − 86 survivors = 1082 destroys. The engine's previously latent nil-map write on `bg.Tech` (would have paniced on any group with non-empty Tech) is fixed in the same patch — it blocked the aggregation regression test. Co-Authored-By: Claude Opus 4.7 (1M context) --- game/internal/controller/battle_test.go | 87 +++++++++++++ game/internal/controller/battle_transform.go | 34 ++++- tools/local-dev/legacy-report/parser.go | 24 ++++ tools/local-dev/legacy-report/parser_test.go | 100 ++++++++++++++ tools/local-dev/reports/dg/KNNTS041.json | 122 +++++++++--------- ui/docs/battle-viewer-ux.md | 42 +++--- .../lib/battle-player/battle-viewer.svelte | 19 --- ui/frontend/src/lib/battle-player/timeline.ts | 40 +++--- ui/frontend/tests/battle-player.test.ts | 9 -- 9 files changed, 344 insertions(+), 133 deletions(-) diff --git a/game/internal/controller/battle_test.go b/game/internal/controller/battle_test.go index 9518c0c..61ffeee 100644 --- a/game/internal/controller/battle_test.go +++ b/game/internal/controller/battle_test.go @@ -8,6 +8,7 @@ import ( "galaxy/calc" "galaxy/game/internal/controller" "galaxy/game/internal/model/game" + "galaxy/model/report" "github.com/stretchr/testify/assert" ) @@ -184,3 +185,89 @@ func TestProduceBattles(t *testing.T) { assert.Zero(t, c.ShipGroup(3).Number) } } + +// TestTransformBattleAggregatesSameShipClass guards against the +// engine-side variant of the duplicate-class bug. Several ShipGroups +// of the same ShipClass.ID can take part in the same battle (arrivals +// from different planets, tech splits, etc.); they must collapse into +// a single BattleReportGroup with summed Number and NumberLeft. The +// pre-fix engine cached the first group's index and silently dropped +// every subsequent group's initial / survivor counts, which manifested +// downstream as more Destroyed shots in the protocol than the +// recorded initial roster could account for. +func TestTransformBattleAggregatesSameShipClass(t *testing.T) { + c, g := newCache() + + assert.NoError(t, g.RaceRelation(Race_0.Name, Race_1.Name, game.RelationWar.String())) + assert.NoError(t, g.RaceRelation(Race_1.Name, Race_0.Name, game.RelationWar.String())) + + // Two Race_0 groups of the SAME ship class (Race_0_Gunship) plus + // one Race_1 group of Race_1_Gunship — all parked on Planet_0 + // (owned by Race_0; the Race_1 group lands there via the Unsafe + // helper that bypasses the ownership check). Group indices land + // at 0, 1, 2 in creation order. + assert.NoError(t, c.CreateShips(Race_0_idx, Race_0_Gunship, R0_Planet_0_num, 10)) + assert.NoError(t, c.CreateShips(Race_0_idx, Race_0_Gunship, R0_Planet_0_num, 10)) + c.CreateShipsUnsafe_T(Race_1_idx, c.MustShipClass(Race_1_idx, Race_1_Gunship).ID, R0_Planet_0_num, 5) + + // Simulate post-battle survivor counts: Group 0 ended the battle + // with 8 ships, Group 1 with 6. The aggregated BattleReportGroup + // must report NumberLeft = 8 + 6 = 14 (not just the last cached + // group's 6 — that's the regression). + c.ShipGroup(0).Number = 8 + c.ShipGroup(1).Number = 6 + + b := &controller.Battle{ + Planet: R0_Planet_0_num, + ObserverGroups: map[int]bool{0: true, 1: true, 2: true}, + InitialNumbers: map[int]uint{0: 10, 1: 10, 2: 5}, + // Protocol must reference every in-battle group at least once + // (otherwise TransformBattle won't register it through the + // `ship()` path). Two shots from Race_1 against each Race_0 + // group hits both groupIds. + Protocol: []controller.BattleAction{ + {Attacker: 2, Defender: 0, Destroyed: true}, + {Attacker: 2, Defender: 1, Destroyed: true}, + }, + } + + r := controller.TransformBattle(c, b) + + // Two BattleReportGroup entries total: one merged Race_0_Gunship + // (groups 0 + 1) and one Race_1_Gunship. NOT three. + if got, want := len(r.Ships), 2; got != want { + t.Fatalf("len(r.Ships) = %d, want %d (duplicate ShipClass.ID must merge)", got, want) + } + + var gunship0, gunship1 *report.BattleReportGroup + for i := range r.Ships { + grp := r.Ships[i] + switch grp.Race { + case Race_0.Name: + gunship0 = &grp + case Race_1.Name: + gunship1 = &grp + } + } + if gunship0 == nil || gunship1 == nil { + t.Fatalf("missing race entry: race0=%v race1=%v", gunship0, gunship1) + } + + if gunship0.ClassName != Race_0_Gunship { + t.Errorf("race0.ClassName = %q, want %q", gunship0.ClassName, Race_0_Gunship) + } + if gunship0.Number != 20 { + t.Errorf("race0.Number = %d, want 20 (10+10)", gunship0.Number) + } + if gunship0.NumberLeft != 14 { + t.Errorf("race0.NumberLeft = %d, want 14 (8+6)", gunship0.NumberLeft) + } + if !gunship0.InBattle { + t.Errorf("race0.InBattle = false, want true (both source groups were in-battle)") + } + + if gunship1.Number != 5 || gunship1.NumberLeft != 5 { + t.Errorf("race1 = (Number=%d, NumberLeft=%d), want (5, 5)", + gunship1.Number, gunship1.NumberLeft) + } +} diff --git a/game/internal/controller/battle_transform.go b/game/internal/controller/battle_transform.go index 9df3041..ebd8714 100644 --- a/game/internal/controller/battle_transform.go +++ b/game/internal/controller/battle_transform.go @@ -18,10 +18,35 @@ func TransformBattle(c *Cache, b *Battle) *report.BattleReport { cacheShipClass := make(map[uuid.UUID]int) cacheRaceName := make(map[uuid.UUID]int) + processedGroup := make(map[int]bool) addShipGroup := func(groupId int, inBattle bool) int { shipClass := c.ShipGroupShipClass(groupId) sg := c.ShipGroup(groupId) + // Several ship-groups of the same race/class can take part + // in the same battle (different tech upgrades, arrivals from + // different planets, …). They share a single + // BattleReportGroup entry keyed by ShipClass.ID — when a + // later group lands on a cached class we add its Number and + // NumberLeft into the existing entry instead of dropping + // them, so the protocol's per-class destroy counts reconcile + // with the recorded totals. `processedGroup` guards against + // double-counting a single groupId across multiple shots in + // the protocol — `ship()` runs on every attacker and defender + // reference, the merge must happen once per groupId. + if existing, ok := cacheShipClass[shipClass.ID]; ok { + if !processedGroup[groupId] { + bg := r.Ships[existing] + bg.Number += b.InitialNumbers[groupId] + bg.NumberLeft += sg.Number + if inBattle { + bg.InBattle = true + } + r.Ships[existing] = bg + processedGroup[groupId] = true + } + return existing + } itemNumber := len(r.Ships) bg := &report.BattleReportGroup{ Race: c.g.Race[c.RaceIndex(sg.OwnerID)].Name, @@ -31,22 +56,19 @@ func TransformBattle(c *Cache, b *Battle) *report.BattleReport { ClassName: shipClass.Name, LoadType: sg.CargoString(), LoadQuantity: report.F(sg.Load.F()), + Tech: make(map[string]report.Float, len(sg.Tech)), } for t, v := range sg.Tech { bg.Tech[t.String()] = report.F(v.F()) } r.Ships[itemNumber] = *bg cacheShipClass[shipClass.ID] = itemNumber + processedGroup[groupId] = true return itemNumber } ship := func(groupId int) int { - shipClass := c.ShipGroupShipClass(groupId) - if v, ok := cacheShipClass[shipClass.ID]; ok { - return v - } else { - return addShipGroup(groupId, true) - } + return addShipGroup(groupId, true) } race := func(groupId int) int { diff --git a/tools/local-dev/legacy-report/parser.go b/tools/local-dev/legacy-report/parser.go index 29e9404..ea99025 100644 --- a/tools/local-dev/legacy-report/parser.go +++ b/tools/local-dev/legacy-report/parser.go @@ -814,6 +814,30 @@ func (p *parser) parseBattleRosterRow(fields []string) { key := shipKey{race: p.pendingBattleRace, class: className} idx := p.assignShipIndex(key) + // Legacy battle rosters may list the same `(race, className)` + // across multiple rows — different tech variants, ships pulled + // from several stacks / planets, etc. We collapse those rows + // into one BattleReportGroup keyed by `(race, className)` (the + // viewer aggregates per class anyway) by SUMMING Number and + // NumberLeft instead of overwriting; otherwise only the last + // row's counts survive and the battle protocol's destroy count + // would dwarf the recorded initial count (the original + // motivation for the now-removed "phantom destroy" workaround). + if existing, found := p.pendingBattle.ships[idx]; found { + existing.Number += uint(number) + existing.NumberLeft += uint(numLeft) + // LoadQuantity is per-ship cargo — average is a fair fallback + // when several stacks of the same class merge into one bucket. + existing.LoadQuantity = report.F( + (existing.LoadQuantity.F() + loadQuantity) / 2, + ) + // Tech / LoadType / InBattle keep their first-seen values: + // the viewer treats them as bucket-wide attributes and the + // first row is normally the most representative tech variant. + p.pendingBattle.ships[idx] = existing + return + } + p.pendingBattle.ships[idx] = report.BattleReportGroup{ Race: p.pendingBattleRace, ClassName: className, diff --git a/tools/local-dev/legacy-report/parser_test.go b/tools/local-dev/legacy-report/parser_test.go index b4d14b3..e499a3a 100644 --- a/tools/local-dev/legacy-report/parser_test.go +++ b/tools/local-dev/legacy-report/parser_test.go @@ -533,6 +533,106 @@ func TestParseBattles(t *testing.T) { } } +// TestParseBattleAggregatesDuplicateClasses guards against the +// regression that produced the original "phantom destroys" symptom: +// the same `(race, className)` pair appearing on multiple roster +// rows must collapse into a single BattleReportGroup whose `Number` +// (the "#" column, initial ship count) and `NumberLeft` (the "L" +// column, survivors) are the sums across rows. Without the +// aggregation only the last row's counts survived and the protocol's +// destroy count dwarfed the recorded initial count (e.g. KNNTS041 +// turn-41 planet #7 lists `pup` seven separate times: 99 + 105 + 291 + +// 287 + 166 + 132 + 88 = 1168 ships, 86 survivors, 1082 destroys). +func TestParseBattleAggregatesDuplicateClasses(t *testing.T) { + in := strings.Join([]string{ + "Race Report for Galaxy PLUS Turn 1", + "", + "Battle at (#7) B-007", + "", + "Foo Groups", + "", + " # T D W S C T Q L", + " 3 Drone 1.0 1.0 0 0 - 0 1 In_Battle", + " 4 Drone 1.2 1.0 0 0 - 0 2 In_Battle", + "10 Cruiser 3.0 2.0 0 0 - 0 9 In_Battle", + "", + "Bar Groups", + "", + "# T D W S C T Q L", + "5 Pistolet 1.0 1.0 0 0 - 0 3 In_Battle", + "", + "Battle Protocol", + "", + "Bar Pistolet fires on Foo Drone : Destroyed", + "Bar Pistolet fires on Foo Drone : Destroyed", + "Bar Pistolet fires on Foo Drone : Destroyed", + "Bar Pistolet fires on Foo Drone : Destroyed", + "", + }, "\n") + + rep, battles, err := Parse(strings.NewReader(in)) + if err != nil { + t.Fatalf("Parse: %v", err) + } + if got, want := len(battles), 1; got != want { + t.Fatalf("len(battles) = %d, want %d", got, want) + } + b := battles[0] + + // The three Foo roster rows collapse into two BattleReportGroup + // entries: one Foo:Drone (rows 1+2) and one Foo:Cruiser (row 3), + // plus one Bar:Pistolet. Total 3 groups, NOT 4. + if got, want := len(b.Ships), 3; got != want { + t.Fatalf("battle.Ships = %d groups, want %d (duplicate class rows must merge)", got, want) + } + + var drone, cruiser, pistolet *report.BattleReportGroup + for i := range b.Ships { + g := b.Ships[i] + switch g.ClassName { + case "Drone": + drone = &g + case "Cruiser": + cruiser = &g + case "Pistolet": + pistolet = &g + } + } + if drone == nil || cruiser == nil || pistolet == nil { + t.Fatalf("missing class: drone=%v cruiser=%v pistolet=%v", drone, cruiser, pistolet) + } + + // Drone rows sum to Number = 3 + 4 = 7 and NumberLeft = 1 + 2 = 3. + // Protocol corroborates: four Destroyed shots against Drone, so + // 7 - 3 = 4 — the protocol's destroy count reconciles with the + // recorded delta only when both rows are summed. + if drone.Number != 7 { + t.Errorf("Drone.Number = %d, want 7 (3+4)", drone.Number) + } + if drone.NumberLeft != 3 { + t.Errorf("Drone.NumberLeft = %d, want 3 (1+2)", drone.NumberLeft) + } + // Cruiser and Pistolet are single-row classes — counts must match + // the file verbatim with no spurious merging across classes. + if cruiser.Number != 10 || cruiser.NumberLeft != 9 { + t.Errorf("Cruiser = (Number=%d, NumberLeft=%d), want (10, 9)", + cruiser.Number, cruiser.NumberLeft) + } + if pistolet.Number != 5 || pistolet.NumberLeft != 3 { + t.Errorf("Pistolet = (Number=%d, NumberLeft=%d), want (5, 3)", + pistolet.Number, pistolet.NumberLeft) + } + + // rep-level summary must reflect the merged shape: 4 shots, one + // battle, no crash or spurious extra battles. + if got, want := len(rep.Battle), 1; got != want { + t.Fatalf("len(rep.Battle) = %d, want %d", got, want) + } + if rep.Battle[0].Shots != 4 { + t.Errorf("rep.Battle[0].Shots = %d, want 4", rep.Battle[0].Shots) + } +} + // TestParseYourGroups exercises the local-group section. Two rows // cover the on-planet ("In_Orbit", origin "-") and in-space ("In_Space", // origin name + range) variants, plus a cargo-loaded row to assert the diff --git a/tools/local-dev/reports/dg/KNNTS041.json b/tools/local-dev/reports/dg/KNNTS041.json index a5abc4e..f5c5eb5 100644 --- a/tools/local-dev/reports/dg/KNNTS041.json +++ b/tools/local-dev/reports/dg/KNNTS041.json @@ -14743,10 +14743,10 @@ "race": "KnightErrants", "className": "PeaceShip", "tech": { - "DRIVE": 9.1 + "DRIVE": 9.09 }, - "num": 50, - "numLeft": 50, + "num": 52, + "numLeft": 52, "loadType": "", "loadQuantity": 0, "inBattle": true @@ -15498,11 +15498,11 @@ "race": "Frightners", "className": "moan", "tech": { - "DRIVE": 7.79, - "SHIELDS": 5.15 + "DRIVE": 7.5, + "SHIELDS": 4.85 }, - "num": 85, - "numLeft": 85, + "num": 259, + "numLeft": 259, "loadType": "", "loadQuantity": 0, "inBattle": true @@ -15988,13 +15988,13 @@ "race": "Slimes", "className": "Fly_1", "tech": { - "DRIVE": 5.79 + "DRIVE": 6.02 }, - "num": 105, - "numLeft": 105, + "num": 348, + "numLeft": 348, "loadType": "", "loadQuantity": 0, - "inBattle": true + "inBattle": false }, "6": { "race": "Slimes", @@ -16325,10 +16325,10 @@ "race": "KnightErrants", "className": "PeaceShip", "tech": { - "DRIVE": 9.1 + "DRIVE": 10.62 }, - "num": 99, - "numLeft": 99, + "num": 100, + "numLeft": 100, "loadType": "", "loadQuantity": 0, "inBattle": true @@ -16497,10 +16497,10 @@ "race": "Enoxes", "className": "Gnat", "tech": { - "DRIVE": 11.4 + "DRIVE": 9.07 }, - "num": 26, - "numLeft": 26, + "num": 167, + "numLeft": 167, "loadType": "", "loadQuantity": 0, "inBattle": true @@ -18583,8 +18583,8 @@ "tech": { "DRIVE": 5.16 }, - "num": 19, - "numLeft": 19, + "num": 1026, + "numLeft": 1026, "loadType": "", "loadQuantity": 0, "inBattle": true @@ -19025,10 +19025,10 @@ "CARGO": 1.1, "DRIVE": 10.85 }, - "num": 1, + "num": 3, "numLeft": 0, "loadType": "COL", - "loadQuantity": 1.4, + "loadQuantity": 1.855, "inBattle": true }, "12": { @@ -19140,9 +19140,9 @@ "race": "Koreans", "className": "d", "tech": { - "DRIVE": 9.87 + "DRIVE": 2.4 }, - "num": 112, + "num": 113, "numLeft": 0, "loadType": "", "loadQuantity": 0, @@ -29824,8 +29824,8 @@ "SHIELDS": 3.19, "WEAPONS": 3.97 }, - "num": 1, - "numLeft": 1, + "num": 2, + "numLeft": 2, "loadType": "", "loadQuantity": 0, "inBattle": true @@ -29893,10 +29893,10 @@ "race": "Nails", "className": "pup", "tech": { - "DRIVE": 4.98 + "DRIVE": 4.97 }, - "num": 88, - "numLeft": 6, + "num": 1168, + "numLeft": 86, "loadType": "", "loadQuantity": 0, "inBattle": true @@ -46450,10 +46450,10 @@ "race": "Ricksha", "className": "Dron", "tech": { - "DRIVE": 7.63 + "DRIVE": 3.2 }, - "num": 647, - "numLeft": 647, + "num": 1211, + "numLeft": 1211, "loadType": "", "loadQuantity": 0, "inBattle": true @@ -46462,11 +46462,11 @@ "race": "Ricksha", "className": "HDron", "tech": { - "DRIVE": 7.63, + "DRIVE": 6.88, "SHIELDS": 3.95 }, - "num": 88, - "numLeft": 88, + "num": 112, + "numLeft": 112, "loadType": "", "loadQuantity": 0, "inBattle": true @@ -46690,10 +46690,10 @@ "race": "KnightErrants", "className": "PeaceShip", "tech": { - "DRIVE": 9.09 + "DRIVE": 9.1 }, - "num": 2, - "numLeft": 2, + "num": 164, + "numLeft": 163, "loadType": "", "loadQuantity": 0, "inBattle": true @@ -48389,10 +48389,10 @@ "className": "FS-6", "tech": { "DRIVE": 11.4, - "SHIELDS": 5.64 + "SHIELDS": 5.1 }, - "num": 48, - "numLeft": 48, + "num": 72, + "numLeft": 72, "loadType": "", "loadQuantity": 0, "inBattle": true @@ -48449,10 +48449,10 @@ "race": "Enoxes", "className": "Gnat", "tech": { - "DRIVE": 11.4 + "DRIVE": 8.4 }, - "num": 100, - "numLeft": 100, + "num": 101, + "numLeft": 101, "loadType": "", "loadQuantity": 0, "inBattle": true @@ -48466,10 +48466,10 @@ "SHIELDS": 2.13, "WEAPONS": 4.22 }, - "num": 1, - "numLeft": 1, + "num": 2, + "numLeft": 2, "loadType": "COL", - "loadQuantity": 5.73, + "loadQuantity": 5.375, "inBattle": true }, "8": { @@ -48479,10 +48479,10 @@ "CARGO": 1, "DRIVE": 11.4, "SHIELDS": 5.1, - "WEAPONS": 5.77 + "WEAPONS": 5.44 }, - "num": 1, - "numLeft": 1, + "num": 2, + "numLeft": 2, "loadType": "COL", "loadQuantity": 1.05, "inBattle": true @@ -48539,10 +48539,10 @@ "race": "Flagist", "className": "Drone", "tech": { - "DRIVE": 8.49 + "DRIVE": 6.08 }, - "num": 1, - "numLeft": 1, + "num": 2, + "numLeft": 2, "loadType": "", "loadQuantity": 0, "inBattle": false @@ -48587,10 +48587,10 @@ "race": "Enoxes", "className": "Gnat", "tech": { - "DRIVE": 11.4 + "DRIVE": 9.07 }, - "num": 49, - "numLeft": 49, + "num": 50, + "numLeft": 50, "loadType": "", "loadQuantity": 0, "inBattle": true @@ -48662,10 +48662,10 @@ "race": "KnightErrants", "className": "PeaceShip", "tech": { - "DRIVE": 9.09 + "DRIVE": 8.71 }, - "num": 1, - "numLeft": 1, + "num": 63, + "numLeft": 63, "loadType": "", "loadQuantity": 0, "inBattle": true @@ -48918,10 +48918,10 @@ "race": "KnightErrants", "className": "PeaceShip", "tech": { - "DRIVE": 8.71 + "DRIVE": 5.6 }, - "num": 1, - "numLeft": 1, + "num": 158, + "numLeft": 158, "loadType": "", "loadQuantity": 0, "inBattle": true diff --git a/ui/docs/battle-viewer-ux.md b/ui/docs/battle-viewer-ux.md index 93f8773..d02bc79 100644 --- a/ui/docs/battle-viewer-ux.md +++ b/ui/docs/battle-viewer-ux.md @@ -132,23 +132,35 @@ on the same defender therefore look like two distinct pulses rather than one continuous line. On pause the line and flash stay drawn so the user can study the current shot. -## Phantom destroys +## Aggregated ship-class buckets -Legacy emitters (the `dg` engine format that feeds the synthetic- -report path) occasionally log more `Destroyed` (and `Shields`) -lines against a ship-group bucket than the bucket's initial -population — the emitter keeps recording hits past the moment a -group emptied. `buildFrames` marks every such frame as -`phantom: true` and skips the race-total decrement so the race -stays on the scene until its actual ships are gone. +Real legacy reports list the same `(race, className)` pair across +several roster rows — different tech variants, ships pulled from +multiple stacks or planets. The legacy-report parser +([parser.go](../../tools/local-dev/legacy-report/parser.go)) +collapses those rows into a single `BattleReportGroup` keyed by +`(race, className)` by SUMMING `Number` and `NumberLeft`; the +engine's `TransformBattle` +([battle_transform.go](../../game/internal/controller/battle_transform.go)) +applies the same merge keyed by `ShipClass.ID`, guarded by a +processed-group set so the same source `groupId` is not summed +twice across multiple protocol references. -During play the BattleViewer fast-forwards through streaks of -phantom frames via a 0 ms timer so the user never sees a silent -gap (KNNTS041 had ~30 phantom frames between shots 224 and 255 -right after the last `Nails:pup` died). Step controls and the -scrubber can still land on a phantom frame deliberately — useful -when inspecting the protocol entry that the engine emitted into -the void. +Without this aggregation only the last roster row's counts +survived, and the protocol's destroy count against the class +would dwarf the recorded initial count — KNNTS041 turn 41 planet +\#7 had 7 separate `Nails:pup` rows totalling 1168 ships; the +buggy parser stored only the last row's 88, so the 1082 destroys +in the protocol looked like phantom hits past the empty bucket. +After the fix both sides reconcile: 1168 initial − 86 survivors = +1082 destroys. + +`buildFrames` +([timeline.ts](../src/lib/battle-player/timeline.ts)) keeps a +defence-in-depth clamp `if (left > 0)` on the destroy decrement so +a malformed protocol never pushes a race below zero; in normal +operation the clamp is a no-op because parser + engine already +folded duplicate rows together. ## Final-frame freeze diff --git a/ui/frontend/src/lib/battle-player/battle-viewer.svelte b/ui/frontend/src/lib/battle-player/battle-viewer.svelte index e1704d6..faae3ff 100644 --- a/ui/frontend/src/lib/battle-player/battle-viewer.svelte +++ b/ui/frontend/src/lib/battle-player/battle-viewer.svelte @@ -67,7 +67,6 @@ matching `pkg/model/report/battle.go` and it plays back. return { shotIndex: cur.shotIndex, lastAction: cur.lastAction, - phantom: cur.phantom, remaining: prev.remaining, activeRaceIds: prev.activeRaceIds, }; @@ -79,29 +78,11 @@ matching `pkg/model/report/battle.go` and it plays back. // 10 % of the frame's interval, then advance. Effect re-arms // whenever frameIndex / playing / speed changes; previous // timers clean up through the return. - // - // A phantom frame (shot against an already-empty defender) - // would otherwise hold the scene silent for the full interval. - // During play we fast-forward to the next non-phantom frame - // through a 0 ms timer, so streaks of phantoms (KNNTS041 - // frames 225..255, 401..414, …) collapse into a single tick - // from the user's POV. $effect(() => { void frameIndex; void speed; shotVisible = true; if (!playing) return; - if (rawFrame.phantom && frameIndex < frames.length - 1) { - let next = frameIndex + 1; - while (next < frames.length - 1 && frames[next].phantom) { - next++; - } - const target = next; - const skip = setTimeout(() => { - frameIndex = target; - }, 0); - return () => clearTimeout(skip); - } const intervalMs = 400 / speed; const blinkOff = setTimeout(() => { shotVisible = false; diff --git a/ui/frontend/src/lib/battle-player/timeline.ts b/ui/frontend/src/lib/battle-player/timeline.ts index 63dfe1c..9aceaed 100644 --- a/ui/frontend/src/lib/battle-player/timeline.ts +++ b/ui/frontend/src/lib/battle-player/timeline.ts @@ -19,19 +19,12 @@ import type { * `BattleReport.ships`; `activeRaceIds` are the race indices with at * least one surviving in-battle group. `lastAction` is the action * applied to produce this frame, or `null` for the initial frame. - * `phantom` is true when the action's defender ship-group was - * already at zero before the action ran — legacy emitters keep - * logging hits past the moment a group emptied. The viewer fast- - * forwards through phantom frames during play so the user never - * sees a silent gap; the frame is still in the sequence so step - * controls and the scrubber can land on it deliberately. */ export interface Frame { shotIndex: number; remaining: Map; activeRaceIds: number[]; lastAction: BattleActionReport | null; - phantom: boolean; } export interface NormalisedGroup { @@ -102,7 +95,6 @@ export function buildFrames(report: BattleReport): Frame[] { remaining: new Map(initialRemaining), activeRaceIds: collectActiveRaces(raceTotals), lastAction: null, - phantom: false, }); const groupRaceByKey = new Map(); @@ -112,20 +104,23 @@ export function buildFrames(report: BattleReport): Frame[] { const runningRaceTotals = new Map(raceTotals); for (let i = 0; i < report.protocol.length; i++) { const action = report.protocol[i]; - // A shot whose defender group was empty before the action - // ran is a phantom: legacy emitters keep logging hits past - // the moment a group emptied. We keep the frame in the - // sequence (step controls and the scrubber can still land - // on it deliberately) but mark it so the play loop fast- - // forwards across the silent gap. - const leftBefore = current.get(action.sd) ?? 0; - const phantom = leftBefore === 0; - if (action.x && !phantom) { - current.set(action.sd, leftBefore - 1); - const raceId = groupRaceByKey.get(action.sd); - if (raceId !== undefined) { - const t = (runningRaceTotals.get(raceId) ?? 0) - 1; - runningRaceTotals.set(raceId, Math.max(0, t)); + if (action.x) { + // Defence in depth: a malformed protocol that fires more + // `Destroyed` rows than the group has ships would push + // `runningRaceTotals` below zero and drop the race from + // `activeRaceIds` prematurely. Real legacy data folds + // duplicate `(race, className)` roster rows into the + // same `BattleReportGroup` (parser + engine), so this + // branch is hit only on a real shrink — but the clamp + // keeps the math from going negative either way. + const left = current.get(action.sd) ?? 0; + if (left > 0) { + current.set(action.sd, left - 1); + const raceId = groupRaceByKey.get(action.sd); + if (raceId !== undefined) { + const t = (runningRaceTotals.get(raceId) ?? 0) - 1; + runningRaceTotals.set(raceId, Math.max(0, t)); + } } } frames.push({ @@ -133,7 +128,6 @@ export function buildFrames(report: BattleReport): Frame[] { remaining: new Map(current), activeRaceIds: collectActiveRaces(runningRaceTotals), lastAction: action, - phantom, }); } diff --git a/ui/frontend/tests/battle-player.test.ts b/ui/frontend/tests/battle-player.test.ts index b5af86e..c735c05 100644 --- a/ui/frontend/tests/battle-player.test.ts +++ b/ui/frontend/tests/battle-player.test.ts @@ -208,15 +208,6 @@ describe("buildFrames phantom-destroy clamp", () => { // the only active race for the remainder of the protocol. expect(frames[5].remaining.get(10)).toBe(0); expect(frames[5].activeRaceIds).toEqual([1]); - // Phantom flags: first two destroys land on a non-empty - // group → real shots; the remaining three are phantoms. - expect(frames[1].phantom).toBe(false); - expect(frames[2].phantom).toBe(false); - expect(frames[3].phantom).toBe(true); - expect(frames[4].phantom).toBe(true); - expect(frames[5].phantom).toBe(true); - // The initial frame is never a phantom. - expect(frames[0].phantom).toBe(false); }); it("keeps a race active while phantom destroys hit one of its empty groups", () => {