YieldController MVP — per-PCH yield state with unified wire envelope#10555
Merged
tool4ever merged 22 commits intoCard-Forge:masterfrom May 3, 2026
Merged
YieldController MVP — per-PCH yield state with unified wire envelope#10555tool4ever merged 22 commits intoCard-Forge:masterfrom
tool4ever merged 22 commits intoCard-Forge:masterfrom
Conversation
…elope Each PlayerControllerHuman now composes a YieldController instance that owns all yield state for its player: legacy autoPassUntilEndOfTurn, phase markers, stack-yield, per-card/ability auto-yield, trigger decisions, and skip-phase prefs. The 8 yield-related "ugly netplay fields" on PCH are deleted (7 migrated, 1 deferred); every yield accessor collapses to a one-line delegation, removing the isRemoteGuiProxy() fork from each method. Five fragmented per-method ProtocolMethod entries collapse into two unified entries (sendYieldUpdate CLIENT, applyYieldUpdate SERVER) carrying a sealed YieldUpdate envelope with seven record cases. Game- start chatter drops from N messages (one per saved auto-yield) to a single SeedFromClient(snapshot) message; same path serves reconnection. New MVP features ungated: - Phase markers via right-click (desktop) / long-press (mobile) on phase indicators. Auto-passes priority until the marked phase, then clears. - "Yield to entire stack" stack-item context menu entry. Stack-yield resolves the entire stack including post-cancel additions; only ESC clears it. Master interrupt behavior preserved: MagicStack.add() opponent-spell cancel routes through PlayerController.autoPassCancel() -> YieldController.cancelYield() (clears legacy + marker; stack-yield deliberately survives). PhaseHandler turn-boundary cancel and GameAction game-start cleanup unchanged. Carve-out from PR <!-- -->Card-Forge#9643. Deferred features (smart suggestions, interrupt prefs, auto-pass-no-actions, speed settings, host gating) land in the YieldRework follow-up PR rebased onto this foundation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tool4ever
reviewed
Apr 28, 2026
Wire path for skip-phase updates now goes SetSkipPhase(turnPlayer, phase, skip) -> PCH.applyYieldUpdate switch -> yieldController.setSkipPhase(...), bypassing the old per-method IGameController.setUiShouldSkipPhase entry. PCH's override of that method was no longer reachable. NetGameController.setUiShouldSkipPhase stays — still called via instanceof cast in NetworkGuiGame.pushSkipPhaseToControllers when the GUI's PhaseLabel toggles fire — but no longer overrides an interface method. Also drops two YieldController accessors that had no callers: getTriggerDecisions and getSkipPhases. They were defensively added during the Phase 3 field migration as enumeration accessors, but the only call sites that would have needed them (snapshot building) read from GUI state and the AutoYieldStore directly. Single-key getters (getTriggerDecision, isSkippingPhase) cover every actual use. The PhaseType import in IGameController was load-bearing only for the deleted setUiShouldSkipPhase signature. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tool4ever
reviewed
Apr 28, 2026
Both PhaseLabel (desktop) and VPhaseIndicator.PhaseLabel (mobile) had two click paths: a Runnable-callback "onToggled" for left-click skip- phase wiring, and an inline right-click / long-press handler that reached statically into CMatchUI / MatchController to dispatch yield- marker updates. Unify on the callback pattern. The view widgets now only expose setOnRightClick / setOnLongPress hooks; CMatchUI.actuateMatchPreferences and MatchController.actuateMatchPreferences register the marker logic next to the existing setOnToggled wiring. PhaseLabel drops 5 imports (PlayerView, YieldMarker, YieldUpdate, IGameController, CMatchUI) plus its phaseOwner field; the now-redundant PhaseIndicator.setOwner push and VField/VPlayerPanel call sites also go away. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tool4ever
reviewed
Apr 29, 2026
Hanmac
reviewed
Apr 29, 2026
tool4ever
reviewed
Apr 29, 2026
tool4ever
reviewed
Apr 29, 2026
tool4ever
reviewed
Apr 29, 2026
PCH and NetGameController had a parallel local-vs-remote-or-cache fork in every yield method (shouldAutoYield, setShouldAutoYield, getAutoYields, clearAutoYields, get/setDisableAutoYields, shouldAlways*Trigger, setShouldAlways*Trigger). Each side decided where to read or write based on isRemoteClient() and routed to either the LobbyPlayer's persistent AutoYieldStore or YieldController's per-controller cache fields. YieldController now owns one AutoYieldStore resolved by activeStore(): LobbyPlayer's persistent store for host PCH controlling a local player, or its own store as a per-game cache for host PCH controlling a remote player and as a session-lifetime store for NetGameController. Tier and install-mode logic that consulted FPref also moves into YieldController. PCH's eleven yield methods become one-line delegators; NetGameController loses its parallel yieldStore field and the duplicate FPref helpers. The SetCardAutoYield and SetTriggerDecision arms in NetGameController.applyYieldUpdate are dropped - server never pushes those, and user-initiated setters bypass local-apply. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
setMarker only suppressed pastTarget firing when priority was AT the target at activation. Right-clicking a phase already past on the marker owner's current turn left activationOnMarker = false, so the very next shouldAutoYield tick fired and cleared the marker. Visible to clients as the chevron disappearing instead of fast-forwarding to next turn. Treat "priority at-or-past target on owner's turn" as one condition: in both cases the marker must wait for the next turn's occurrence of the target phase. The existing isPriorityAt helper (only used here) becomes isPriorityAtOrPastMarker; setMarker uses it for both hasLeftMarker and activationOnMarker. Other-player-turn activations still take the existing path - pastTarget is gated by inMarkerOwnerTurn in shouldAutoYield, so the misfire is impossible there. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per Hanmac's review on PR <!-- -->Card-Forge#10555, mirrors mobile's VPhaseIndicator structure: one EnumMap<PhaseType, PhaseLabel> instead of twelve hand-named fields plus their getters and the getLabelFor switch. populatePhase becomes twelve addPhaseLabel calls (caption, phase, tooltip key); resetPhaseButtons becomes a single loop. allLabels returns Iterable to match mobile. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per tool4ever's review on PR <!-- -->Card-Forge#10555: the ESC clearing logic duplicated across desktop's VPrompt belongs in YieldController, and mobile MatchScreen.keyDown was missing it entirely. YieldController.clearActiveYields(local, controller) takes a player and an IGameController, clears any active marker / legacy autopass / stack-yield, and returns whether anything was cleared so callers can suppress the ESC fallthrough and refresh the chevron UI. Marker and stack-yield go over the wire via controller.sendYieldUpdate; legacy autopass is local to host PCH (still gated by the existing instanceof PlayerControllerHuman check). Persistent per-card auto-yields are unaffected. Desktop VPrompt's keyPressed handler collapses to a single helper call. Mobile MatchScreen.keyDown's ESCAPE arm gains the same prelude before its existing btnCancel trigger. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per tool4ever's review on PR <!-- -->Card-Forge#10555: the ESC handler was reaching for the global CMatchUI.getActive() static when it could just read controller.getMatchUI() off VPrompt's own CPrompt field. The controller-scoped accessor is also more correct - the static lookup could in principle return a different instance than the one this prompt belongs to. Drops the redundant null check that was guarding against the static returning null. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tool4ever
reviewed
Apr 30, 2026
tool4ever
reviewed
Apr 30, 2026
tool4ever
reviewed
Apr 30, 2026
tool4ever
reviewed
Apr 30, 2026
Per PR Card-Forge#10555 review (tool4ever, r3168957452): the server→client side of the unified yield envelope only ever carried ClearMarker, and that's derivable on the client from cached marker + phase events. Marker-fire detection moves into YieldController.checkAndClearMarker(GameView) — pure derivation called both from the host's shouldAutoYield priority loop (so the host stops auto-passing promptly at the target phase) and from an EDT phase-event listener (so the chevron + auto-pass prompt refresh on both host and remote client without a wire round-trip). Wire surface drops from 2 ProtocolMethod entries to 1: removes ProtocolMethod.applyYieldUpdate, IGuiGame.applyYieldUpdate, and the RemoteClientGuiGame/NetworkGuiGame overrides; AbstractGuiGame.applyYieldUpdate becomes checkMarkerAutoClear. YieldUpdate envelope itself stays as the unified shape for client→host messages. Marker mutators synchronized. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per PR Card-Forge#10555 review (tool4ever, r3169048394): the client-side yieldController.setSkipPhase write in setUiShouldSkipPhase is unread — the client GUI checks PhaseLabel state directly, and the host's auto-pass reads from the host PCH's own yieldController. Only the wire send to host is load-bearing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
VPrompt (desktop) and MatchScreen (mobile) ESC handlers were clearing active yields before invoking the prompt's cancel button. If a trigger confirm or other input prompt was open, ESC silently dropped the yield and left the prompt waiting -- breaking the user's "ESC = No / cancel this action" expectation. Flip the precedence: if btnCancel is in a contextually meaningful state (enabled and not the no-op "End Turn" label with UI_ALLOW_ESC_TO_END_TURN off), ESC clicks it and returns. Only when ESC would otherwise be a no-op does it fall back to clearing marker / legacy autopass / stack-yield. Net result is a strict superset of master ESC semantics, plus yield-clear in the slots where master already did nothing. Addresses review feedback on PR <!-- -->Card-Forge#10555. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Host's local chevron stayed drawn after the priority loop fired the marker: shouldAutoYield's checkAndClearMarker call discarded its return value. Now refreshYieldUi runs when the priority-loop path actually clears the marker; the EDT phase-event listener still handles the race-win case. Setting a marker on a past phase from the remote client never showed the chevron: NetGameController constructs YieldController with a null owner, so setMarker's at-or-past check returned false, activationOnMarker defaulted to false, and the next phase event made checkAndClearMarker fire prematurely. Pass the at-or-past flag via SetMarker (computed at click time by the UI), so client cache and host PCH initialize identically. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tool4ever
reviewed
Apr 30, 2026
tool4ever
reviewed
May 1, 2026
Per tool4ever review (r3170857314): host-side cancel routing stays at
master's InputLockUI.selectButtonCancel -- uniform regardless of which
Input is active. That's correct on the host, but it's not sufficient by
itself in network play. The remote client caches its own YieldController
for chevron rendering, and round-2 deleted the server->client
applyYieldUpdate propagation path, so host-side state changes no longer
reach the client automatically. And on the host, a cancel can route to
a non-LockUI Input (e.g. InputPayMana) which never reaches the
autoPassCancel-for-all-players loop in InputLockUI.selectButtonCancel.
Four client-side fixes were needed:
- Yield prompt persists across opponent-turn waits: updatePromptForAwait
and the 1s elapsed-time tick append "Waiting for opponent..." below
the yield text instead of replacing it.
- Yield-cancel detection by exact prompt match (lastPromptMessage)
instead of cancel-button-label heuristic, so InputPayMana's "Cancel"
doesn't disarm a marker.
- Cancel-button click sends ClearMarker/StackYield(false) via
sendYieldUpdate -- self-applies locally (chevron) and ships to host
so server-side state clears regardless of which Input is active.
- Clearing the marker (cancel, right-click toggle-off, host fast-forward
past target) refreshes the prompt locally via a new
refreshPromptAfterLocalYieldClear() so stale "Yielding until..." text
drops immediately.
Adjacent: FDialog only consumes ESC when modal/focused;
PCH.applyYieldUpdate triggers updateAutoPassPrompt on yield activation;
lblYieldingUntilPhaseFmt apostrophe escape ('s -> ''s) for MessageFormat.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tool4ever review (r3172286642): the 12 htmlPhase*Tooltip strings all followed the same "<html>Phase: <name><br>Click to toggle.</html>" template. Replace them with a single htmlPhaseTooltipFmt that takes PhaseType.nameForUi as the format argument. PhaseIndicator.addPhaseLabel loses its tooltipKey parameter and builds the tooltip inline. The shared text also documents the new right-click-to-fast-forward behavior introduced earlier on this branch. Removed 12 keys from each of the 9 .properties files. New key only added to en-US -- Localizer falls back to English when missing in the active locale, so translators can fill in localized versions later without blocking this consolidation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cancel becomes host-driven: clears all three yield modes per-PCH via
the host's InputLockUI; server→client applyYieldUpdate wire restored
for chevron sync, with forward-compat for YieldRework's interrupt-
driven cancel records (mass-destruction, opponent-targeting). Per-PCH
instead of master's all-players iteration -- in netplay that would
let a remote client's cancel disarm the host's own auto-pass.
- Restore ProtocolMethod.applyYieldUpdate (Mode.SERVER) on the
existing YieldUpdate sealed envelope; MVP ships ClearMarker /
StackYield. RemoteClientGuiGame override sends over wire;
AbstractGuiGame default routes to local IGameController
- InputLockUI.selectButtonCancel: per-PCH clear of all three modes;
autoPassCancel() runs first to keep its mayAutoPass gate satisfied
- Drop client-side machinery: lastPromptMessage, doShowPromptMessage
abstract rename, isYieldPromptShowing, clearLocalYieldsForCancel,
refreshPromptAfterLocalYieldClear, EDT phase listener
checkMarkerAutoClear, actCancel wrappers on desktop CPrompt +
mobile MatchScreen
- YieldController.checkAndClearMarker now private and unsynchronized
(game-thread only via shouldAutoYield); setMarker/clearMarker keep
synchronized
- updateAutoPassPrompt restarts the elapsed-time timer so "Yielding
until X / Waiting for {opponent} (Xs)" stays visible during slow
opponent decisions; per-tick wire cost is zero
- Rename misnamed YieldController.autoPassUntilEOT() to
autoPassUntilStackEmpty(); add autoPassUntilEndOfTurn() for the
legacy field; rewrite currentYieldMessage with explicit branches
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tool4ever
approved these changes
May 3, 2026
Contributor
tool4ever
left a comment
tool4ever
left a comment
There was a problem hiding this comment.
While I'm not fully happy with all data flows yet it's arguably cleaner than before
MostCromulent
added a commit
to MostCromulent/forge
that referenced
this pull request
May 3, 2026
Replace boolean UI_PRESELECT_PREVIOUS_ABILITY_ORDER with a tri-state UI_REORDER_SIMULTANEOUS_ABILITY_MODE (Off / Preselect / Auto-apply). In Auto mode, when the same simultaneous-ability combo fires again within a match the saved order is applied silently — no dialog. A "Clear Ability Orders" entry on the in-match Game menu (visible only in Auto mode) wipes the saved orders so the dialog reappears. The clear action rides on the YieldUpdate envelope from <!-- -->Card-Forge#10555 as a new ClearAbilityOrders record, so remote clients reach the host-side PCH's orderedSALookup without a dedicated ProtocolMethod. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MostCromulent
added a commit
to MostCromulent/forge
that referenced
this pull request
May 3, 2026
Restructure trigger Always-Yes / Always-No persistence to fit the YieldController architecture introduced in PR Card-Forge#10555. - AutoYieldStore: replace int-keyed per-game trigger map with EnumMap<Tier, Map<String, TriggerDecision>> + per-store triggerDecisionsDisabled flag, mirroring the yield side. - PersistentYieldStore renamed to PersistentAutoDecisionStore and extended with trigger.accept.<key>= / trigger.decline.<key>= schema lines (additive, backwards compatible with existing auto-yields.dat files). - YieldStateSnapshot: split trigger decisions by scope (card vs ability), add autoTriggersDisabled flag. - YieldUpdate.TriggerDecision reshaped to (storageKey, decision, abilityScope), mirroring CardAutoYield. New SetDisableYields and SetDisableTriggers records carry runtime disable toggles through the unified envelope (incidentally fixes a master bug where remote NetGameController.setDisableAutoYields never reached the host's cache). - YieldController: tier-aware String-keyed trigger API (shouldAlways*Trigger / setAlways*Trigger / applyTriggerDecision FromWire / getAutoTriggers / get+setDisableAutoTriggers) paralleling the yield API. activeTriggerTier reads a separate UI_AUTO_TRIGGER_MODE preference. - ForgeConstants: AUTO_TRIGGER_PER_CARD/_PER_ABILITY/_SESSION/ _INSTALL constants. UI_AUTO_TRIGGER_MODE defaults to Per Card to preserve today's per-instance trigger behavior. - IGameController, NetGameController, PlayerControllerHuman: String-keyed trigger setters, dispatcher branches for the new YieldUpdate records. - Trigger: toString(active, includeRemembered) overload + stable getYieldKey() helper paralleling SpellAbility.yieldKey(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Spin-off MVP carved from PR #9643 (YieldRework). Refactors yield state into a per-
PlayerControllerHumanYieldControllerwith a unifiedYieldUpdatewire envelope. Deletes 8 yield-related "ugly netplay fields" from PCH; collapses 5 fragmented yield ProtocolMethod entries into 2.Architecture doc attached: yield-controller-architecture.md
Included
YieldControllerclass — owns markers, stack-yield, legacy autopass, per-card/ability auto-yield, trigger decisions, skip-phase prefs (one instance per PCH).YieldUpdatesealed envelope — singlesendYieldUpdate/applyYieldUpdateProtocolMethod pair carries every yield sync direction.SeedFromClient(snapshot)at game start — replaces N-message replay loops with one wire message per controlled player. Covers per-card/ability auto-yields,autoYieldsDisabled, skip-phase prefs.MagicStack.add()opponent-spell cancel clears legacy autopass; markers and stack-yield deliberately survive engine-driven cancels).Deferred to follow-up
InputPassPriority(PendingSuggestionenum)YIELD_INTERRUPT_ON_*),YIELD_AUTO_PASS_RESPECTS_INTERRUPTS,YIELD_AUTO_PASS_NO_ACTIONSYIELD_SKIP_PHASE_DELAY,YIELD_SKIP_RESOLVE_DELAY)VYieldOptionsmobile dialog andVYieldSettingsSpeed Settings sectionThe remainder of YieldRework rebases onto this MVP and ships those features in a follow-up PR.
🤖 Generated with Claude Code