Add lobby option to extend command sources when an army is abandoned #6997
Conversation
📝 WalkthroughWalkthroughAdds a "union when disconnected" lobby option and localization strings, implements AIBrain.ShareControlWithAllies and uses it on abandon when enabled, localizes AI chat text, wires focus-change UI hooks to clear mode text, and adds minor sim/init logging and docs clarification. Changes
Sequence Diagram(s)sequenceDiagram
participant AIBrain as AIBrain (abandoned)
participant Ally as Allied Human Brain
participant Sim as Sim / SetCommandSource
participant Net as Sync (SyncAIChat)
participant UI as Tabs/UI
AIBrain->>AIBrain: AbandonedByPlayer()
AIBrain->>AIBrain: check CommonArmy == "UnionWhenDisconnected"
alt option enabled
AIBrain->>Ally: iterate allied human brains
loop for each eligible ally
AIBrain->>Sim: SetCommandSource(targetArmyIndex, sourceHumanIndex) (0-based)
AIBrain->>Net: SyncAIChat(share notification)
AIBrain->>AIBrain: SPEW log
end
AIBrain->>AIBrain: return true (share occurred)
else
AIBrain->>AIBrain: existing defeat handling
end
Note over Net,UI: Focus change -> UserSync -> tabs.focusArmyChanged -> ClearModeText -> UpdateModeDisplay
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✏️ Tip: You can disable this entire section by setting 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
🤖 Fix all issues with AI agents
In @loc/US/strings_db.lua:
- Around line 4716-4717: The lobui_CAUponDisconnectDescription localization
entry is missing the gameplay constraint that defeated armies are not shared;
update the lobui_CAUponDisconnectDescription string to include the same clause
used in the fallback help text (e.g., append "and the share condition does not
trigger" or equivalent phrasing) so the tooltip matches the behavior described
in lobbyOptions.lua's help text.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
engine/Sim.lualoc/US/strings_db.lualua/AIChatSorian.lualua/aibrain.lualua/simInit.lualua/ui/lobby/lobbyOptions.lua
🧰 Additional context used
🧬 Code graph analysis (2)
lua/simInit.lua (2)
engine/Core.lua (2)
SPEW(408-409)IsAlly(237-238)engine/Sim.lua (1)
SetCommandSource(1177-1178)
lua/aibrain.lua (3)
engine/Core.lua (2)
IsAlly(237-238)SPEW(408-409)engine/Sim.lua (3)
SetCommandSource(1177-1178)IsGameOver(714-715)GetGameTick(624-625)lua/SimSyncUtils.lua (1)
SyncAIChat(15-19)
🪛 Luacheck (1.2.0)
lua/aibrain.lua
[error] 491-491: expected '=' near 'end'
(E011)
🔇 Additional comments (8)
lua/AIChatSorian.lua (1)
19-19: LGTM: Localization support added correctly.The localization lookup with fallback ensures backward compatibility while supporting the new localized chat messages introduced in this PR.
lua/simInit.lua (2)
460-460: LGTM: Helpful debug logging added.The logging additions provide useful debugging information for the new union army control feature without affecting runtime behavior.
478-479: LGTM: Debug logging added for common army setup.Consistent with the union army logging approach.
lua/aibrain.lua (2)
517-545: Well-structured abandon handling with union control support.The modification correctly:
- Records abandonment time and logs the event
- Conditionally triggers union control sharing based on the lobby option
- Bypasses default defeat flow when control sharing succeeds
The logic is sound, assuming the
continuekeyword issue inShareControlWithAlliesis resolved.
474-513: No issue: FAF Lua supports thecontinuekeyword.The
continuekeyword is fully supported in FAF's extended Lua implementation. This is evident from the extensive use ofcontinuethroughout the codebase (200+ instances across multiple files including lua/simInit.lua, lua/EffectUtilities.lua, lua/WeaponPriorities.lua, and many AI/UI files), with no syntax errors or failures reported. The code is correct as written.Likely an incorrect or invalid review comment.
engine/Sim.lua (1)
1174-1176: LGTM: Documentation clarified for SetCommandSource.The updated documentation clearly specifies that both parameters use 0-based indexing and explains that
sourceHumanIndexrefers to actual command sources as shown inGetClientSessions. This clarification aligns with the implementation inlua/aibrain.lualine 500.lua/ui/lobby/lobbyOptions.lua (2)
195-195: Nice formatting cleanup.The indentation adjustment improves consistency with the rest of the options structure.
188-192: LGTM! New option integrated correctly.The new "Union control upon disconnect" option is well-positioned as a middle ground between default and full union control. The localization keys are properly defined in
loc/US/strings_db.lua, and the backend implementation correctly handles theUnionWhenDisconnectedkey inlua/aibrain.lua. Structure follows the established pattern with consistent indentation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @lua/aibrain.lua:
- Line 494: The inline comment above the check "if brain.AbandonedAt then
continue end" is incorrect (it says allied players "are also abandoned")—update
that comment to say allied players "are NOT also abandoned" or "that are not
abandoned" so it matches the actual logic; locate the comment near the
brain.AbandonedAt check in aibrain.lua and replace the wording to correctly
reflect the skip behavior.
🧹 Nitpick comments (1)
lua/aibrain.lua (1)
525-529: Optional: Consider simplifying the condition formatting.The multi-line condition with separate checks is readable but could be slightly more compact. This is purely a style preference.
♻️ Alternative formatting
- if ScenarioInfo.Options.CommonArmy == "UnionWhenDisconnected" and - - -- do not trigger this behavior when this army is already defeated - (not self:IsDefeated()) - then + -- Attempt union control if enabled and army is not already defeated + if ScenarioInfo.Options.CommonArmy == "UnionWhenDisconnected" and not self:IsDefeated() then
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
loc/RU/strings_db.lualoc/US/strings_db.lualua/aibrain.lua
✅ Files skipped from review due to trivial changes (1)
- loc/RU/strings_db.lua
🚧 Files skipped from review as they are similar to previous changes (1)
- loc/US/strings_db.lua
🧰 Additional context used
🧬 Code graph analysis (1)
lua/aibrain.lua (3)
engine/Core.lua (2)
IsAlly(237-238)SPEW(408-409)engine/Sim.lua (3)
SetCommandSource(1177-1178)IsGameOver(714-715)GetGameTick(624-625)lua/SimSyncUtils.lua (1)
SyncAIChat(15-19)
🪛 Luacheck (1.2.0)
lua/aibrain.lua
[error] 491-491: expected '=' near 'end'
(E011)
🔇 Additional comments (4)
lua/aibrain.lua (4)
522-524: Good addition of abandonment tracking and logging.Recording the abandonment timestamp and logging the event will help with debugging and understanding game flow. The use of
GetGameTick()provides a precise timestamp for tracking when the abandonment occurred.
525-535: Verify the feature works as intended with the new option.The integration logic looks correct: the code checks if the UnionWhenDisconnected option is enabled and the army isn't already defeated, then attempts to share control with allies. The early return on line 533 properly prevents the army from being defeated when sharing succeeds.
However, this depends on the correctness of
ShareControlWithAllies(see previous comments about index calculation andcontinuekeyword).Test scenarios to verify:
- Player disconnects with UnionWhenDisconnected enabled → allies can control the army
- Player disconnects with option disabled → army is defeated immediately
- Already-defeated player's ally disconnects → control is not shared (line 528 check)
- Last player on a team disconnects → entire team defeated (mentioned in PR objectives)
491-491: Remove thecontinuekeyword concern—it is valid in this codebase.The
continuekeyword is a supported language feature widely used throughout the repository (50+ instances), including in similar patterns in lua/simInit.lua and other files. The static analysis error is a false positive and does not reflect actual syntax validity.
486-500: sourceIndex must only count human brains that pass all filtersThe sourceIndex counter increments for every human brain (line 492), but some humans are skipped by the ally/abandoned checks (lines 494-496). This breaks the alignment with GetSessionClients indexing. Per the SetCommandSource documentation, sourceHumanIndex must "include only actual sources."
Move the sourceIndex increment to after all filtering checks to ensure it only counts humans that will actually receive command source updates.
| if brain.BrainType ~= 'Human' then continue end | ||
| sourceIndex = sourceIndex + 1 | ||
|
|
||
| -- only take into account allied players are also abandoned |
There was a problem hiding this comment.
Fix incorrect comment.
The comment states "are also abandoned" but the logic on line 496 actually skips abandoned players (if brain.AbandonedAt then continue end). The comment should read "are NOT also abandoned" or "that are not abandoned".
📝 Proposed fix
- -- only take into account allied players are also abandoned
+ -- only take into account allied players that are not abandoned🤖 Prompt for AI Agents
In @lua/aibrain.lua at line 494, The inline comment above the check "if
brain.AbandonedAt then continue end" is incorrect (it says allied players "are
also abandoned")—update that comment to say allied players "are NOT also
abandoned" or "that are not abandoned" so it matches the actual logic; locate
the comment near the brain.AbandonedAt check in aibrain.lua and replace the
wording to correctly reflect the skip behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @lua/ui/game/tabs.lua:
- Around line 1203-1209: ClearModeText mutates the upvalue table modes while
iterating which can skip keys and leave stale entries; instead reset modes by
replacing it with a new empty table (or build a separate list of keys and clear
those) and then call UpdateModeDisplay(); locate the ClearModeText function and
the modes upvalue and replace the in-loop nil assignments with an assignment
that reinitializes modes to {} (or clear via a collected key list) before
invoking UpdateModeDisplay().
🧹 Nitpick comments (1)
lua/ui/game/tabs.lua (1)
1272-1274: Exported focusArmyChanged wrapper is fine; consider making it a normal function for consistency.
This works, but for readability/consistency with other exported handlers in the codebase, considerfunction focusArmyChanged()instead of assigning an anonymous function.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lua/UserSync.lualua/ui/game/tabs.lua
🔇 Additional comments (1)
lua/UserSync.lua (1)
294-300: focusArmyChanged() is safe to call before tabs UI initialization. The handler only clears a local module-levelmodestable and callsUpdateModeDisplay(), which safely checksif controls.modeDisplay thenbefore accessing UI state. The defensive pattern matches other handlers like massfabs and is idempotent—it works correctly whether Create has been called or not.
4z0t
left a comment
4z0t
left a comment
There was a problem hiding this comment.
I'm not a professional translator, but overall it is good
|
I casually forgot to make a changelog snippet 😞 |
Description of the proposed changes
Union mode is a game mode where players can switch focus army to issue commands to units that are not theirs. It is originally designed to be used in a co-op or tutorial setting. It enables players to collaborate. It requires a certain level of trust.
The last year or so union mode is being (ab)used as a measurement to reduce the impact of disconnecting players. This is a much broader scope, and as a result there are complaints about players abusing union mode to forcefully share resources, units or worse.
With this pull request we introduce a step between union mode being completely enabled or disabled: union mode is enabled for the players that end up disconnecting from the game. Upon a disconnect the defeat conditions are not applied. Instead, all allied players, defeated or playing, can then switch focus to the abandoned army and issue commands accordingly.
A new lobby option that is in between the regular experience and flat out applying union control to all armies
Testing done on the proposed changes
union-control-upon-disconnect-01.mp4
Other details
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.