Yield Rework V2 — configurable interrupts, APINA, suggestions, settings UI by MostCromulent · Pull Request #10606 · Card-Forge/forge

MostCromulent · 2026-05-04T22:37:50Z

Refactor of #9643 (Yield Rework v1) on top of the per-PCH YieldController MVP shipped in #10555.

Re-implements the deferred YieldRework feature set against the MVP's architecture (single YieldUpdate wire envelope, per-PCH controller, atomic seed) without adding new ProtocolMethod entries.

Architectural reference doc attached separately covers the design in detail.

Deferred features included

Configurable interrupt system — six interrupt prefs governing when an active autopass cancels: OPPONENT_SPELL, ATTACKERS, TARGETING, MASS_REMOVAL, TRIGGERS, REVEAL. First two default ON to preserve master's existing hardcoded interrupts; the rest default OFF.
APINA (Auto-Pass If No Actions) — persistent toggle that auto-passes priority whenever no playable actions exist. Backed by a new forge.ai.AvailableActions heuristic with a per-PCH timeout budget. Separate RESPECTS_INTERRUPTS opt-in (default OFF).
Smart suggestions — InputPassPriority offers Accept/Decline prompts when conditions match: stack-yield when there's no playable response to the stack, marker-on-upkeep when no actions exist on your turn. Each suggestion type has a configurable decline scope (NEVER/ALWAYS/STACK/TURN).
Speed settings — opt-in skip of the inter-phase pause and the post-resolve pause during sustained auto-passes.
Per-PCH preference sync — every yield FPref is overridable per-controller, propagated via two new YieldUpdate envelope variants. Atomic seed at game start carries the full overlay map in a single message.
Settings UIs — desktop VYieldSettings dialog + VYield dock-tab panel (Auto-Pass toggle button + settings shortcut), mobile VYieldOptions scrollable dialog. Game-menu entries on both platforms. Three configurable desktop shortcuts (Ctrl+Y / F2 / ESC).

Hardcoded autoPassCancel() sites in MagicStack, PCH.declareAttackers, and HostedMatch deleted — the event-driven handler covers them, governed by per-PCH prefs. Default-ON pref values keep observable behavior identical for unconfigured users.

🤖 Generated with Claude Code

…ttings UI Builds on the per-PCH YieldController MVP shipped in master. Adds the deferred YieldRework feature set on top of that architecture without introducing new ProtocolMethod entries — everything rides the existing YieldUpdate envelope. Features - Six configurable interrupt prefs that control when an active autopass (EOT, marker) is cancelled: OPPONENT_SPELL, ATTACKERS, TARGETING, MASS_REMOVAL, TRIGGERS, REVEAL. First two default ON to preserve master's hardcoded behavior; the rest default OFF. - APINA (Auto-Pass If No Actions) — per-tick predicate gated by the AvailableActions heuristic in forge-ai, with configurable timeout. - Smart suggestions — InputPassPriority offers a stack-yield prompt when there are spells on the stack and no playable response, and a no-actions marker prompt on the player's own turn with no playable cards. Each has a configurable decline scope (NEVER / ALWAYS / STACK / TURN). - Speed settings — opt-in skip of inter-phase and post-resolve delays. - Desktop UI: VYield dock-tab panel (Auto-Pass toggle + Settings buttons) and VYieldSettings dialog. Three configurable shortcuts: Ctrl+Y opens settings, F2 toggles APINA, ESC clears the active yield. - Mobile UI: VYieldOptions scrollable settings dialog + VGameMenu entries for Yield Options and the Auto-Pass toggle. Architecture - All new state lives on per-PCH YieldController; each client's preferences govern only that client's host-side proxy. - Three new YieldUpdate variants (SetAutoPassUntilEndOfTurn, SetYieldBoolPref, SetYieldStringPref) plus pref-overlay extensions to the SeedFromClient snapshot. Zero new ProtocolMethod entries. - Single-yield invariant: at most one of EOT/marker/stack-yield is active at a time (setters enforce). Stack-yield is deliberately immune to event-driven interrupts — only stack-empty turns it off, since its whole point is "ride through stack additions." - YieldController.apply(YieldUpdate) is the single dispatch point for wire envelopes; PCH and NetGameController applyYieldUpdate are thin delegators. - Type-safe domain enums: DeclineScope (NEVER/ALWAYS/STACK/TURN) and SuggestionType (STACK_YIELD/NO_ACTIONS, owns allowed scopes + scope FPref) replace the previous stringly-typed code. - PCH.tryAutoPassNow runs after every yield-state change and re-evaluates mayAutoPass at the current input, so toggles fire on the current prompt (no priority-window lag). Hardcoded interrupt sites deleted from MagicStack, PCH.declareAttackers, and HostedMatch — the event-driven handler covers the same conditions, now governed by per-PCH prefs. Also lower DragCell minimum height to 50px so the dragged-out yield-options tab can shrink to fit a single row of buttons. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

autumnmyst · 2026-05-05T18:25:19Z

I notice you still have something checking state despite the yield interrupts. I don't think this is necessary and will just cause the state-scanner to re-prompt on every priority pass while the interrupt state persists. shouldInterruptYield(player) shouldn't need to check the state of the game to decide whether to yield, since we have event driven interrupts now. Also, should make the stack yield respect interrupts, so if things are added that should interrupt, they indeed interrupt. I don't think isInterruptibleYieldActive() is necessary, all yields should be interruptible.

autumnmyst · 2026-05-05T18:28:18Z

Also, I recall we talked about removing the smart suggestion prompts? Are they actually useful enough to justify as a feature?

Previously APINA + RESPECTS_INTERRUPTS evaluated `shouldInterruptYield` on every priority tick — meaning a Wrath sitting on the stack caused APINA to re-prompt every priority window for as long as the spell stayed there. Yields had the right semantic (event-driven, one prompt per event); APINA duplicated the classifiers in a per-tick state-scanner that produced the wrong UX. APINA now shares the yield event path. Each event handler (`onSpellAbilityCast`, `onAttackersDeclared`, `maybeInterruptOnReveal`) calls a unified `applyInterrupt()` that clears any interruptible yield and sets a transient `autoPassInterrupted` flag when APINA + RESPECTS_INTERRUPTS are on. The flag is cleared by `noteMayAutoPassResult` once the user has been prompted, so APINA resumes on the next priority window without re-prompting. `shouldInterruptYield` and `hasMassRemovalOnStack` are deleted — both were only reachable from the APINA path. Side effect: REVEAL now applies to APINA. The state-scanner had no REVEAL check, so users with INTERRUPT_ON_REVEAL enabled were silently missing that interrupt under APINA; the event handler covers it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

MostCromulent · 2026-05-05T21:25:43Z

@autumnmyst

I notice you still have something checking state despite the yield interrupts. I don't think this is necessary and will just cause the state-scanner to re-prompt on every priority pass while the interrupt state persists.

this was for APINA, have updated so interrupts apply on event driven basis there too. much cleaner architecture now

Also, should make the stack yield respect interrupts, so if things are added that should interrupt, they indeed interrupt. I don't think isInterruptibleYieldActive() is necessary, all yields should be interruptible.

i didn't think we want stack yield to be interrupted? i.e. when you click 'yield to entire stack' you're saying "I don't want to interact with this stack in any way / I have no instants I can play, don't bother me again".

e.g. you're in a commander game and your opponents are interacting on the stack but you don't want/cant participate. if stack yield interrupts you get bothered every time opponents interact when you've already opted out.

if we really we want this i'd suggest we add a 'Stack yield respects interrupt' option to the settings, same as the APINA respects, so at least its optional

Also, I recall we talked about removing the smart suggestion prompts? Are they actually useful enough to justify as a feature?

in discussion on Discord, TRTs view was worth keeping because its useful for players who don't want full automation and want to use Forge to eg. train for playing paper games

autumnmyst · 2026-05-05T21:38:52Z

I think we still want stack interrupts, since yielding the stack is saying “I don’t want to interact with the stack in its current state” but you yield and then say, a mass removal spell is added, you might want to reconsider. You can always just turn off interrupts too. I don’t really see stack yield as “special” in that it should ignore interruption.

Example: play a spell, 10 prowess triggers appear, you have a counterspell but you don’t want to counter your own spell, but that means it doesn’t autopass. You don’t want to click through each trigger, so you yield the stack. The next player plays their own counterspell on your spell. You had targeting interruptions on, but it was ignored and you never had a chance to respond to them countering your spell.

The dedicated yield options panel was a tab in the prompt cell hosting just two controls (Auto-Pass toggle + Settings dialog launcher). Fold both into the existing dock — Auto-Pass becomes an icon with a goldenrod- highlighted toggled state, and the previously-dormant cog button now opens VYieldSettings. Promote BUTTON_DOCK to its own match.xml cell directly above the prompt panel, aligning the prompt top with the hand top. Drop the in-dock Revert/Open/Save Layout buttons (still available from the menu bar's Layout menu). Update the Advanced Yield Options wiki page to match the new access points. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The stack-item context menu now offers two choices on desktop and mobile. "Yield to stack" auto-passes until the stack empties but backs off when any of the existing yield-interrupt prefs trip (opponent spell, targeting, mass removal, triggers, reveal). "Resolve entire stack" preserves today's fire-and-forget behavior — only stack-empty turns it off. The "you cannot respond to the stack" suggestion defaults to the interruptible variant. Implemented as a single boolean flag (stackYieldRespectsInterrupts) paired with the existing autoPassUntilStackEmpty state on YieldController, with the flag carried over the wire as a third component on YieldUpdate.StackYield. Reuses the existing applyInterrupt() path — no new prefs introduced. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Collapse the dedicated passPriorityUntilEndOfTurn ProtocolMethod into the existing YieldUpdate envelope so all yield-related wire traffic rides one ProtocolMethod pair, matching the V2 single-surface principle. The four call sites (CDock, GameMenu, KeyboardShortcuts, MatchScreen) now go through a YieldController.endTurn helper that mirrors the existing toggleAutoPassNoActions shape. Removing the isYieldActive early-return in tryAutoPassNow is safe because the mayAutoPass check below already encompasses it — mayAutoPass is shouldAutoYield || isAutoPassingNoActions, and shouldAutoYield is true exactly when a yield is active. The early-return only diverged from mayAutoPass in one direction: when a yield had just been activated, it blocked the OK click that the original passPriority(true) always made unconditionally. Dropping it lets the envelope path reproduce that behavior on the current InputPassPriority instead of waiting for the next cycle. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Snapshot building (snapshotBoolPrefs / snapshotStringPrefs) was returning a copy of the local override map, which is empty at game start because the override map only fills when the user toggles a setting mid-game. The host's proxy of a remote player would receive an empty seed, then fall through to the host's FModel for every read — silently substituting the host's preferences for the client's. Defaults-only players didn't notice because the two default-on interrupts matched between unconfigured host and client; anyone who customized their settings would see them ignored until they re-toggled each pref mid-match. Fix: enumerate the synced yield FPrefs (SYNCED_BOOL_PREFS / SYNCED_STRING_PREFS) and read each effective value from FModel when building the snapshot, so the proxy is seeded with the client's actual values from turn 1. This makes the override map's role coherent: populated only on the host's proxy of a remote player, via applyClientSeed and the SetYieldBoolPref/SetYieldStringPref envelopes. Local controllers (host's own PCH and client's NetGameController) always read through FModel fallback — the dialog already writes there, so the parallel local store was dead weight. Drop the yieldController.setBoolPref/setStringPref calls from PCH.setYieldBoolPref/StringPref and the equivalent in NetGameController; PCH.setYieldBoolPref still fires tryAutoPassNow because toggling APINA may flip mayAutoPass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Addresses review comment on Card-Forge#10606. When a yield is active, mayAutoPass() short-circuits via shouldAutoYield() and never consults the available-actions field, so computing it is wasted work. Match the gate already used by chooseSpellAbilityToPlay. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Replace the single Auto-Yield section with "Auto-Pass and Yield Options" (concept summary plus Auto-Pass No Actions, End Turn, yield markers, and stack-yield) and "Auto-Yield and Trigger Decisions" (the four lifetime scopes for sticky stack decisions). Sidebar gains an entry for advanced-yield-options.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…interrupt PhaseHandler.declareAttackersTurnBasedAction already gates on CombatUtil.canAttack, so by the time declareAttackers is invoked the player has at least one legal attacker — the prompt itself is the available action. Switch the gate from mayAutoPass to shouldAutoYield so APINA never skips, while explicit yields keep their legacy "skip when not-attacking is legal" behavior. When a must-attack/goaded creature forces the prompt during a yield, clear the yield via applyInterrupt() to match every other interrupt classifier. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Collapse SetYieldBoolPref/SetYieldStringPref into SetYieldPref. PreferencesStore is already String-typed, so the type split duplicated infrastructure with no behavior gain. Wire field is String; callers wrap with String.valueOf. Chose call-site conversion over the host-side Object→String approach to keep the wire/storage types aligned and avoid Object in a record. The substantive consolidation is the same either way. setYieldPref now skips tryAutoPassNow unless the changed pref is YIELD_AUTO_PASS_NO_ACTIONS — that's the only pref whose toggle can flip mayAutoPass for a sitting prompt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

V2 introduced two parallel mechanisms for yield-clear that bypassed the Input layer: VPrompt.buttonKeyAdapter ran an unconditional triple sendYieldUpdate ahead of btnCancel.doClick(), and SHORTCUT_YIELD_CANCEL sent the same triple as a global keyboard action. The KeyAdapter also regressed the MVP precedence rule -- cancel-click takes priority over yield-clear so ESC during a confirm/mana-pay prompt doesn't silently drop an armed yield. - VPrompt: revert KeyAdapter to master form. Consume the event after cancel-click so the global shortcut doesn't double-fire on the same ESC. - KeyboardShortcuts: actCancelYield now calls ctrl.selectButtonCancel() instead of sending envelopes directly. Single yield-cancel path through the Input layer; InputLockUI.selectButtonCancel triggers clearActiveYieldAndDispatch as a side effect. Tradeoff: the shortcut no longer clears yield while the host is in a non-LockUI Input (e.g. mid-mana-pay). User dismisses the active prompt first, then ESC clears yield -- consistent with normal layered-ESC UI conventions. Addresses review on PR Card-Forge#10606. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Repartitions the yield keyboard surface so ESC carries no yield-specific role and a single unconditional key clears all yield state regardless of what prompt is focused. - F2 (was APINA toggle) becomes "stop all yields": calls YieldController.stopAllYields, which clears transient yield state via clearActiveYieldAndDispatch and turns APINA off if it's on. Lives in the global WHEN_IN_FOCUSED_WINDOW InputMap and isn't intercepted by VPrompt.buttonKeyAdapter (which only consumes ESC), so it fires through mana-pay or confirm prompts without dismissing them. - F3 (was unbound) becomes the pure APINA toggle. - SHORTCUT_YIELD_CANCEL deleted -- ESC already reaches btnCancel via VPrompt's KeyAdapter, so the dedicated binding was a no-op duplicate. Each key now has one unconditional behavior: ESC clicks cancel, F2 stops everything yield-related, F3 toggles APINA. No mode-dependent layering on any single key. User-Guide and advanced-yield-options updated to match. Addresses review on PR Card-Forge#10606. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Replace the F2/F3 split with a single P key. If any yield is currently active (transient state or APINA), the key cancels everything; otherwise it turns Auto-Pass on. Y would have been the obvious mnemonic for "Yield" but it's already the default for auto-yes on stack items (SHORTCUT_AUTOYIELD_ALWAYS_YES); P for "Pass" is the next-best fit and keeps shortcuts off F-keys. Also refresh the dock auto-pass icon highlight after the keyboard path runs -- previously only the dock-button click refreshed it, so pressing the key left the icon stale until the next CDock.update. Drive-by: reword the End Turn shortcut's description to "pass priority until end of turn or interrupt", matching the "interrupt" wording introduced by the renamed yield-to-stack option. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

tool4ever · 2026-05-09T12:18:45Z

+
+        // Game-thread only; no cross-thread races on the YIELD_SUPPRESS_AFTER_END flag
+        boolean nowMayAutoPass = mayAutoPass();
+        yieldController.noteMayAutoPassResult(nowMayAutoPass);


why is this method synchronized if this is the only place it gets called?

The fields touched are read cross-thread:

yieldJustEndedFlag is read+cleared by didYieldJustEnd() from InputPassPriority.showMessage() (line 87). showMessage() runs on the EDT — InputProxy.update() line 73 dispatches it via FThreads.invokeInEdtLater(showMessage).

noteMayAutoPassResult() is called from chooseSpellAbilityToPlay() on the game thread.

The synchronized pairs with the synchronized didYieldJustEnd() to provide a happens-before edge for the game→EDT visibility of yieldJustEndedFlag.

So synchronized is doing real work — but the inline comment "Game-thread only; no cross-thread races on the YIELD_SUPPRESS_AFTER_END flag" at PlayerControllerHuman.java:1510 is wrong/misleading about that. The flag IS read cross-thread; the synchronized is what makes it safe. The fix is to keep the synchronized and correct/remove the misleading comment.

(autoPassInterrupted and wasAutoPassingLastTick look game-thread-only, so they're not what carries the synchronization need — yieldJustEndedFlag is.)

tool4ever · 2026-05-09T16:52:27Z

+    }
+
+    /** Clear all transient yield state so it doesn't carry into the next game of the match. */
+    public synchronized void resetForNewGame() {


I feel like AI got confused by the refactor:
is this method really useful now - new YieldController is created at game start anyway...?

YieldController is reconstructed per game via LobbyPlayerHuman.createIngamePlayer, so HostedMatch.endCurrentGame was resetting PCHs about to be discarded. The comment above noteMayAutoPassResult was wrong: yieldJustEndedFlag is read on the EDT via didYieldJustEnd, and the synchronized writer/reader pair is what makes that safe. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

MostCromulent mentioned this pull request May 4, 2026

Expanded yield system / auto-pass options #9643

Closed

MostCromulent requested a review from tool4ever May 4, 2026 22:39

MostCromulent added Enhancement New feature or request QOL Quality of Life labels May 4, 2026

MostCromulent force-pushed the YieldReworkV2 branch from 6e214be to 86c2e37 Compare May 5, 2026 00:10

MostCromulent and others added 6 commits May 6, 2026 20:49

Merge branch 'master' into YieldReworkV2

32036f3

Fix merge

415d0cb

Clean up start

8fa44c9

Clean up start

d4ac70c

tool4ever reviewed May 6, 2026

View reviewed changes

Comment thread forge-gui/src/main/java/forge/gamemodes/match/YieldUpdate.java

Clean up start

949837c

tool4ever reviewed May 6, 2026

View reviewed changes

Comment thread forge-gui/src/main/java/forge/gamemodes/match/YieldController.java Outdated

tool4ever reviewed May 6, 2026

View reviewed changes

Comment thread forge-gui/src/main/java/forge/player/PlayerControllerHuman.java Outdated

tool4EvEr and others added 3 commits May 6, 2026 20:49

Clean up start

3925c20

tool4ever reviewed May 7, 2026

View reviewed changes

Comment thread forge-gui/src/main/java/forge/player/PlayerControllerHuman.java Outdated

MostCromulent and others added 3 commits May 7, 2026 20:29

Clean up

4249262

Fix import

6ee9452

tool4ever reviewed May 7, 2026

View reviewed changes

Comment thread docs/advanced-yield-options.md

Minor clean up

4049989

MostCromulent and others added 2 commits May 8, 2026 07:51

Merge remote-tracking branch 'origin/master' into YieldReworkV2

1e7f792

tool4ever reviewed May 8, 2026

View reviewed changes

Comment thread forge-gui/src/main/java/forge/player/PlayerControllerHuman.java Outdated

MostCromulent and others added 3 commits May 8, 2026 17:44

Remove getters

723a2b4

tool4ever reviewed May 8, 2026

View reviewed changes

Comment thread forge-gui-desktop/src/main/java/forge/screens/match/views/VPrompt.java Outdated

tool4EvEr and others added 2 commits May 8, 2026 21:27

Clean up obsolete refactor

dab3fe3

tool4ever reviewed May 8, 2026

View reviewed changes

Comment thread forge-gui-desktop/src/main/java/forge/control/KeyboardShortcuts.java Outdated

tool4ever reviewed May 9, 2026

View reviewed changes

Comment thread forge-gui/res/languages/en-US.properties

tool4ever reviewed May 9, 2026

View reviewed changes

Comment thread forge-gui/res/languages/en-US.properties

MostCromulent and others added 5 commits May 9, 2026 18:15

Translate updated lblSHORTCUT_ENDTURN into other locales

b6f104a

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Refinements

64b26fc

Remove redundancy

8acf9a7

Fix redundant label

415c34c

Fix redundant label

f34b8ab

tool4ever reviewed May 9, 2026

View reviewed changes

Clean up

cb74bc8

tool4ever reviewed May 9, 2026

View reviewed changes

MostCromulent mentioned this pull request May 9, 2026

Network multiplayer development meta issue #9861

Open

23 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Yield Rework V2 — configurable interrupts, APINA, suggestions, settings UI#10606

Yield Rework V2 — configurable interrupts, APINA, suggestions, settings UI#10606
MostCromulent wants to merge 32 commits intoCard-Forge:masterfrom
MostCromulent:YieldReworkV2

MostCromulent commented May 4, 2026 •

edited

Loading

Uh oh!

autumnmyst commented May 5, 2026 •

edited

Loading

Uh oh!

autumnmyst commented May 5, 2026

Uh oh!

MostCromulent commented May 5, 2026

Uh oh!

autumnmyst commented May 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tool4ever May 9, 2026

Uh oh!

MostCromulent May 9, 2026

Uh oh!

tool4ever May 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

MostCromulent commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deferred features included

Uh oh!

autumnmyst commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

autumnmyst commented May 5, 2026

Uh oh!

MostCromulent commented May 5, 2026

Uh oh!

autumnmyst commented May 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tool4ever May 9, 2026

Choose a reason for hiding this comment

Uh oh!

MostCromulent May 9, 2026

Choose a reason for hiding this comment

Uh oh!

tool4ever May 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MostCromulent commented May 4, 2026 •

edited

Loading

autumnmyst commented May 5, 2026 •

edited

Loading