ui/phase-27: root-cause aggregation of duplicate (race, className) rows

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) <noreply@anthropic.com>
2026-05-13 18:52:40 +02:00
parent 2e7478f5ea
commit bd11cd80da
9 changed files with 344 additions and 133 deletions
@@ -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)
+	}
+}
@@ -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 {