Skip to content

Persistent Always-Yes / Always-No trigger preferences#10506

Merged
tool4ever merged 8 commits intoCard-Forge:masterfrom
MostCromulent:autotrigger
May 6, 2026
Merged

Persistent Always-Yes / Always-No trigger preferences#10506
tool4ever merged 8 commits intoCard-Forge:masterfrom
MostCromulent:autotrigger

Conversation

@MostCromulent
Copy link
Copy Markdown
Contributor

Summary

Extends the tiered Auto-Yield architecture from #10448 to repeatedly-firing optional triggered abilities. Pressing Y/N on an optional trigger prompt can now persist that decision across future firings of the same trigger, at a tier chosen by the user.

Triggers are governed by a new UI_AUTO_TRIGGER_MODE preference that is separate from the existing yield-mode preference, so users can tier triggers and yields independently (e.g. Per Card for yields, Per Ability (Install) for triggers).

Both desktop and mobile get an Auto-Triggers menu mirroring the existing Auto-Yields menu, with its own listing dialog and a separate Disable-All toggle. Most of the diff is this UI work — new VAutoTriggers views on both platforms and menu/hotkey/preferences wiring to mirror the existing Auto-Yields dialogs.

Persistence follows the same four-tier hierarchy as yields:

  • Per Card (Each Game) — the default; preserves today's per-instance, per-game behaviour
  • Per Ability (Each Match)
  • Per Ability (Each Session)
  • Per Ability (Each Install) — written to the existing `auto-yields.dat` file (schema is additive, so existing installs load unchanged)

Default is Per Card (Each Game), so there is no behavioural change unless the user opts into a longer-lived tier.

Design notes

  • `Trigger.getStableKey()` / `getYieldKey()` produce user-decision keys paralleling `SpellAbility.toUnsuppressedString()` / `yieldKey()`.
  • Public API mirrors yields: `setShouldAlways{Accept,Decline,Ask}Trigger(String, boolean)`.
  • Tier is resolved per-side from the preference and does not cross the wire.
  • Remote-client match startup replays persisted ACCEPT/DECLINE decisions via `NetGameController.replayActiveTriggerDecisions`, symmetric to `replayActiveYields`.
  • Engine-internal integer trigger IDs (`SpellAbility.getSourceTrigger()`, `MagicStack.hasStateTrigger(int)`) are untouched — load-bearing for game-state queries, separate from the user-decision layer.
  • Disable toggle is orthogonal to yields (separate menu item, separate flag).
  • Drive-by fix: the existing yield-side `setDisableAutoYields` was client-local only — the server's `remoteAutoYieldsDisabled` flag never flipped on remote clients. Now wired through `ProtocolMethod.setDisableAutoYields(Boolean)` alongside the new trigger equivalent.

🤖 Generated with Claude Code

Comment thread forge-game/src/main/java/forge/game/trigger/Trigger.java Outdated
@MostCromulent MostCromulent added GUI QOL Quality of Life labels Apr 24, 2026
RafaelHGOliveira added a commit to RafaelHGOliveira/forge that referenced this pull request Apr 27, 2026
Ports upstream PR Card-Forge#10506: persistent always-yes/no trigger preferences.
RafaelHGOliveira added a commit to RafaelHGOliveira/forge that referenced this pull request Apr 27, 2026
@MostCromulent MostCromulent added the WAIT Needs something before merging label Apr 29, 2026
MostCromulent and others added 2 commits May 4, 2026 07:30
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) <[email protected]>
Wire the trigger Always-Yes / Always-No tier-aware persistence
into the desktop Swing and mobile libGDX UIs.

- StackItemView: add SourceTriggerYieldKey TrackableProperty +
  getSourceTriggerYieldKey() accessor, populated from the source
  Trigger.getYieldKey() in updateSourceTrigger.
- VStack (desktop + mobile): convert the Always-Yes / Always-No
  context-menu items from int triggerID to String triggerYieldKey,
  computing abilityScope from UI_AUTO_TRIGGER_MODE. Master's
  "Yield to entire stack" item is preserved unchanged.
- KeyboardShortcuts (desktop) + MatchScreen (mobile): same int
  to String migration for the Y / N hotkeys.
- VAutoTriggers (desktop + mobile): new dialog mirroring the
  VAutoYields layout. Lists trigger decisions via
  getYieldController().getAutoTriggers(). Disable-all checkbox
  toggles getDisableAutoTriggers / setDisableAutoTriggers. Remove
  button calls setShouldAlwaysAskTrigger with abilityScope
  computed from UI_AUTO_TRIGGER_MODE.
- GameMenu (desktop) + VGameMenu (mobile): single Auto-Triggers
  entry beside the existing Auto-Yields entry.
- VSubmenuPreferences + CSubmenuPreferences (desktop) +
  SettingsPage (mobile): radio group for UI_AUTO_TRIGGER_MODE,
  same four tier options as auto-yields, default Per Card.
- en-US.properties: cbpAutoTriggerMode, nlpAutoTriggerMode,
  lblAutoTriggers, lblDisableAllAutoTriggers, lblRemoveTrigger,
  lblNoActiveAutoTrigger, lblNoAutoTrigger.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@MostCromulent MostCromulent removed the WAIT Needs something before merging label May 3, 2026
Comment thread forge-gui/src/main/java/forge/localinstance/properties/ForgeConstants.java Outdated
Comment thread forge-gui-desktop/src/main/java/forge/screens/match/VAutoTriggers.java Outdated
Address review feedback to merge the two parallel surfaces:

- Single FPref (UI_AUTO_DECISION_MODE) with one set of scope constants
  replaces UI_AUTO_YIELD_MODE + UI_AUTO_TRIGGER_MODE. Migration on first
  load copies any existing UI_AUTO_YIELD_MODE value into the new key.
- Single dialog (VAutoYieldsAndTriggers) with a tagged unified list,
  search filter, and two "Disable All" checkboxes replaces VAutoYields
  + VAutoTriggers on both desktop and mobile. Same-card entries sort
  adjacent via a comparator that ignores the leading [Yield]/[Always
  Yes]/[Always No] tag.
- Single menu item replaces the two prior items in GameMenu (desktop)
  and VGameMenu (mobile); auto-yield-on-stack re-enable behavior on
  mobile is preserved.
- YieldController.isAbilityScope() collapses 12 duplicated inline
  pref-reads across VStack, KeyboardShortcuts, MatchScreen, VGameMenu,
  and CMatchUI into a single call.
- yieldKey() in SpellAbility, WrappedAbility, and Trigger now uses
  getHostCard().getName() instead of getHostCard().toString() so
  storage keys lead with the card name. Card.toString() inherits
  GameEntity's empty-name field, which left existing entries without
  the intended card-name prefix.
- Settings UIs (CSubmenuPreferences desktop, SettingsPage mobile)
  collapse to a single dropdown.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@MostCromulent
Copy link
Copy Markdown
Contributor Author

Have consolidated yield and triggers into same menu structure.

Also added search filter since plausibly if you have "Per Install" preference you'll end up with a lot of saved decisions.

Screenshot 2026-05-05 064145

Comment thread forge-game/src/main/java/forge/trackable/TrackableProperty.java
Comment thread forge-game/src/main/java/forge/game/trigger/Trigger.java Outdated
…eTriggerYieldKey

Addresses review comments on PR Card-Forge#10506. Trigger.toString() previously
appended " (rememberedObjects)" -- a per-game-state suffix only
meaningful when the trigger has fired and sits on the stack. The PR
worked around the unstable suffix with a toString(boolean, boolean)
overload, a Trigger.getYieldKey() helper, and a parallel
SourceTriggerYieldKey TrackableProperty so persistence had something
stable to key off.

Moving the suffix to WrappedAbility.getStackDescription (alongside the
existing (Targeting: ...) and [important] suffixes) makes
Trigger.toString() stable unconditionally, so WrappedAbility.yieldKey()
-- already stored in StackItemView.Key -- is the persistence key.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Comment thread forge-game/src/main/java/forge/game/spellability/SpellAbility.java Outdated
Comment thread forge-gui-desktop/src/main/java/forge/screens/match/views/VStack.java Outdated
Comment thread forge-gui/src/main/java/forge/interfaces/IGameController.java Outdated
MostCromulent and others added 4 commits May 5, 2026 21:24
Addresses review comments r3187997419 and r3188023459 on PR Card-Forge#10506.

SpellAbility/WrappedAbility.yieldKey() revert from getHostCard().getName()
to getHostCard().toString(). The earlier change collapsed the two-tier
scope distinction: AutoYieldStore.abilitySuffix() finds the "): "
boundary in Card.toString() to derive the per-ability key from the
per-card raw key. With getName() that boundary disappears, abilitySuffix
becomes a no-op, and per-card/per-ability scopes silently merge.

VStack.AbilityMenu's jmiAutoYield handler now reuses triggerYieldKey
and triggerAbilityScope.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Address review comment r3188140947 on PR Card-Forge#10506. The five trigger-decision
methods on IGameController (shouldAlwaysAcceptTrigger,
shouldAlwaysDeclineTrigger, setShouldAlwaysAcceptTrigger/Decline/Ask) all
funnel into AutoYieldStore's existing tri-state TriggerDecision enum
(ASK/ACCEPT/DECLINE). Lift the enum into the interface so the API matches
the underlying storage:

  TriggerDecision getTriggerDecision(String key);
  void setTriggerDecision(String key, TriggerDecision decision, boolean isAbilityScope);

Yield methods stay separate -- yield-priority on any SA is orthogonal to
auto-responding to optional-trigger prompts. Toggle pattern at call sites
becomes a single setTriggerDecision call with a conditional next-state.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@tool4ever tool4ever merged commit dcb5da0 into Card-Forge:master May 6, 2026
2 checks passed
@MostCromulent MostCromulent deleted the autotrigger branch May 6, 2026 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

GUI QOL Quality of Life

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants