Restore mobile auto-yield persistence across matches#10448
Restore mobile auto-yield persistence across matches#10448tool4ever merged 5 commits intoCard-Forge:masterfrom
Conversation
There was a problem hiding this comment.
I think I actually misunderstood the issue and didn't even consider the different behaviour on mobile.
But AI missed the bigger problem - it doesn't even transfer between games of the same match now on Desktop either.
And that's something we certainly need depending how this feature is configured:

Should probably just revise it to support these variants uniformly on both platforms 🤔
- this additional "Each Session" is certainly interesting for Desktop too
- converting that into full persistence ("Each Install") instead could be even more helpful
- should probably still keep the Match-boundary around for cases where user is involved in multiple ones simultaneously
- might even bring back the old per-UI style as option?
imo AI can have some freedom on relocating how we store this data if it helps with a cleaner architecture
sorry for the extra work
…ch persistence Adds two new scope tiers to the auto-yield mode dropdown. AutoYieldStore (per-LobbyPlayerHuman) holds Game/Match/Session buckets; PersistentYieldStore (file-backed singleton at <userDir>/auto-yields.dat) holds the Install tier. Moving storage off PlayerControllerHuman onto LobbyPlayerHuman makes Per Ability (Each Match) yields actually survive between games of a match. Supersedes prior commit ce56d72 (boolean preference gated by isLibgdxPort); folded into the dropdown and made uniform across desktop and mobile. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ce56d72 to
33b9ae1
Compare
|
Alright, Take 2 on this. See what you think of latest commit. New "Each Session" and "Each Install" yield persistence options added across-platform and integrated into existing drop-down menus on both desktop and mobile, replacing implementation on first commit. Architectural reference document here.
|
|
I still need to test a bit but this looks pretty neat now - I'd imagine some combo players will be happy to manage these files for their decks 👍 And while I originally thought "Each Session" might not be needed if you can just use "Each Install" and then clear when you want to start fresh it seems the way AI implemented it makes both useful - since you can switch between them if you're doing some testing session and don't want to modify your more regular defaults :) |
Parallels the tiered Auto-Yield design from <!-- -->Card-Forge#10448 with a persistent Always-Yes / Always-No for repeating optional triggers (e.g. Barrin's Codex, Spare Dagger). New "Auto-Trigger Mode" preference offers four scopes: Per Card (Each Game) — the default, preserving today's per-instance, per-game behaviour — or Per Ability at Each Match / Session / Install. "Auto-Triggers" menu entry and listing dialog mirror Auto-Yields on desktop and mobile, with a separate Disable-All toggle. Install-tier decisions persist to the existing auto-yields.dat file (schema is additive, so existing installs load unchanged). Trigger.getStableKey() / getYieldKey() produce user-decision keys paralleling SpellAbility.toUnsuppressedString() / yieldKey(). API mirrors yields: setShouldAlways{Accept,Decline,Ask}Trigger(String, boolean). Tier is resolved per-side from the preference and does not cross the wire. Engine-internal int trigger IDs are untouched — load-bearing for game-state queries, separate from the user-decision layer. ProtocolMethod wire entries for the three trigger methods change from (Integer) to (String, Boolean); new setDisableAutoTriggers(Boolean) and setDisableAutoYields(Boolean) wire entries are added — the existing yield-side setDisableAutoYields only updated client-local state, so the server's remoteAutoYieldsDisabled flag never flipped. Breaks old-client/new-server compatibility on these entries, same pattern as <!-- -->Card-Forge#10355/<!-- -->Card-Forge#10448. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Summary
Closes #10442.
The bug — Auto-yield state used to persist across matches for mobile users, but it appears this was an architectural accident. Before #10355, auto-yields were instance fields on
AbstractGuiGame. On mobile,MatchControlleris a singleton (public static final MatchController instance = new MatchController()), so itsautoYieldsset survived between matches. On desktop,CMatchUIis created fresh per match, so the same code never persisted yields there. No user-facing preference, and behaviour was asymmetric between platforms.#10355 moved auto-yield state from
AbstractGuiGametoPlayerControllerHuman(per-player ownership — to prevent hotseat cross-contamination and to simplify network sync). This was architecturally correct, but eliminated mobile's accidental singleton-backed persistence. Mobile users noticed.The fix
PlayerControllerHumanandNetGameControllernow hold yield state in two parallel stores:isLibgdxPort()AND a new preferenceUI_PERSIST_AUTO_YIELDS_ACROSS_MATCHESis enabled (defaulttrue). Lives for the Forge session.A selector (
useSharedYieldStore()) picks between them on each access; all getters/setters route through it. The new toggle is exposed on the mobile Settings screen next toUI_AUTO_YIELD_MODE.Why it's safe
isLibgdxPort()gate hard-blocks the shared-store path regardless of the preference value. Desktop users who've never seen persistence won't start seeing it, even if the pref is flipped on viaforge.preferences.PlayerControllerHumaninstances (identified viagui instanceof RemoteClientGuiGame) always use per-instance fields. The Add network sync for auto-yield and trigger accept/decline #10355 invariant that "Player A's yields don't affect Player B's server-side view" still holds.clearAutoYields()only clears per-instance state soHostedMatch.endCurrentGame()'s end-of-match cleanup no longer wipes the session-persistent shared store. Explicit user actions (removing a yield in the dialog, toggling "Disable All") still work because they go throughsetShould*which route through the selector.Scope / open design questions
This is deliberately a minimal fix for the regression — it restores previous mobile behaviour, nothing more. PR could be amended to go further and address:
autoYieldskeys are stable ability text and could be serialised toforge.preferencesso yields survive Forge restarts, not just session lifetime.triggersAlwaysAcceptcannot be persisted this way (trigger IDs are runtime-assigned).🤖 Generated with Claude Code