Add network sync for auto-yield and trigger accept/decline#10355
Merged
tool4ever merged 6 commits intoCard-Forge:masterfrom Apr 16, 2026
Merged
Add network sync for auto-yield and trigger accept/decline#10355tool4ever merged 6 commits intoCard-Forge:masterfrom
tool4ever merged 6 commits intoCard-Forge:masterfrom
Conversation
Client-side auto-yield and trigger always-yes/always-no settings had no effect for remote network clients because preferences were only stored locally. Notifications are folded into AbstractGuiGame's setter methods so existing GUI call sites (VStack, VAutoYields, VGameMenu) work without modification. An instanceof NetGameController check ensures notifications only fire on the client side, avoiding re-entrancy when the server applies the received state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
RafaelHGOliveira
added a commit
to RafaelHGOliveira/forge
that referenced
this pull request
Apr 11, 2026
…c into AbstractGuiGame setters Architecture change per upstream feedback: instead of GUI call-sites (VStack, VAutoYields, VGameMenu) explicitly calling notifyAutoYieldChanged / notifyTriggerChoiceChanged, those calls are now triggered from the corresponding setters in AbstractGuiGame (setShouldAutoYield, setShouldAlwaysAcceptTrigger, setShouldAlwaysDeclineTrigger, setShouldAlwaysAskTrigger). An instanceof NetGameController guard ensures notifications only fire on the client side, preventing re-entrancy when the server-side PlayerControllerHuman applies received state back through the same setters. Also restores symbols lost from upstream merge (-X theirs): - IGuiGame: isRemoteGuiProxy(), isAutoPassingNoActions(), showPlayerDisconnected() - RemoteClientGuiGame: clientIndex field + getSlotIndex() - Missing imports: CompatibleObjectEncoder/Decoder (RemoteClient), ForgePreferences (InputPassPriority), FServerManager + MessageEvent (PlayerControllerHuman), Tracker (CompatibleObjectEncoder/Decoder)
tool4ever
reviewed
Apr 15, 2026
Relocate auto-yield and trigger-accept/decline preferences from the shared GUI layer (AbstractGuiGame/IGuiGame) into per-player controllers (PlayerControllerHuman/IGameController). Each human player now owns independent yield preferences. In local multiplayer, Player A auto-yielding to an ability no longer affects Player B. Network sync is simplified: NetGameController mirrors state locally for UI display and sends mutations directly via protocol methods that map 1:1 to IGameController. The TriggerChoice enum and notify* methods are removed in favor of direct method dispatch. Addresses review feedback on <!-- -->Card-Forge#10355. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
yea, looks better now imo I believe this also fixed the "disable all" not working on mobile, since that was for some reason interacting with the previously unused controller field possibly we can still reduce the footprint a bit by refactoring Accept/Decline/Ask methods pattern to the earlier approach, but maybe it's better to wait with that until we have a better understanding on how it should integrate with the more extended yield framework in the future 🤔 |
tool4ever
previously approved these changes
Apr 16, 2026
tool4ever
approved these changes
Apr 16, 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
2 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.
Cherry-picked from #9643 with architectural changes per tool4ever's feedback.
Closes #3699
Summary
Client-side auto-yield and trigger always-yes/always-no settings had no effect for remote network clients because preferences were only stored locally. This adds
notifyAutoYieldChangedandnotifyTriggerChoiceChangedprotocol methods to sync these settings from client to server.Rather than adding separate notification calls at each GUI call site (VStack, VAutoYields, VGameMenu across desktop and mobile), notifications are folded into
AbstractGuiGame's existing setter methods (setShouldAutoYield,setShouldAlwaysAcceptTrigger, etc.). Aninstanceof NetGameControllercheck ensures notifications only fire on the client side, which also avoids re-entrancy when the server-sidePlayerControllerHumanapplies the received state back through the same setters.Trigger choices are carried as a
TriggerChoiceenum (ASK / ALWAYS_YES / ALWAYS_NO) rather than bare ints.🤖 Generated with Claude Code