Skip to content

Fix phase stop preferences for multiplayer and network games#9820

Merged
tool4ever merged 4 commits intoCard-Forge:masterfrom
MostCromulent:networkphaseprefs
Feb 24, 2026
Merged

Fix phase stop preferences for multiplayer and network games#9820
tool4ever merged 4 commits intoCard-Forge:masterfrom
MostCromulent:networkphaseprefs

Conversation

@MostCromulent
Copy link
Copy Markdown
Contributor

Summary

Addresses the issue raised in #8335, which was closed as stale. Per @tool4ever's request, this is a fresh PR implementing a clean fix from first principles for both desktop and mobile.

The bug

actuateMatchPreferences() and writeMatchPreferences() hardcode the assumption that field index [0] is the human player and index [1+] is AI. This means the wrong phase stop preferences are applied whenever the local player is not at index 0 — which can occur in:

  • 3+ player games where the local player may not be first in the sorted player list
  • Network multiplayer where field ordering is not guaranteed to place the local player first
  • Hotseat games where multiple fields are human-controlled

The bug is difficult to reproduce in 2-player network testing because openView() swaps the field order to ensure the local player is at index 0 in the 2-player case. It is also invisible when the human and AI phase preferences happen to have the same values.

The fix

Replace the hardcoded index assumption with isLocalPlayer() checks. For each field, determine whether its player is controlled by this GUI instance and apply PHASE_HUMAN_* or PHASE_AI_* preferences accordingly.

Added FPref.PHASES_HUMAN and FPref.PHASES_AI arrays to enable iteration over phase preferences in PhaseType order, replacing the per-label boilerplate.

The fix is applied separately in desktop (CMatchUI) and mobile (MatchController) rather than consolidated into AbstractGuiGame, because the two platforms use different phase indicator classes with different APIs (PhaseIndicator.setEnabled() vs VPhaseIndicator.setStopAtPhase()). Unifying them would require abstracting over these platform-specific UI types in the shared base class, which is not warranted for this fix.

3 files changed, 54 insertions, 127 deletions.

Test plan

  • 1v1 vs AI: phase labels match saved preferences, toggles persist across games
  • Network 2-player: both host and client show correct phase labels for their own field
  • Verify no regression in phase stop behavior during gameplay

Coded with Claude Code

…ndex

The old code assumed field index [0] is always the human player and
index [1+] is always AI. This is incorrect in multiplayer and network
games. Use isLocalPlayer() to select the correct pref set for each
field. Fixes both desktop (CMatchUI) and mobile (MatchController).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@tool4ever tool4ever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about the additional writeMatchPreferences() call that was previously being added (see open thread in old PR):
I thought it might make sense since otherwise it can only be reached by one player via ControlWinLose.nextGameAction (though maybe not in that exact location)?

…anges

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@MostCromulent
Copy link
Copy Markdown
Contributor Author

what about the additional writeMatchPreferences() call that was previously being added

Yep, have added back in.

@tool4ever
Copy link
Copy Markdown
Contributor

seems AI isn't that good at analyzing previous PR progression:
"this means in local game preferences will get written an additional time?"

Local games already save via ControlWinLose.saveOptions().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@MostCromulent
Copy link
Copy Markdown
Contributor Author

MostCromulent commented Feb 24, 2026

seems AI isn't that good at analyzing previous PR progression: "this means in local game preferences will get written an additional time?"

Digging into it, looks like AI did consider this but came to view the double-write was harmless. Have now got it to implement the netplay check suggested on the old PR, sorry about that.

Copy link
Copy Markdown
Contributor

@tool4ever tool4ever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good enough for now

@tool4ever tool4ever merged commit be46c95 into Card-Forge:master Feb 24, 2026
2 checks passed
@MostCromulent MostCromulent deleted the networkphaseprefs branch February 24, 2026 21:20
clairchiara pushed a commit to clairchiara/forge that referenced this pull request Feb 28, 2026
…rge#9820)

* Save phase stop prefs in finishGame so all network players persist changes

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants