Make per-player UI a first-class API instead of a localPlayer block - #6
Conversation
Warcraft III is lockstep: frames must be allocated identically on every
client, and only the visibility flag may differ per player. The library
had no way to express that, so the natural thing for a consumer to write
was a localPlayer block around a component call:
if localPlayer == p
dialog.show()
Every component allocated lazily from its presentation methods
(show / setOpen / placeAt / withTooltip), so that one line created the
frames, their click triggers and their listener closures on a single
client. Frame events are synced, so the click then fired everywhere but
only that client resolved a listener and ran the map's callback - a local
branch reaching a synchronized sink.
Library side:
- Every component gains show(player) / hide(player) / setVisible(player,
bool). They call create() unconditionally first and only then apply the
local flag, so allocation is on the safe side of the guard by
construction: UIConfirmDialog, UIDialog, UIModalBackdrop, UISelect,
UISlider, UIEditBox, UITabs, UIBar, UIStatBar, UICheckbox, UISimpleBar,
UISimpleTexture.
- TableUiDefaultUi gains setXVisible(player, bool) overloads for the whole
HUD, so per-player default-UI work no longer depends on the consumer
remembering reserveDefaultUiHandles(). The global setters also stop
dereferencing a null frame on patches where it does not exist, and
command/inventory buttons are now cached like the other getters.
- New TableUiPerPlayer: perPlayerFrames(factory) builds one tree per owner
on every client, in owner order, and exposes owner-scoped visibility.
For UI each player must own independently.
- New TableUiSyncCheck (opt-in, not re-exported by the facade):
checkUiSync() syncs each player's UI allocation count and logs an error
naming any player whose count differs. Diagnostic only.
- Listener interfaces now pass the acting player first, matching
TextInputListener/ConfirmDialogListener which already did. A shared
widget fires for everyone, so a handler that assumes it belongs to one
player is a griefing hole. BREAKING: SliderListener, SelectListener,
SelectableListener, SelectableGroupListener, CheckboxListener and
ModalDismissCallback take a leading player (null when programmatic).
Two defects found in the same audit:
- UISlider only updated `value` and the value text from the synced event,
never the knob. A drag moves the knob on the dragging client alone, so
remote clients showed the new number against a stale knob. The handler
now re-asserts the knob on every client, which also brings that widget
state into the synced half. Re-entry terminates because the re-fired
event carries the same value.
- UITooltip.ondestroy called setTooltip(null), contradicting this repo's
own rule that setTooltip cannot be undone and that re-setting a pair can
crash on hover. It now only hides the box, which is what actually stops
the tooltip rendering.
MultiboardAttach is documented as global-only: unlike the rest of the
library it mutates synchronized multiboard state and creates a timer.
TableLayoutTest gains a perPlayerFrames demo panel. PerPlayerFrames has no
headless test: the compiletime interpreter cannot represent a framehandle,
so its build contract needs the WC3 run.
Brings the instruction files in line with the new API so agents and devs reach for it instead of rediscovering locality by hand. - AGENTS.md: "never write a localPlayer block around a library call" as a hard rule, perPlayerFrames for independent per-player UI, hideDefaultUi and MultiboardAttach called out as global-only, listeners pass the acting player. New packages added to Core Files. - WC3_FRAMEHANDLE_GUIDE.md: Multiplayer Safety now leads with the player-scoped overloads and explains why the local-block version reaches a synchronized sink, not just a cosmetic difference. Adds the checkUiSync diagnostic, and two consequences of synced frame events that surprise people in opposite directions - a handler runs on every client for any player's click, while widget state the player changed by hand (slider knob, edit box text) stays local. Records why removeTooltip only hides the box. - AI_USAGE.md: per-player step in the decision tree, per-player recipe, player-scoped dialog prompt, two checklist entries. - README.md: per-player UI section and feature bullet, per-player default HUD example.
2f2e42b to
35d64e1
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f2e42baf0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let root = factory.build(owner) | ||
| if root != null | ||
| owners.add(owner) | ||
| roots.add(root) |
There was a problem hiding this comment.
When the factory returns a normally visible frame—as both the documented slider.getFrame() example and the new card() demo do—the constructor stores it without first hiding it. showEach() later sets each root visible only for its owner, but it never hides that root on the other clients, where its default visibility remains true; consequently every player sees every owner's tree instead of only their own. Initialize each returned root as globally hidden before applying owner-scoped visibility.
AGENTS.md reference: AGENTS.md:L23-L23
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — confirmed and fixed in ab1e65c.
The diagnosis is exactly right: setVisible(owner, flag) resolves to if localPlayer == owner, so it only ever touches the owner's client. Every other client kept the root at its creation-time default of visible, and nothing in the class ever hid it there — so showEach() gave each player the full set of trees rather than their own.
The constructor now hides each root globally before any owner-scoped call can run, which makes owner-scoped show the only reveal and means a factory does not have to hide what it builds:
let root = factory.build(owner)
if root != null
root.hide() // globally, on every client
owners.add(owner)
roots.add(root)
ondestroy was also switched from per-owner setVisible(owner, false) to a global hide(), so teardown cannot leave a tree visible on any client.
Worth flagging that this pointed at the same bug one level down, fixed in the same commit: UISelect's dropdown is deliberately created in the OVERLAY band, outside the select's root, so that it renders above panels — which also means root visibility never covered it. setOpen() showed the menu globally, so one player opening a dropdown put it on every screen, and on an owner-scoped select it leaked the menu to players who cannot see the select at all. Opening is now scoped to the acting player via setOpenFor(player, bool); a hidden toggle receives no clicks, so on an owner-scoped select that is always the owner. Closing stays unconditional, which is always safe.
Verification: grill typecheck and grill test (78/78) pass and CI is green, but both of these change what a remote client sees, so they are still unconfirmed against a real two-client run — that is called out as outstanding in the PR description.
…pener Addresses the Codex review finding on PerPlayerFrames, plus a second bug the fix exposed. PerPlayerFrames stored the factory's root as-is. A freshly created frame defaults to visible and setVisible(owner, flag) only ever touches the owner's client, so nothing ever hid owner B's tree on owner A's screen: showEach() made each player see EVERY owner's tree instead of only their own. Roots are now hidden globally at construction, so owner-scoped show is the only reveal and the factory does not need to hide what it builds. That fix exposed the same class of bug one level down. UISelect's dropdown deliberately lives in the OVERLAY band, OUTSIDE the select's root, so it renders above panels - which also means root visibility does not cover it. setOpen() showed it globally, so one player opening a dropdown put it on every screen, and on an owner-scoped select it leaked the menu to players who cannot see the select at all. Opening is now scoped to the acting player via setOpenFor(player, bool); a hidden toggle receives no clicks, so on an owner-scoped select that is always the owner. Closing stays unconditional, which is always safe. This is also better shared-UI behaviour: a transient dropdown never belonged on everyone's screen. Both lists in PerPlayerFrames (and the participant list in the sync check) move from LinkedList to ArrayList, presized to the slot count. The access pattern is append-only with indexed lookup and no removal anywhere - the case ArrayList documents itself for - and presizing means one allocation with no resize copy. Iteration is by index; ArrayList has no iterator by design.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab1e65c582
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /** Shows or hides the slider FOR ONE PLAYER. Allocates for every client first, then flips only | ||
| that player's visibility flag - the safe shape. Never wrap this in a localPlayer block. */ | ||
| function setVisible(player p, bool flag) returns thistype | ||
| create().setVisible(p, flag) |
There was a problem hiding this comment.
When a newly constructed default-visible component such as UISlider receives show(p), create() makes its root visible on every client and this call only sets the flag on p's client, leaving every other client unchanged and still visible. The follow-up only added the initial global hide inside PerPlayerFrames; these component overloads still leak UI intended for one player unless callers happen to call global hide() first. Initialize default-visible component roots as globally hidden before an owner-scoped show.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, and worse than described — fixed in 3158eda.
It is not only UISlider: every component whose root is created visible had this, i.e. UISelect, UIEditBox, UITabs, UIBar, UIStatBar, UICheckbox, UISimpleBar and UISimpleTexture as well. Only UIConfirmDialog, UIDialog and UIModalBackdrop were safe, because those already ..hide() at creation.
Rather than repeat a latch in nine classes, the rule now lives in one place and is public, so consumer frames get the same guarantee:
public function framehandle.setVisibleForOwner(player p, bool flag)
if flag and not ownerScopedFrames.has(this)
ownerScopedFrames.put(this, true)
this.hide()
this.setVisible(p, flag)
I did not make the roots hidden at creation, which was the literal suggestion: components are created visible on purpose for the ordinary flow — build it, add it to a layout, everyone sees it — and flipping that default would silently blank existing consumers' UI. The latch fires only on the first player-scoped show, so the global path is untouched.
Scoped calls stay additive after the first, so showing to a set still works:
panel.show(p1) // p1 only
panel.show(p2) // now exactly p1 and p2
panel.hide(p1) // now p2 only
|
|
||
| /** Opens/closes the dropdown for every player. */ | ||
| function setOpen(bool flag) | ||
| setOpenFor(null, flag) |
There was a problem hiding this comment.
For callers of the existing setOpen(true) or toggle() API, passing null into the player-scoped setVisible overload does not mean “everyone”; that overload only mutates visibility when the local client matches the supplied player. Consequently open becomes true while the menu, options, and click catcher remain hidden on every client, and the next toggle merely closes this invisible state. Keep the global path using the non-player show() calls and reserve setOpenFor for a real player.
AGENTS.md reference: AGENTS.md:L61-L61
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I don't think this one holds — null does mean "everyone" here.
stdlib defines the player-scoped overload as an explicit null check, not a bare local-player comparison (wurst/_handles/Framehandle.wurst:278):
public function framehandle.setVisible(player p, bool flag)
if localPlayer == p or p == null
this.setVisible(flag)
So p == null short-circuits the guard and the visibility change applies on every client. All three frames on that path resolve to it: menu and the option frames are plain framehandles, and clickCatcher is a UIModalBackdrop whose setVisible(player, bool) delegates straight through. setOpen(true) therefore does show the menu globally, and open does not go out of step with what is on screen.
Happy to be corrected if you're reading a different stdlib version — this is against the wurstStdlib2 master that CI resolves.
The sibling report about a second player's toggle was real, though, and is fixed in 3158eda — see that thread.
| if flag == open | ||
| return |
There was a problem hiding this comment.
When a shared select is visible to multiple players and player A opens it, the synchronized open field becomes true even though only A sees the menu. If player B then clicks the toggle, toggleFor(B) requests false and closes A's menu instead of opening one for B; similarly, an explicit setOpenFor(B, true) returns early while A owns the open menu. Track the current opener alongside open, or transfer visibility when a different player requests the already-open state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid — fixed in 3158eda by taking the second option: open state is now tracked per player rather than as one shared flag.
private boolean array[MAX_SLOTS] openByPid
function toggleFor(player p)
setOpenFor(p, not isOpenFor(p))
setOpenFor(p, flag) now compares and writes only openByPid[p.getId()], so B's toggle no longer reads A's state: B opens their own menu while A's stays open, and setOpenFor(B, true) no longer early-returns because A owns the open menu. An outside click on the barrier and picking an option both close only for the acting player (setOpenFor(clicker, false) / setOpenFor(p, false)). setOpen(flag) still writes every slot for the genuine global case.
I went with per-player state over tracking a single opener because it is correct for the case a single opener still cannot express: a shared select that several players use at the same time.
Worth noting the geometry work in applyOpen (syncMenuOverlay, menuBackdrop.setAllPoints) deliberately stays global and runs identically on every client — only the setVisible calls are player-scoped, so nothing diverges but the visibility flag.
…per player Two more review findings, both valid. Component roots are created VISIBLE - correct for the ordinary flow (build it, add it to a layout, everyone sees it) but wrong for the per-player overloads: setVisible(p, flag) resolves to a local-player guard and so only touches p's client, leaving a default-visible root untouched (and therefore still visible) on every other client. show(p) on a fresh UISlider, UISelect, UIEditBox, UITabs, UIBar, UIStatBar, UICheckbox, UISimpleBar or UISimpleTexture put the UI on every screen. Only the dialogs and the modal barrier were safe, because they already create hidden. Rather than repeat a latch in nine classes, framehandle.setVisibleForOwner now owns the rule in one place: the FIRST player-scoped show hides the frame globally, then reveals it to the requested player. Later scoped calls stay additive, so showing to a set still works - show(p1); show(p2) leaves exactly p1 and p2. A global show()/hide() is untouched, so the ordinary layout flow is unchanged. The helper is public, so consumer frames get the same guarantee. UISelect's open state moves from one shared bool to per-player. The menu lives in the OVERLAY band outside the select's root, so scoping the open to the acting player (previous commit) meant a second player's toggle read the shared flag as "open" and closed the first player's menu instead of opening their own; an explicit setOpenFor(B, true) returned early for the same reason. Open is now tracked per slot, so several players can use one shared select independently, and an outside click or a pick closes only that player's menu. Not changed: the report that passing null into the player-scoped setVisible does not mean "everyone". Stdlib defines it as `if localPlayer == p or p == null`, so null is explicitly the global path that setOpen(flag) relies on.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3158eda89c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if players[i].isIngame() | ||
| participants.add(players[i]) |
There was a problem hiding this comment.
In maps with computer-controlled players, isIngame() also includes those playing slots, but no client has an AI slot as its local player and therefore nobody transmits for that slot through sync(p). Its callback never decrements pending, so finish() and the supplied listener never run, and every later check is rejected as already in flight. Restrict participants to active human-controlled clients (and handle departures while the check is pending).
Useful? React with 👍 / 👎.
Partially addresses the review note on TableUiSyncCheck. The valid half is the parenthetical: a player who leaves after their sync request is queued never transmits, so `pending` never reached zero. The listener would never fire, and because the entry guard rejects a call while `pending > 0`, every later check was refused for the rest of the game. checkUiSync now arms a timeout (uiSyncTimeout, default 10s) and reports whatever arrived. A per-check epoch counter makes a late or post-timeout callback from an earlier run a no-op, so it cannot drive `pending` negative or fire a second finish(); the callback also ignores a duplicate answer for a slot it already recorded. finish() now separates the two failure modes, which the previous version conflated. A genuine count DIVERGENCE still logs an error naming the local block as the cause. A MISSING answer logs a warning that says the check is inconclusive, not failed - reporting a departure as a desync would send people hunting a bug that is not there. The report callback returns false for either, with the non-responders named in `detail`. Not changed: the claim that isIngame() includes computer-controlled slots. stdlib defines it as slot PLAYING *and* controller MAP_CONTROL_USER, so AI slots are already excluded and every participant is a real client that can transmit. Added a comment recording that, since it is the exact question a reader will have at that line.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d19fb43b57
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for i = 0 to MAX_SLOTS - 1 | ||
| openByPid[i] = flag |
There was a problem hiding this comment.
After setOpen(true), all 28 entries—including slots with no client—remain marked open. If every active player then closes the dropdown individually by selecting an option or clicking outside, their own entries become false but the unused entries stay true, so isOpenForAnyone() still reports an invisible menu as open and the next global toggle() only clears those stale flags instead of reopening the menu. Restrict global open-state tracking to actual client slots, or make the aggregate check ignore unused slots.
AGENTS.md reference: AGENTS.md:L61-L61
Useful? React with 👍 / 👎.
Valid review finding. setOpen(true) marked all 28 slots open, including empty and AI ones. Those slots have no client and so can never close their own menu: once every actual player had closed theirs individually - by picking an option or clicking outside - the unused entries stayed true. isOpenForAnyone() then reported an invisible menu as open, and the next global toggle() spent itself clearing those stale flags instead of reopening the dropdown, so the first toggle after that sequence did nothing. setOpen now only flags slots that are actually ingame, and isOpenForAnyone() skips non-client slots as well, which also covers a player who leaves while their menu is open. Slot and controller state are lockstep values, so isIngame() here reads identically on every client and introduces no locality.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 598cf1c223
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /** `p` is the player whose drag produced the value, or null when the change came from setValue(). */ | ||
| public interface SliderListener | ||
| function onChange(real value) | ||
| function onChange(player p, real value) |
There was a problem hiding this comment.
Existing consumers that pass a one-argument callback to UISlider.onChange no longer typecheck because SliderListener.onChange(real) was replaced rather than supplemented; the same breaking change affects the public select and selectable listeners. Introduce a backward-compatible listener overload/type while exposing the acting-player variant separately so maps can upgrade without rewriting every callback.
AGENTS.md reference: AGENTS.md:L61-L61
Useful? React with 👍 / 👎.
| // globally, then reveals it to the requested player. Later scoped calls are additive, so showing to a | ||
| // set of players still works (show(p1); show(p2) leaves exactly p1 and p2). A global show()/hide() is | ||
| // untouched, so the ordinary layout flow behaves exactly as before. |
There was a problem hiding this comment.
When a globally visible component is temporarily hidden with hide(p) and later restored with show(p), the false call is not recorded in ownerScopedFrames; the later show is therefore treated as first use and calls this.hide() globally, unexpectedly removing the component from every other player's screen. Record the first owner-scoped hide as initialized too, while reserving the global hide for a genuinely first scoped show.
AGENTS.md reference: AGENTS.md:L22-L22
Useful? React with 👍 / 👎.
Valid review finding. setVisibleForOwner only recorded a frame as owner-scoped on a show, so the subtractive path was not tracked: hide(p) on a globally visible component correctly hid it from p alone, but the matching show(p) afterwards then looked like a first scoped show, fired the global hide, and pulled the component off every other player's screen. The frame is now marked owner-scoped on the first scoped call in either direction, while the global hide stays reserved for a genuinely first scoped SHOW. hide(p) means "take it away from p", not "take it away from everyone", and show(p) after it restores exactly that player.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Follow-up to a desync audit of the library. The library's own code was clean — no
GetLocalPlayer, noBlzDestroyFrame, no gameplay natives outsideMultiboardAttach, and input already read through the syncedBlzGetTriggerFrame*accessors. What it lacked was a way for a consumer to express "only this player sees it" without writing the unsafe thing themselves.The hole this closes
Every component allocated lazily from its presentation methods (
show(),setOpen(),placeAt(),withTooltip()), and there were no player-scoped overloads. So the natural line to write was:That is not cosmetic. Frame events are synced, so the button's click fires on every client — but only the client that ran
create()resolves a listener inClosureFrames, so the map's callback executes on exactly one machine. A local branch reaching a synchronized sink.Library
show(player)/hide(player)/setVisible(player, bool)onUIConfirmDialog,UIDialog,UIModalBackdrop,UISelect,UISlider,UIEditBox,UITabs,UIBar,UIStatBar,UICheckbox,UISimpleBar,UISimpleTexture. Each callscreate()unconditionally, then applies the local flag — allocation is on the safe side of the guard by construction.TableUiDefaultUigainssetXVisible(player, bool)for the whole HUD, so per-player HUD work no longer depends on rememberingreserveDefaultUiHandles()(which stays, now belt-and-braces). Also fixes the global setters dereferencing a null frame on patches where it doesn't exist, and caches command/inventory buttons like the other getters.TableUiPerPlayer—perPlayerFrames(factory)builds one tree per owner on every client in owner order, and exposes owner-scoped visibility. For UI each player owns independently.TableUiSyncCheck(opt-in; deliberately not re-exported by the facade, since importing it registers stdlib sync events) —checkUiSync()syncs each player's UI allocation count and logs an error naming any player whose count differs.TextInputListener/ConfirmDialogListenerwhich already did.Two defects found in the same pass
UISliderknob drift. The handler updatedvalueand the value text from the synced event but never the knob. A drag moves the knob only on the dragging client, so everyone else saw the new number against a stale knob. The handler now re-asserts the knob on all clients, bringing that widget state into the synced half. Re-entry terminates because the re-fired event carries the same value (changed == false) — safe whether the engine firesSLIDER_VALUE_CHANGEDsynchronously or deferred.UITooltip.ondestroycalledsetTooltip(null), contradicting this repo's own rule (WC3_FRAMEHANDLE_GUIDE.md: "setTooltipcannot really be undone", and re-setting a pair can crash on hover). It now only hides the box, which is what actually stops it rendering.MultiboardAttachis documented as global-only: unlike the rest of the library it mutates synchronized multiboard state (MultiboardSetRowCount/MultiboardSetItemsStyle) and creates a timer on first attach.Breaking change
Listener interfaces take a leading
player(null when the change was programmatic rather than a click):SliderListeneronChange(real)onChange(player, real)SelectListeneronSelect(int, string)onSelect(player, int, string)SelectableListeneronSelect(bool)onSelect(player, bool)SelectableGroupListeneronSelect(int)onSelect(player, int)CheckboxListeneronChange(boolean)onChange(player, boolean)ModalDismissCallbackrun()run(player)Migration is adding the parameter.
UISelect.selectOption(index)andUISelectableGroup.select(chosen)keep their old arity as programmatic overloads that passnull.Validation
grill typecheck --quiet— passes.grill test— 78/78, zero compiler warnings.TableLayoutTest.wurstgains aperPlayerFramesdemo panel: each player should see exactly one card naming themselves (in a single-player run, only P1's).PerPlayerFrameshas no headless test on purpose — the compiletime interpreter cannot represent a framehandle (it treats a null one as non-null), so a test would have meant contorting library code to satisfy the harness. Its build contract needs the real run.Review follow-ups
PerPlayerFramesroots were visible by default (caught in review). A fresh frame defaults to visible andsetVisible(owner, flag)only touches the owner's client, so nothing hid owner B's tree on owner A's screen —showEach()showed every tree to every player. Roots are now hidden globally at construction, so owner-scoped show is the only reveal.UISelect's dropdown had the same bug one level down. The menu lives in the OVERLAY band, outside the select's root, so root visibility never covered it: one player opening a dropdown put it on every screen, and on an owner-scoped select it leaked the menu to players who cannot see the select. Opening is now scoped to the acting player (setOpenFor); a hidden toggle receives no clicks, so on an owner-scoped select that is always the owner. Closing stays unconditional. This is better shared-UI behaviour too — a transient dropdown never belonged on everyone's screen.LinkedList→ArrayListinPerPlayerFramesand the sync check, presized to the slot count. Append-only with indexed lookup and no removal anywhere is the caseArrayListdocuments itself for; presizing gives one allocation and no resize copy. Iteration is by index (ArrayListhas no iterator by design). Note this needs awurstStdlib2new enough to haveArrayList— CI already resolvesmaster, so no manifest change was required.