Fix multiplayer stuck UI when selecting entities (e.g. proliferate)

[shoeless]

Problem

When a player must select entities (e.g. proliferate targets) in a network game, the client can end up with the target cards highlighted but OK/Cancel disabled — a stuck selection UI.

InputSelectEntitiesFromList calls tempShowZones on the controller's gui from the EDT. For a remote player that routes through RemoteClientGuiGame.syncAndSendAndWait(), blocking the server EDT for a full network round trip. That delays showMessageInitial (which sends updateButtons to the client), so under any latency the client sees the highlighted cards before the buttons are enabled.

Fix

Keep the tempShowZones call for everyone, but for the remote proxy move the blocking wait off the EDT (background thread):

• the remote client still receives tempShowZones and opens its zone panels (no behavior removed — desktop net clients keep zone auto-open),
• its reply sets zonesShown for onStop cleanup; hideZones is null-safe in the rare case the selection resolves before the reply,
• local guis (including the desktop host's own player) are completely unchanged,
• zonesShown becomes volatile for the cross-thread write.

The fire-and-forget alternative was rejected: the protocol replies based on the method's declared return type, so an unawaited reply would hit ReplyPool.complete with no registered slot.

Single file, forge-gui only.

🤖 Generated with Claude Code

[tool4ever]

what about simply overriding showMessageInitial so we can control the order directly?

[shoeless]

Done — moved it into showMessage() after super.showMessage(), so the prompt and buttons go out before the round trip starts (showMessageInitial itself is final, but it just delegates here). The constructor now only builds the zone list.

Two small guards because the buttons are now live during the in-flight tempShowZones: the call is EDT-only (a refresh() arriving on the netty thread during the round trip must not start a second one), and onStop falls back to hiding the requested set so the client's temp-shown zones can't be left open if OK lands before the reply does.

Local play is unchanged in practice — prompt and zones land in the same EDT dispatch, so they still appear together.

[tool4ever]

hmn, when I debugged this constructor being called from the game thread I still see showMessage() running before these EDT calls...

[dschartman]

Nice find — I hit the other half of this same call site recently and I've been digging in this area for a bit (#11535 — a host crash from tempShowZones/hideZones reaching collectDeltas on the EDT while the game thread was live; there's some history in the PR discussion there about the delta-sync design intending all graph walks to stay on the game thread).

One thing I noticed in the new placement: for a remote player the tempShowZones call still runs on the EDT via syncAndSendAndWait, so the graph walk and the blocking wait stay on the EDT — deferred, but the same thread. When I was experimenting with this site I tried running the call on the calling game thread for remote controllers instead (the way the other syncAndSendAndWait dialogs work), which as a side effect also got the zones to the client before the prompt/buttons — but I may well be missing a constraint that makes the EDT the right place here. Is the isGuiThread() gate mainly about preventing the double-invoke from a netty-thread showMessage, or is there another reason it needs to stay on the EDT? Happy to share what I tried if useful.

[tool4ever]

yea, there are multiple things that need to be researched here:

• can the network round trip of tempShowZones simply be avoided?
• why does EDT not finish the buttons update first?
• is running it from Netty thread another undesired desync risk?

[MostCromulent]

I asked Claude to have a look at this:

Q1 — can the network round trip of tempShowZones simply be avoided?

Yes. The payload is only PlayerView + Set<ZoneType> — contents never go on the wire, the client renders the zone from its own copy. The return value is just the subset actually opened, and its only use is being fed back to hideZones. CMatchUI.openZones:1379 already discards it and hands the unfiltered set to restoreOldZones → hideZones, so void is what the targeting path does today. Mobile (MatchScreen:808) and the headless client (HeadlessNetworkGuiGame:92) are pass-through stubs.

Two consumers to update, not one: CMatchUI.openZones and PlayerControllerHuman:952-966 (the floating-hand reveal). And on the server side it turns RemoteClientGuiGame.tempShowZones from syncAndSendAndWait into syncAndSend — the wait goes away, the graph walk stays.

One catch: FloatingZone.hide has no record of who opened a window, so a zone the user opened manually would be closed when the selection ends — the filtered return is the only thing preventing that, since show() returns false when already visible. A provenance flag fixes it, and the pre-existing restoreOldZones case with it. It has to be cleared in the FDialog.setVisible override in FloatingCardArea rather than in hide(), because the X button, Esc, double-click and closeAll() all bypass hide() — and above that override's isVisible() == b0 early return.

