Remove game options of unused mods from game launch info - #7057
Conversation
Mostly generated by ai
This fixes the removal logic incorrectly removing options with the same key as a default option that is not from the AIOpts table. This fixes M28AI's UnitCap option.
lL1l1
added
area: ui
|
Warning Review limit reached
Next review available in: 26 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe lobby records default and mod-owned game option keys, removes non-default options contributed only by disabled mods during launch, reuses the computed simulation mod list, and adds related LuaDoc annotations. A changelog entry documents the fix. ChangesMod-aware game option filtering
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant LobbyLaunch
participant Mods
participant ModOptionMapping
participant GameInfo
LobbyLaunch->>Mods: Get selected simulation mods
LobbyLaunch->>ModOptionMapping: Check option ownership
ModOptionMapping->>GameInfo: Remove options unused by selected mods
LobbyLaunch->>GameInfo: Set GameMods from simMods
LobbyLaunch->>GameInfo: Broadcast Launch
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lua/ui/lobby/lobby.lua (1)
154-154: Prefer stable mod identifiers over display names for matching.Using
nameforModOptionMappingandenabledModNamescan misclassify options when different mods share a display name. Use a stable identifier (uid/UID) when available.💡 Proposed hardening
- ModOptionMapping[t.key][ModData.name] = true + local modIdentifier = ModData.uid or ModData.UID or ModData.name + ModOptionMapping[t.key][modIdentifier] = true- for _, modInfo in gameInfo.GameMods do - enabledModNames[modInfo.name] = true + for _, modInfo in gameInfo.GameMods do + local modIdentifier = modInfo.uid or modInfo.UID or modInfo.name + enabledModNames[modIdentifier] = true endAlso applies to: 2355-2358
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lua/ui/lobby/lobby.lua` at line 154, The code uses ModData.name when keying ModOptionMapping and enabledModNames which can collide across mods; change the keys to use a stable identifier (e.g., ModData.uid or ModData.UID) wherever ModOptionMapping[t.key][ModData.name] and enabledModNames are set or checked (also update the similar occurrences around the enabledModNames block at the other location), and add a safe fallback to ModData.name only if uid/UID is nil to preserve behavior for older mods; ensure all lookups and assignments that previously used ModData.name are updated to the chosen stable identifier to keep mapping consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lua/ui/lobby/lobby.lua`:
- Around line 2353-2389: The GameOptions filtering runs too late; move the whole
filtering block (the code that builds enabledModNames, iterates
gameInfo.GameOptions, uses IsDefaultOption, ModOptionMapping and
IsOptionUsedByGivenMods, collects keysToRemove and nils them from
gameInfo.GameOptions) so it executes before the launch data broadcast is
created/sent (i.e. before the function that prepares/broadcasts the launch
payload to clients). Also ensure any mod resolution this filtering depends on
(the population of gameInfo.GameMods and ModOptionMapping) is performed
beforehand so enabledModNames is accurate; after moving, confirm the broadcast
uses the now-filtered gameInfo.GameOptions.
---
Nitpick comments:
In `@lua/ui/lobby/lobby.lua`:
- Line 154: The code uses ModData.name when keying ModOptionMapping and
enabledModNames which can collide across mods; change the keys to use a stable
identifier (e.g., ModData.uid or ModData.UID) wherever
ModOptionMapping[t.key][ModData.name] and enabledModNames are set or checked
(also update the similar occurrences around the enabledModNames block at the
other location), and add a safe fallback to ModData.name only if uid/UID is nil
to preserve behavior for older mods; ensure all lookups and assignments that
previously used ModData.name are updated to the chosen stable identifier to keep
mapping consistent.
Four things players reported, in the order they were hit. **"Ack! Unable to load replay from gpgnet" when watching a live replay.** Live spectating never updated the install it launches. `play_file` stages the exact build its replay pins, so the replay install sits on whatever old replay was watched last, and nothing else ever touches that directory. A live stream is always the *current* build, and FA refuses one whose build does not match. `watch_live` now runs `ensure_latest_game_version` for the game's featured mod before the stream is opened, deliberately before: the replay server starts sending on the handshake, so a patch download after that point would be spent buffering frames nobody is reading yet. **"Live Replays Workaround", ported from the Python client.** Settings, Game: stream a live replay through a Windows named pipe instead of the local TCP proxy (`game/pipe_live_replay` there). Off by default, because the pipe freezes the window while catching up and ends the replay abruptly with no army selection or statistics. It exists because the TCP path still hits the engine's oversized-`ScenarioInfo` bug until `FAForever/fa#7057` lands upstream, and the pipe does not. Includes the Python client's 2048-byte terminator, without which FA simply hangs when the stream ends. The Python client's sibling switch, "Run in own process", is deliberately not ported: replays already run on the separate `replaydata` install here, so the switch could only take that away. **"could not prepare simulation mods: <folder> is already installed".** FAF mod uids are per *version*, so a host on an older release asks for a uid nobody holding the newer one has. `ensure_game_mods` downloaded it and the extraction then refused, because the folder was occupied. It now reads the archive's own top-level folder without extracting, and a collision stops the join having changed nothing on disk, reporting the versions involved. The user answers a prompt, and only then is the installed version removed, through `uninstall_mod` so its uid also leaves `game.prefs`. Mods that do not collide are still installed straight away; only the destructive step waits. Same posture as the Python client's `downloadMod`. **Installed mods were not clickable.** The description lives in the vault, so reading it meant leaving the client. The card body now opens a detail dialog with the description, version, folder and uid, and a link through to the mod's vault entry.
2 participants
Description of the proposed changes
Removes unused mod options from scenario info that was causing live replays to not work according to this message on zulip #general > Live replay @ 💬
On initialization:
On game launch, remove all options that are not a default option and are not used by any enabled mods.
Testing done on the proposed changes
Playing the game without any mods gives a small ScenarioInfo.Options table (1320 bytes).
Playing the game with a mod that has lots of options such as "Swarm Survival Vanilla" produces no errors in the log from the mod and printing the scenario info shows the expected options. The debugging also does not show that any removed options that were used by the mod.
M28AI adds an option with the key "UnitCap" which is the same as the base game unit cap option's key. M28AI's option is not removed and its value of 10000 works as expected as shown by the scoreboard.
Ending the game and entering a new skirmish with adjusted mods and without restarting the application does not cause any issues as long as the lobby file was not dirtied in the session.
Checklist
Summary by CodeRabbit
Bug Fixes
Documentation