-
Notifications
You must be signed in to change notification settings - Fork 839
Network multiplayer optimization (UI improvements) #9671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
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
Improves user experience during network games: - Waiting timer shows player name and elapsed time (CMatchUI, InputLockUI) - Connection errors display detailed messages instead of generic failure - Adds CONN_ERROR_PREFIX constant for structured error handling - New localization strings for waiting and error messages UI-only changes, no game logic or delta sync functionality. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use GameView data only in findWaitingForPlayerName() to work on both host and client (fixes null Game on network clients) - Add findPriorityPlayer() using PlayerView.getHasPriority() - Use localized string prefixes in isWaitingMessage() instead of hardcoded English strings - Add volatile to timer fields for thread safety Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
tool4ever
reviewed
Feb 5, 2026
Addresses review feedback from tool4ever: consolidate timer logic into the existing awaitNextInput() mechanism in AbstractGuiGame instead of adding separate timer systems in InputLockUI and CMatchUI. Changes: - Revert InputLockUI.java to master (remove ~100 lines of timer code) - Revert CMatchUI.java to master (remove ~185 lines of timer/prompt code) - Enhance AbstractGuiGame.updatePromptForAwait() with: - Player name lookup (shows "Waiting for [Player]..." in network games) - Elapsed time display (shows "(5s)" or "(1:23)" suffix) - 1-second reschedule loop for network games only - Remove unused lblYieldingWaitingForPlayer from en-US.properties Benefits: - 1 timer system instead of 3 (reuses existing awaitNextInputTimer) - Platform-neutral: works on mobile automatically - No duplicate code: single findWaitingForPlayerName() implementation - Smaller diff: 155 lines total vs 369 before refactor Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This reverts commit 9cfa358.
…8-UP5xm Revert "Refactor waiting timer to AbstractGuiGame (PR 9671 feedback)"
tool4ever
reviewed
Feb 6, 2026
forge-gui-desktop/src/main/java/forge/screens/match/CMatchUI.java
Outdated
Show resolved
Hide resolved
Reapplies 9cfa358 which was reverted in 2afb047. Consolidates timer logic into the existing awaitNextInput() mechanism in AbstractGuiGame instead of separate timer systems in InputLockUI and CMatchUI. - Revert InputLockUI to simple lockUI input (remove ~100 lines) - Revert CMatchUI.showPromptMessage to simple pass-through (remove ~185 lines) - Enhance AbstractGuiGame.updatePromptForAwait() with player name lookup, elapsed time display, and 1-second reschedule for network games - Remove unused lblYieldingWaitingForPlayer from en-US.properties Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These server-side chat features were missed during the earlier extraction
to NetworkPlay/chat (since merged to master). Moving here as UI-focused.
- Broadcast ready state with player count when toggling ready
- Show (Host) indicator next to host's chat messages
- Distinguish host login ('Lobby hosted by') vs player join ('joined the lobby')
- Detect and announce player name changes
- Fix duplicate message display by removing MessageEvent from LobbyInputHandler
- Use 'left the lobby' instead of 'left the room' for consistency
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… feedback) Adds an overloaded isNetworkplay(IGuiGame) that checks whether a specific game instance is networked, rather than relying solely on the global static boolean which reflects whichever lobby last called setNetworkplay(). This follows tool4ever's proposed API from PR Card-Forge#9671. New API surface: - IGuiGame.isNetGame(): default method returning false. Only NetGuiGame overrides this to return true, so no other implementors need changes. - IGuiBase.hasNetGame(): implemented by GuiDesktop and GuiMobile, both delegating to the existing static flag. This serves as the fallback when no IGuiGame reference is available. - GuiBase.isNetworkplay(IGuiGame): if a game reference is provided, queries game.isNetGame() directly; otherwise falls back to getInterface().hasNetGame(). The existing no-arg isNetworkplay(), the static field, and setNetworkplay() are all unchanged. Migrated call sites (10 total) — these pass their available IGuiGame: - AbstractGuiGame: 2 sites use AbstractGuiGame.this / this - FControlGameEventHandler: 1 site uses matchController field - PlayerControllerHuman: 3 sites use getGui() - InputBase: 2 sites use controller.getGui() - MatchController (mobile): 5 sites use this Unmigrated call sites (no-arg fallback, no IGuiGame in scope): - FThreads (utility class, no game context) - VCardDisplayArea, MatchScreen (mobile views, no direct IGuiGame ref) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…apsed time PR Card-Forge#9671 feedback: AbstractGuiGame.scheduleTimerUpdate() sent showPromptMessage over the wire every second in network games, creating constant traffic just for a clock counter. Replace with a one-shot showWaitingTimer protocol signal. The server sends it once after "Waiting for Player X", then each client (CMatchUI via javax.swing.Timer, MatchController via java.util.Timer) runs its own local 1s timer for elapsed time display. Any new showPromptMessage from the server cancels the local timer. In AbstractGuiGame: delete scheduleTimerUpdate(), getElapsedTimeString(), and awaitStartTime (all dead after this change). Add onWaitingStopped() hook so subclasses can clean up local timers when cancelAwaitNextInput() fires. Also fix host not showing network-specific messages. A network match has three IGuiGame contexts: host's local GUI (CMatchUI), server-side proxy (NetGuiGame), and remote client. Only NetGuiGame returned true from isNetGame() — the host's CMatchUI inherited the default false, so it showed "Waiting for opponent" and never triggered showWaitingTimer. Fix: AbstractGuiGame now tracks a networkGame field with setNetworkGame(), called by FServerManager.getGui() when creating the host's local GUI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…anager Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
tool4ever
approved these changes
Feb 8, 2026
Contributor
tool4ever
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
MostCromulent
added a commit
to MostCromulent/forge
that referenced
this pull request
Feb 9, 2026
Resolve 7 conflicts from PR Card-Forge#9671 (UI improvements) merge to master: - .gitignore: accept master's .claude ignore - CSubmenuOnlineLobby/OnlineLobbyScreen: accept CONN_ERROR_PREFIX error display - NetConnectUtil: use detailed connection error messages - GameClientHandler: keep delta sync init path, add gui.setNetGame() and version string from master, preserve client username for test harness - FServerManager: accept pattern matching instanceof, name-change and ready-state broadcasting, keep NetworkDebugLogger disconnect logging - IGuiGame: keep both delta sync methods and showWaitingTimer/isNetGame Validated with 10-game batch test (9/10 pass, 1 known 4p desync bug). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
THIS PR HAS BEEN SPLIT FROM #9642.
Dependency
This branch is has been refactored to be fully independent from other features in #9642.
Features
Waiting timer and connection error improvements for network games:
timeout, no route to host)
Files Changed
CMatchUI.javaInputLockUI.javaNetConnectUtil.javaCSubmenuOnlineLobby.javaCONN_ERROR_PREFIXfor detailed error displayOnlineLobbyScreen.javaCONN_ERROR_PREFIXfor detailed error displayForgeConstants.javaCONN_ERROR_PREFIXconstanten-US.properties