From d4fbcb19872abfc2ab92ab1dd1bb1cbfa3c0806f Mon Sep 17 00:00:00 2001
From: Dan Crosby <dan@danielcrosby.net>
Date: Thu, 9 Apr 2026 21:29:10 -0700
Subject: [PATCH] Swap models when available provinces change, even
 mid-selection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two related bugs in EagleGameController could leave the UI pointing
at provinces that the model no longer considers actionable:

1. SwapModel's early return at the "still have commands" check skipped
   the swap unconditionally whenever both the old and new model had
   any commands. When a refresh arrived after a phase advance, the
   controllers (mapController.SelectedProvinceId, _commandPanelController,
   mapController.ProvincesWithCommands, etc.) kept pointing at the old
   provinces, but _currentModel.CommandToken in the model layer had
   already advanced. A subsequent commit then posted a stale province
   with the freshly-advanced token, producing
   "Attempted to select province N out of TreeSet(...)" on the server.

   Tighten the early return to only skip when the available province
   set is identical between the two models. If the set has changed,
   we must swap so stale UI references get cleared.

2. NextActiveProvinceKeyPair used .First(predicate) to look up the
   suggested province, which throws InvalidOperationException when
   no element matches. This can happen when SuggestedProvinceId lags
   AvailableCommandsByProvince by a refresh, and the throw aborts
   SwapModel partway through — leaving a half-updated UI behind.
   Switch to TryGetValue with a fallback to the first available
   province.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 .../Assets/Eagle/EagleGameController.cs       | 49 +++++++++++++++----
 1 file changed, 40 insertions(+), 9 deletions(-)

diff --git a/src/main/csharp/net/eagle0/clients/unity/eagle0/Assets/Eagle/EagleGameController.cs b/src/main/csharp/net/eagle0/clients/unity/eagle0/Assets/Eagle/EagleGameController.cs
index 7829f4c3f9..1e465766c1 100644
--- a/src/main/csharp/net/eagle0/clients/unity/eagle0/Assets/Eagle/EagleGameController.cs
+++ b/src/main/csharp/net/eagle0/clients/unity/eagle0/Assets/Eagle/EagleGameController.cs
@@ -125,11 +125,24 @@ namespace eagle {
 
         public GameObject shardokContainer;
 
-        private KeyValuePair<ProvinceId, OneProvinceAvailableCommands>? NextActiveProvinceKeyPair =>
-                Model == null || Model.AvailableCommandsByProvince.Count == 0
-                        ? null
-                        : Model.AvailableCommandsByProvince.First(
-                                  kvp => kvp.Key == Model.SuggestedProvinceId);
+        private KeyValuePair<ProvinceId, OneProvinceAvailableCommands>? NextActiveProvinceKeyPair {
+            get {
+                if (Model == null || Model.AvailableCommandsByProvince.Count == 0) { return null; }
+
+                // If the server's suggested province is still actionable, prefer it.
+                // Otherwise fall back to the first available province rather than throwing —
+                // a stale SuggestedProvinceId can lag a turn behind AvailableCommandsByProvince
+                // after a refresh, and an exception here aborts SwapModel partway through.
+                if (Model.AvailableCommandsByProvince.TryGetValue(
+                            Model.SuggestedProvinceId,
+                            out var suggested)) {
+                    return new KeyValuePair<ProvinceId, OneProvinceAvailableCommands>(
+                            Model.SuggestedProvinceId,
+                            suggested);
+                }
+                return Model.AvailableCommandsByProvince.First();
+            }
+        }
 
         void Start() {
             gameIdButton.GetComponentInChildren<Text>().text = "";
@@ -576,11 +589,19 @@ namespace eagle {
                 chronicleCanvasController.Entries = _newModel.ChronicleEntries;
             }
 
-            // If we already had commands, and we still have commands, don't actually swap the
-            // model. But always update battle effects — battles can start or end between
-            // command updates, and skipping cleanup leaves stale animations on the map.
+            // If we already had commands, and we still have commands FOR THE SAME SET OF
+            // PROVINCES, don't actually swap the model — keep the user's selection state
+            // intact. If the available province set has changed, we MUST swap, otherwise
+            // the controllers (mapController.SelectedProvinceId, _commandPanelController,
+            // mapController.ProvincesWithCommands, etc.) keep pointing at provinces that
+            // are no longer in the model, and a subsequent commit posts a stale province
+            // with the freshly-advanced token.
+            //
+            // Always update battle effects — battles can start or end between command
+            // updates, and skipping cleanup leaves stale animations on the map.
             if (Model != null && _newModel != null && _newModel.AvailableCommandsByProvince.Any() &&
-                Model.AvailableCommandsByProvince.Any()) {
+                Model.AvailableCommandsByProvince.Any() &&
+                SameAvailableProvinces(Model, _newModel)) {
                 mapController.UpdateBattleEffects(_newModel);
                 UpdateBattleProgressDisplay(_newModel);
                 return;
@@ -757,6 +778,16 @@ namespace eagle {
             }
         }
 
+        private static bool SameAvailableProvinces(IGameModel a, IGameModel b) {
+            if (a.AvailableCommandsByProvince.Count != b.AvailableCommandsByProvince.Count) {
+                return false;
+            }
+            foreach (var key in a.AvailableCommandsByProvince.Keys) {
+                if (!b.AvailableCommandsByProvince.ContainsKey(key)) { return false; }
+            }
+            return true;
+        }
+
         private void UpdateBattleProgressDisplay(IGameModel model) {
             if (model.ShardokBattles.Any()) {
                 string fightableGameId =