Skip to content

Eliminate redundant protocol round-trip: cache UI skip-phase prefs#10536

Merged
tool4ever merged 4 commits intoCard-Forge:masterfrom
MostCromulent:NetworkPlay/skip-phase-prefs
Apr 27, 2026
Merged

Eliminate redundant protocol round-trip: cache UI skip-phase prefs#10536
tool4ever merged 4 commits intoCard-Forge:masterfrom
MostCromulent:NetworkPlay/skip-phase-prefs

Conversation

@MostCromulent
Copy link
Copy Markdown
Contributor

What's the issue

In network multiplayer, the host pings the remote client over the wire on every priority transition to ask "is your UI configured to skip this phase?". Each call goes through RemoteClientGuiGame.isUiSetToSkipPhasesendAndWait(ProtocolMethod.isUiSetToSkipPhase, …) — a full TCP round-trip. The host's game thread parks until the reply lands.

PlayerControllerHuman.getActivePlayerInput makes this call every time a remote human gets priority and the stack is empty — order-of-magnitude ~13 calls per turn for one remote human, ~26 for two.

The data being asked for is static across most of a match — just the client's UI label state for "stop at this phase". There's no reason to round-trip it every time.

What's the fix

The client tells the host its full skip-phase state once at match start, and pushes per-toggle deltas thereafter. The host caches it per PlayerControllerHuman and answers isUiSetToSkipPhase from local memory — zero round-trips on the hot path.

This is the same pattern recently established for auto-yield prefs (the controller-owns-prefs refactor): per-player preference state lives on IGameController, the host's PlayerControllerHuman keeps a remote-mirror map gated on isRemoteClient(), and NetGameController ships mutations via 1:1-named Mode.CLIENT protocol methods. Initial state is replayed at game start by the client (replayActiveYields for yields; replayUiSkipPhases for skip-phase here).

The only structural difference: yield prefs live in stores loaded before the game (so the seed fires from FGameClient.setGameControllers), whereas skip-phase prefs live in UI label state, which is populated by actuateMatchPreferences after game start. The seed therefore fires from the end of actuateMatchPreferences — the first point at which all label state and player views are settled.

How it works

Three logical pieces:

  1. Host-side cache. PlayerControllerHuman gains Map<PlayerView, EnumSet<PhaseType>> remoteSkipPhases. New IGameController.isUiSetToSkipPhase reads from it when serving a remote (gui instanceof RemoteClientGuiGame); the empty-cache default returns false (don't skip), matching a fresh UI's conservative default. The two getActivePlayerInput call sites swap from getGui().isUiSetToSkipPhase(…) to this.isUiSetToSkipPhase(…).

  2. Client-side push. New Mode.CLIENT ProtocolMethod.setUiShouldSkipPhase(PlayerView, PhaseType, Boolean) carries a single (player, phase, shouldSkip) mutation. NetGameController.replayUiSkipPhases(allPlayers, isSkipped) is a one-shot helper that pushes the full snapshot — only shouldSkip == true entries, since the host's empty cache already represents false.

  3. UI wiring. PhaseLabel (desktop) and VPhaseIndicator.PhaseLabel (mobile) gain setOnToggled(Runnable), fired post-flip. CMatchUI.actuateMatchPreferences (desktop) and MatchController.actuateMatchPreferences (mobile) wire per-(player, phase) callbacks at the end of label initialization, then fire the initial replayUiSkipPhases for any local NetGameController. Mid-game toggles ship one ~80-byte message each.

The old Mode.SERVER ProtocolMethod.isUiSetToSkipPhase is deleted. The IGuiGame.isUiSetToSkipPhase method stays — still used locally by FControlGamePlayback, and by the local-fallback branch inside PlayerControllerHuman.isUiSetToSkipPhase. All are local reads with no network cost.

Why it's safe

  • Local play and hotseat are unchanged. When isRemoteClient() is false, isUiSetToSkipPhase delegates to getGui() (label state, exactly as today) and setUiShouldSkipPhase is a no-op. The on-toggle push and the seed are gated on instanceof NetGameController, so local controllers (PlayerControllerHuman) skip them entirely. Existing hotseat semantics — both rows seeded from PHASES_HUMAN, independent toggles mid-match, last-write-wins on save — are preserved exactly.
  • Mind-slave correctness preserved. Desktop's CMatchUI.isUiSetToSkipPhase(turnPlayer, phase) AND-combines the turn player's row with the master's row. When the master toggles their label, the cached answer for every player they control changes too. pushSkipPhaseToControllers iterates players and re-pushes any dependent — narrow edge case (Mindslaver, Sorin Markov ult) but the cache stays consistent with what the GUI would return locally.
  • Empty-cache default is the conservative choice. Until the seed lands (a brief window at match start), the host returns "don't skip" → stop at every phase → user gets prompted as if they hadn't configured anything. No phase ever gets accidentally skipped before the snapshot arrives.

🤖 Generated with Claude Code

…-trip

PlayerControllerHuman.getActivePlayerInput called getGui().isUiSetToSkipPhase
on every priority transition; for remote players that routed through
RemoteClientGuiGame as a sendAndWait — a full TCP round-trip per check.

Each PlayerControllerHuman now keeps a per-(turnPlayer, phase) EnumSet seeded
once at match start from the client's UI label state and updated via a new
Mode.CLIENT setUiShouldSkipPhase. Reads hit the host's own map. Local play
and hotseat fall through unchanged — gui delegate on read, no-op on write.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread forge-gui-mobile/src/forge/screens/match/MatchController.java Outdated
Move pushSkipPhaseToControllers and the host-cache seeding loop into
AbstractGuiGame so desktop and mobile share one implementation. While
unifying, mobile's isUiSetToSkipPhase gained the same mindslave
AND-combine desktop has — toggling a master's row now correctly
invalidates the cached skip value for every player they control.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@MostCromulent MostCromulent marked this pull request as ready for review April 27, 2026 10:20
@tool4ever tool4ever merged commit fcbf56f into Card-Forge:master Apr 27, 2026
2 checks passed
@MostCromulent MostCromulent deleted the NetworkPlay/skip-phase-prefs branch April 27, 2026 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants