Skip to content

Add Modes#1041

Draft
keithharvey wants to merge 4 commits into
beyond-all-reason:masterfrom
keithharvey:sharing_tab
Draft

Add Modes#1041
keithharvey wants to merge 4 commits into
beyond-all-reason:masterfrom
keithharvey:sharing_tab

Conversation

@keithharvey

@keithharvey keithharvey commented Aug 19, 2025

Copy link
Copy Markdown

Closes #1040

Summary

Creates a dedicated Sharing tab that loads the game's modes/sharing/*.lua presets, applies per-mode values/locks/visibility, and shows ranked eligibility. Enables clean separation between sharing policy configuration and gadget implementation.

Changes

  • New Sharing tab with Mode dropdown and organized category sections
  • Load and apply declarative modes: set values, lock controls, hide rows
  • Read the declarative ModeConfig presets from modes/sharing/
  • Visual lock indicators with read-only overlays
  • Display "Not Ranked" badge and communicate ranked eligibility to lobby (Note: this is currently ranked=true for every sharing mode)

Screenshot of BAR changes without this PR

image This screenshot shows the BAR side WITHOUT the Chobby side. As you can see, it still works just fine, and this demonstrates specifically what this Chobby PR specifically does:
  • populate the Sharing Mode dropdown
  • hides and locks mod options based on the selected sharing mode

Related PRs

BAR: beyond-all-reason/Beyond-All-Reason#5704
Architecture: beyond-all-reason/Beyond-All-Reason#8018
Issue: #1040

Why

Moves sharing configuration from scattered individual modoptions to coherent, mode-driven policies. Players choose high-level intent (Enabled, Disabled, Easy Tax, Tech Core, Customize) rather than manually configuring multiple interdependent options.

Screenshots

(See the bar PR)

LLM Disclosure

The Chobby part of this PR was generated with grok-code-fast-1. I’d already written and reviewed and had a JSON version, then manually implemented the lua sharing mode type, each of the modes and modoption changes on the BAR side. I asked the model to update Chobby to match the new declarative format—needed only minor fixes, and it performed well. LGTM after full review.

@AntlerForce

Copy link
Copy Markdown
Collaborator

Marked as draft because I tested and this isn't working as-is. With BAR on branch pr/5074 and BYAR-Chobby on branch pr/1041 clicking on Skirmish does nothing and produces the following error:
[t=00:00:20.320549][f=-000001] [Chili] Error: in `Battle Room Window`:singleplayerWrapper : [string "LuaMenu/Widgets/gui_modoptions_panel.lua"]:1432: attempt to index global 'json' (a nil value) [t=00:00:20.322711][f=-000001] [Chili] Error: stacktrace: [C]: in UseArchive [string "LuaMenu/Widgets/gui_modoptions_panel.lua"]:1439: in LoadModoptions [string "LuaMenu/Widgets/gui_battle_room_window.lua"]:950: in SetupInfoButtonsPanel [string "LuaMenu/Widgets/gui_battle_room_window.lua"]:3209: in InitializeControls ... (18 calls) [string "LuaHandler/Utilities/specialCallinHandlers.lua"]:229 [string "libs/chiliui/chili/controls/object.lua"]:277: in SetParent [t=00:00:20.322772][f=-000001] [Chili] Error: In widget: Battle Room Window
infolog (22).txt

@keithharvey keithharvey force-pushed the sharing_tab branch 2 times, most recently from 6a15372 to f2c74d8 Compare October 24, 2025 05:05
@keithharvey keithharvey marked this pull request as ready for review October 24, 2025 05:23
@keithharvey keithharvey changed the title Add declarative sharing modes system Add Sharing Modes Oct 25, 2025
@keithharvey keithharvey marked this pull request as draft October 25, 2025 18:13
@keithharvey

keithharvey commented Oct 25, 2025

Copy link
Copy Markdown
Author

Converting this to draft again. It is technically ready for review (and entirely functional) on this side but the BAR side is not quite feature complete yet so holding off until the whole package is ready to go.


isProgrammaticUpdate = true
if not allowRanked then
localModoptions["ranked_game"] = "0"

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.

When a mode has allowRanked false this forces ranked_game to '0', but I don't see anywhere that sets it back when you switch to a mode that does allow ranked. So if I pick a non-ranked mode and then switch back to a ranked one, won't ranked_game stay at '0'? And since it goes out through SetModOptions as a normal value change, I think it actually gets sent to the host, not just shown locally. It's also easy to miss because SetRankedModeAllowed just below doesn't seem to be defined anywhere, so nothing updates the battle-room UI. Could this restore ranked_game on the allowRanked path?

end

if mode.modOptions then
for optKey, rule in pairs(mode.modOptions) do

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 only sets or clears lockedOptions for keys that are in the new mode's modOptions, and lockedOptions itself looks like it's only reset when the window is first created, not here. So if mode A locks an option and you then switch to a Customize/retain mode that doesn't list it, won't that key keep its old lockedOptions entry and then get overwritten with the server value on Accept? I think someone who had set that option would see it silently revert. Would resetting lockedOptions at the top of applyMode, the same way modeLocked is rebuilt, fix it?

mode_key = params[1]
kv_params = params[2:]

category_data = _as_dict(_modes().get('categories')).get(category)

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 calls .get('categories') straight on whatever _modes() returns. If the fetched or cached json is ever not an object (a list, say), won't that throw an AttributeError right into the perl bridge? The accesses below it go through _as_dict but this first one doesn't. Could the top level be wrapped the same way?

if isinstance(spec, dict) and spec.get('locked')
)
for (key, _val) in settings:
if key in locked_keys:

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.

The user's key gets lowercased up at line 209, but locked_keys is built from the mod_options keys as-is. So if a mode locks 'ResourceMult' and someone passes resourcemult=something, won't this check miss it and apply the override anyway? I think BAR option names are all lowercase in practice so it may not bite today, but it makes the lock weaker than it looks. Lowercasing both sides would close it.


for url in urls:
try:
with urllib.request.urlopen(url, timeout=_FETCH_TIMEOUT) as resp:

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 is a synchronous urlopen on what I understand is SPADS's single thread, with a 5s timeout and up to two URLs, so a slow or unreachable release could stall the host for around 10s on the first !mode after a rehost. From what I can see the vote-eligibility check reaches it too, which a normal player can trigger. It's once per version so not constant, but a multi-second freeze of the autohost still seems bad. Could this be backgrounded or moved to load time?

@burnhamrobertp

Copy link
Copy Markdown
Contributor

I'm not deep in the SPADS side or a lot of this Lua, so some of this might be me misreading it.

  1. Fetching game_modes.json over the network at runtime. The plugin pulls it from a GitHub release the first time someone runs !mode after a rehost. Is there a reason it has to be fetched at runtime instead of shipped inside the game archive the host already has synced? I get that the host can't regenerate it itself since it's baked in CI from the modes lua, but if CI is already producing the file, couldn't it be bundled into the archive so the host just reads it locally? My worry with the release fetch is that the failure modes are all silent: if the asset is missing, or the host's network is locked down, or the slug doesn't match, the host just quietly serves stale or empty data, which seems painful to debug for whoever is running the host.

  2. How do we guarantee the host's modes match the version players are actually on? From what I can tell the right file is picked by slugifying the game's display name, and the schemaVersion field in the json isn't read by anything. If the pinned asset isn't there it falls back to a rolling channel file that isn't tied to the hosted version. Won't that let the host apply one version's settings while clients read a different version's modes out of the archive, with nothing flagging the mismatch? Even just checking schemaVersion and logging loudly when it falls back would help, I think.

  3. The expansion logic lives in two places. The rule that turns a mode into its full set of modoptions is in the Lua resolver (moderesolver.lua) and again in the CI generator that bakes the json, in two different languages, and I don't see anything testing that they agree. Since the drift between copies looks like what caused several of the regressions this PR is already fixing, would a shared fixture or a test that runs both and compares be worth it? Otherwise I think it just drifts again the next time someone touches one side.

Add the server-side ModeCommand SPADS plugin backing the Chobby sharing
tab. !mode <category> <key> [opt=val ...] expands a mode preset to its
full modoption set (a bare !mode applies the whole preset) and levels
each option identically to the built-in bSet, rejecting unknown option
keys instead of silently setting them.

Game-mode/preset data is fetched directly via the plugin and pinned to a
short-sha game version, reading the host-regenerated game_modes.json
(which replaces the committed sharing_presets.json; the generated file is
gitignored). The plugin records the selector key so the client can
reflect an externally-set mode.
Add the sharing tab to the modoptions panel and battle room: pick a
sharing mode, route the category change through the !mode plugin, and
reflect a mode set externally (live-refreshing the panel when it changes
under us). Mode descriptions show as hover tooltips.

Introduce a shared ModeResolver (libs/liblobby/lobby/moderesolver.lua)
that resolves a mode + user deviations to a full modoption set, used by
both the panel and skirmish so single-player expands a mode locally with
the same logic the SPADS plugin mirrors server-side. !mode deviations are
scoped to the mode's own options, and only deviations are sent (guarded
against the 256-char say cap).

Rounding/retain fixes along the way: round number values to their step in
the editbox seed, locked overlay, and summary; honor retainValues for
non-sticky modes; reset category to defaults when switching mode; hide
subsection headers with no visible options; and stop leaking
subheader/separator pseudo-rows as modoptions.
GetDesyncGameStates logged "Failed to load" then fell through to
string.len(infolog) on a nil result. Wrap processing in the else branch
so a failed VFS.LoadFile just logs and skips.
Configuration.password is false when rememberPassword is off; the old
unconditional auto-login passed that boolean to lobby:Login, which then
gsub'd a boolean in TextEraseNewline. Gate auto-login on a string
password (or steamMode) so manual login still works.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sharing Modes

3 participants