#47 Fix late battles referencing unrelated battles (event id collision)

[Crocatowa]

Summary

Fixes #47. Late-worldgen battles were showing an Events list full of "battle fought" events belonging to a different, much older battle, with none of their own events. This was caused by an ID collision between synthetically-generated events and real events.

Root cause

BattleFought events are generated in code (not read from XML) and were assigned Id = world.Events.Count:

Id = world?.Events.Count ?? -1;

When the legends XML contains an event type the parser doesn't recognize, that event is silently skipped (the default case in XMLParser.AddEvent adds nothing). This makes World.Events.Count smaller than maxEventId + 1.

As a result the synthetic IDs (= Count) collide with real event IDs and get appended out of order, breaking the ascending-sort invariant that World.GetEvent → GetLegendsObject relies on. Its fast path if (id < list.Count && list[id].Id == id) return list[id] then deterministically returns a synthetic BattleFought (their Id equals their list index), so when a late battle resolves its <event> children it receives battle-fought events from an unrelated earlier battle.

Early battles reference low event IDs below the collision boundary, which is why the issue only appears later in worldgen.

Fix

Assign synthetic event IDs strictly above the highest existing event ID, keeping World.Events sorted and collision-free (O(1), since events are appended in ID order):

Id = world == null
    ? -1
    : world.Events.Count > 0 ? world.Events[^1].Id + 1 : 0;

Changes

• BattleFought: assign synthetic event id above the current max instead of using Events.Count.
• BattleFoughtTests: add regression tests for the gapped-list id assignment and the sorted/collision-free invariant.

Testing

• Regression tests added
• Full dotnet test run — not run by me (no .NET SDK on my dev machine); please verify in CI / locally.

Notes for reviewers

• The fix assumes World.Events is in ascending ID order when battles are parsed, which holds today (events parse before collections, in document/ID order). I used last.Id + 1 over Events.Max(...) to keep it O(1).
• Targeted at develop to match the repo's integration flow — happy to retarget main if preferred.
• A related latent assumption exists in War.cs (world.Events.Last().Year for war length), which is out of scope here but may be worth a separate look.

Made with Cursor