Skip to content

Phase label fix for multiplayer#8335

Closed
tovernaar123 wants to merge 5 commits intoCard-Forge:masterfrom
tovernaar123:master
Closed

Phase label fix for multiplayer#8335
tovernaar123 wants to merge 5 commits intoCard-Forge:masterfrom
tovernaar123:master

Conversation

@tovernaar123
Copy link
Copy Markdown

Me and my friends noticed that when playing forge multiplayer only the host has the correct phase labels enabled (others seem to skip main phase 1 and 2), this pull request should fix that.
This is my first pull request for this project so if you preferer something to be different feel free to ask.

@tool4ever tool4ever added the GUI label Aug 9, 2025
Comment thread forge-gui-desktop/src/main/java/forge/screens/match/CMatchUI.java
final GameView gameView = getGameView();
if (hasLocalPlayers() || gameView.isMatchOver()) {
//Otherwise only the host will save in a multiplayer match.
writeMatchPreferences();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this means in local game preferences will get written an additional time?
should probably have some condition to avoid that...
also is this really only relevant for Desktop?

thanks for looking into this

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I assume the same problem is also on android but I would not really know how to fix that, is their some shared logic file I could change that would effect both gui's? And yes I should add an extra check to make sure no 2 writes happen

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some debugging it seems to me that either host or client gets to save
but it depends on who acts in ViewWinLose first

doesn't look like we have a more generic solution available yet that could catch it when that class just stops its displaying

so just a check for netplay would be enough imo, so the double saving is limited to that mode at least

and for android @kevlahnota might later adapt some parts

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could not really find a nice way to check for a netgame so I have added a simple boolean check I hope that that is fine too.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh are you sure a new CMatchUi object is created after each game? otherwise this breaks saving after the first one...
what about GuiBase.isNetworkplay()?

@github-actions
Copy link
Copy Markdown

This PR has not been updated in a while nad has been marked on stale. Stale PRs will be auto closed

@tool4ever
Copy link
Copy Markdown
Contributor

@MostCromulent
want to check if this still relevant + make a fresh PR if fixing it is viable? 🙏

@MostCromulent
Copy link
Copy Markdown
Contributor

MostCromulent commented Feb 20, 2026

want to check if this still relevant + make a fresh PR if fixing it is viable? 🙏

TBH I haven't been able to replicate the problem, but I think that's due to local network testing conditions using two instances of forge reading the same user preferences. The AI assures me this is still an issue with the default preferences on remote network play so I'll look at submitting a separate PR for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

GUI keep no stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants