Swap models when available provinces change, even mid-selection

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>
This commit is contained in:
2026-04-09 21:35:08 -07:00
co-authored by Claude Opus 4.6
parent a912347dbd
commit d4fbcb1987
@@ -125,11 +125,24 @@ namespace eagle {
public GameObject shardokContainer; public GameObject shardokContainer;
private KeyValuePair<ProvinceId, OneProvinceAvailableCommands>? NextActiveProvinceKeyPair => private KeyValuePair<ProvinceId, OneProvinceAvailableCommands>? NextActiveProvinceKeyPair {
Model == null || Model.AvailableCommandsByProvince.Count == 0 get {
? null if (Model == null || Model.AvailableCommandsByProvince.Count == 0) { return null; }
: Model.AvailableCommandsByProvince.First(
kvp => kvp.Key == Model.SuggestedProvinceId); // 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() { void Start() {
gameIdButton.GetComponentInChildren<Text>().text = ""; gameIdButton.GetComponentInChildren<Text>().text = "";
@@ -576,11 +589,19 @@ namespace eagle {
chronicleCanvasController.Entries = _newModel.ChronicleEntries; chronicleCanvasController.Entries = _newModel.ChronicleEntries;
} }
// If we already had commands, and we still have commands, don't actually swap the // If we already had commands, and we still have commands FOR THE SAME SET OF
// model. But always update battle effects — battles can start or end between // PROVINCES, don't actually swap the model — keep the user's selection state
// command updates, and skipping cleanup leaves stale animations on the map. // 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() && if (Model != null && _newModel != null && _newModel.AvailableCommandsByProvince.Any() &&
Model.AvailableCommandsByProvince.Any()) { Model.AvailableCommandsByProvince.Any() &&
SameAvailableProvinces(Model, _newModel)) {
mapController.UpdateBattleEffects(_newModel); mapController.UpdateBattleEffects(_newModel);
UpdateBattleProgressDisplay(_newModel); UpdateBattleProgressDisplay(_newModel);
return; 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) { private void UpdateBattleProgressDisplay(IGameModel model) {
if (model.ShardokBattles.Any()) { if (model.ShardokBattles.Any()) {
string fightableGameId = string fightableGameId =