diff --git a/LegendsViewer.Backend.Tests/Legends/Events/BattleFoughtTests.cs b/LegendsViewer.Backend.Tests/Legends/Events/BattleFoughtTests.cs index 38777fb..c77ddd0 100644 --- a/LegendsViewer.Backend.Tests/Legends/Events/BattleFoughtTests.cs +++ b/LegendsViewer.Backend.Tests/Legends/Events/BattleFoughtTests.cs @@ -125,4 +125,55 @@ public void Print_WithHiredScout_ReturnsScoutString() Assert.IsTrue(result.Contains("was hired")); Assert.IsTrue(result.Contains("as a scout")); } + + [TestMethod] + public void Constructor_WhenEventListHasGaps_AssignsIdAboveHighestExistingId() + { + // Arrange: simulate unsupported event types being skipped during parsing, which leaves + // World.Events with gaps so Count (3) is lower than the highest existing Id (5). + // Regression test for issue #47 where synthetic Ids collided with real event Ids. + var existingEvents = new List + { + new(new List(), _mockWorld.Object) { Id = 0 }, + new(new List(), _mockWorld.Object) { Id = 1 }, + new(new List(), _mockWorld.Object) { Id = 5 }, + }; + _mockWorld.Setup(w => w.Events).Returns(existingEvents); + + // Act + var battleFought = new BattleFought(_historicalFigure, _battle, _mockWorld.Object, asAttacker: true); + + // Assert: must be highest Id + 1 (6), NOT the list Count (3), to avoid colliding with Id 5. + Assert.AreEqual(6, battleFought.Id); + } + + [TestMethod] + public void Constructor_MultipleSyntheticEvents_RemainSortedAndCollisionFree() + { + // Arrange: a gapped event list (Count = 3, highest Id = 9) plus a caller that appends each + // synthetic event like Battle does. The resulting list must stay strictly ascending so + // World.GetEvent's binary search keeps resolving events correctly. + var events = new List + { + new(new List(), _mockWorld.Object) { Id = 0 }, + new(new List(), _mockWorld.Object) { Id = 4 }, + new(new List(), _mockWorld.Object) { Id = 9 }, + }; + _mockWorld.Setup(w => w.Events).Returns(events); + + // Act: create and append three synthetic events, mirroring Battle's construction loop. + for (int i = 0; i < 3; i++) + { + var bf = new BattleFought(_historicalFigure, _battle, _mockWorld.Object, asAttacker: true); + events.Add(bf); + } + + // Assert: ids strictly ascending and unique across the whole list. + for (int i = 1; i < events.Count; i++) + { + Assert.IsTrue(events[i].Id > events[i - 1].Id, + $"Event at index {i} (Id {events[i].Id}) must be greater than previous (Id {events[i - 1].Id})."); + } + Assert.AreEqual(events.Count, events.Select(e => e.Id).Distinct().Count(), "Event Ids must be unique."); + } } diff --git a/LegendsViewer.Backend/Legends/Events/IncidentalEvents/BattleFought.cs b/LegendsViewer.Backend/Legends/Events/IncidentalEvents/BattleFought.cs index cab8ba7..f006b40 100644 --- a/LegendsViewer.Backend/Legends/Events/IncidentalEvents/BattleFought.cs +++ b/LegendsViewer.Backend/Legends/Events/IncidentalEvents/BattleFought.cs @@ -19,7 +19,17 @@ public class BattleFought : WorldEvent public BattleFought(HistoricalFigure hf, Battle battle, IWorld? world, bool asAttacker, bool wasHired = false, bool asScout = false) : base([], world) { - Id = world?.Events.Count ?? -1; + // Synthetic events must get an Id strictly greater than any existing event Id so that + // World.Events stays sorted and collision-free. World.GetEvent relies on a binary search + // (with a dense-index shortcut) that breaks on duplicate or out-of-order Ids. + // Using Events.Count fails whenever the XML contained unsupported event types that were + // skipped while parsing: Count then drops below the highest real Id, so synthetic Ids + // collide with real ones and lookups resolve to the wrong event. This is what caused late + // battles to display "battle fought" events belonging to unrelated, much older battles + // (issue #47). Appending with "last Id + 1" keeps the invariant intact in O(1). + Id = world == null + ? -1 + : world.Events.Count > 0 ? world.Events[^1].Id + 1 : 0; Type = "battle fought"; Year = battle.StartYear; Seconds72 = battle.StartSeconds72;