ProtocolMethod:49 has to become Void.TYPE in the same commit. Note it degrades quietly rather than failing loudly: getMethod() warns and falls back to getMethodNoArgs() → null.

Q2 — why does the EDT not finish the buttons update first?

Because by the time the button work is queued, the EDT is already inside the blocking call. The render task doesn't lose a race — it doesn't exist yet.

The constructor runs on the game thread, so invokeInEdtNowOrLater defers and queues task A (updateZones + tempShowZones). The EDT picks A up and enters tempShowZones, which for a remote player is syncAndSendAndWait and holds the EDT for the round trip. Only then does the game thread reach showAndWait() → setInput() → InputProxy.update → invokeInEdtLater(showMessage), queuing task B. updateButtons is the last statement of InputSelectManyBase.showMessage(), so it sits at the tail of a task queued behind one already running and blocked.

Unmodified master, one host and one remote client, remote player selecting:

ctor:return                     Game-0            t=...098282300
edtBlock:enter                  AWT-EventQueue-0  t=...098297300
edtBlock:tempShowZones-pre      AWT-EventQueue-0  t=...098527000
proxy:post-showMessage          Game-0            t=...098981000    (task B queued)
edtBlock:tempShowZones-post     AWT-EventQueue-0  t=...113987100    (+15.46 ms)
showMessage:updateButtons-pre   AWT-EventQueue-0  t=...114604000
showMessage:updateButtons-post  AWT-EventQueue-0  t=...114813000

The difference isn't total latency, it's what the EDT is doing. The host's own player in the same run reaches its buttons in 16.55 ms against the remote player's 16.53 ms — near identical. But the local case spends 14 ms with task A sitting in the queue and 158 µs executing it, while the remote case spends 15.46 ms blocked inside it, unable to repaint or process input.

On showMessage() appearing to run before the EDT block — I couldn't reproduce that ordering. In both InputSelectEntitiesFromList samples the EDT block completes first. One candidate: InputSelectManyBase.refresh() also calls showMessage(), and refresh() comes from onCardSelected/onPlayerSelected on the per-message Game BT threads, so a click during a stepped session gives you a showMessage with no EDT block near it. If you still have the session, the thread name on that frame would settle it.

Q3 — is running it from a Netty thread another undesired desync risk?

Two thread families worth separating. Server-side protocol methods dispatch to Game BT threads (GameProtocolHandler → FThreads.invokeInBackgroundThread), not Netty — and those are engine threads: named "Game BT<n>", so they pass ThreadUtil.isGameThread() and GameAction.invoke runs engine code inline on them. The genuine Netty-thread graph walk is the reconnect path at FServerManager:930.

For ordinary prompt rendering the EDT is mostly safe, because showMessage is dispatched from setInput(), which is immediately followed by awaitLatchRelease() — so Game-0 is normally on the latch by the time the EDT walks. Two caveats, both visible in the trace: nothing synchronises the two (the gap ranges from 7.8 µs to 27 ms across 13 samples — syncPoint() is an empty synchronized block for visibility only), and Game-0 being parked doesn't mean no mutator, since a Game BT thread can be running engine code concurrently.

The specific problem here is work posted from a constructor, which runs before showAndWait():

edtBlock:enter                 AWT-EventQueue-0  t=...098297300
edtBlock:tempShowZones-pre     AWT-EventQueue-0  t=...098527000
showAndWait:setInput-pre       Game-0            t=...098850500
showAndWait:park               Game-0            t=...099087000
edtBlock:tempShowZones-post    AWT-EventQueue-0  t=...113987100

The EDT is inside tempShowZones → updateGameView → collectDeltas for 560 µs while Game-0 is still executing. That's the # 11535 shape — the EDT walking the graph while the engine runs — measured here in an ordinary game with nothing crashing.

Moving the call to showMessage() narrows that window to microseconds but doesn't close it — nothing waits for the park.

[tool4ever]

thanks, so it sounds like the threading part should still be looked at in a follow-up

and here tempShowZones vs. openZones were probably designed without netplay in mind and can probably be consolidated, ideally with more logic moved from input to controller
though the how might depend on if any of these calls can end up being nested together 🤔