Feat/formation bridge - #45
Open
justinneils wants to merge 2 commits into
Open
Conversation
…utton The formation buttons went straight to SendChatMessage, so every formation change posted to party/raid and drew one "Formation set to: x" whisper back per bot. They now call the bridge's new RUN~FORMATION verb, which applies the change natively and sends nothing to chat in either direction. The chat path stays as a fallback for when the server-side module is absent or the bridge handshake has not completed. Also adds the Far formation, which mod-playerbots has always supported but the button table never listed. It follows at AiPlayerbot.FarDistance (20 yards by default) instead of the 1.5 of the other follow formations. Far reuses the generic formation.blp: there is no formation_far.blp artwork. The tooltip is added to enUS only (plus enGB, same language) -- enUS is registered as the AceLocale default, and GetLocaleString falls back to it, so the other seven locales pick it up without inventing translations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LibDBIcon-1.0 resolves LibDataBroker-1.1 through LibStub at load time and
calls error() when it is missing:
local ldb = LibStub("LibDataBroker-1.1", true)
if not ldb then error(DBICON10 .. " requires LibDataBroker-1.1.") end
The TOC listed LibDBIcon-1.0.lua immediately before LibDataBroker-1.1.lua.
TOC entries execute in order, so LibDBIcon ran one line before the library
it depends on had registered itself, throwing on every login:
LibDBIcon-1.0.lua:39: LibDBIcon-1.0 requires LibDataBroker-1.1.
Both files were already shipped; only the order was wrong. Swap them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR adds native formation command execution through the communication bridge, handles formation acknowledgements, adds the Far formation option, and retains chat-based fallback selection. It also adds localization text and changes library load order. ChangesFormation command flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant FormationButton
participant Comm
participant Bridge
FormationButton->>Comm: Call RunFormationCommand
Comm->>Bridge: Send FORMATION command
Bridge-->>Comm: Return FORMATION_ACK
Comm-->>FormationButton: Apply formation state
Suggested reviewers: 🚥 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Core/MultiBotComm.lua`:
- Line 424: The formation flow currently treats Comm.Send transport success as
command success instead of waiting for FORMATION_ACK. In Core/MultiBotComm.lua
lines 424-424, update Comm.RunFormationCommand to expose native formation
support or completion status separately from send success; in
UI/MultiBotFormationUI.lua lines 32-49, update the selected icon only after an
acknowledgement with executed == 1, and invoke the chat fallback when native
support is unavailable or the acknowledgement rejects the command.
In `@Locales/MultiBotAceLocale-enGB.lua`:
- Line 365: Revise the execution attribution in the tips.format.far tooltip to
state that the command executes through the bridge or group chat. Apply the same
wording in Locales/MultiBotAceLocale-enGB.lua at line 365 and
Locales/MultiBotAceLocale-enUS.lua at line 365.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 36b24f08-1d84-469c-89a6-a18955e32323
📒 Files selected for processing (5)
Core/MultiBotComm.luaLocales/MultiBotAceLocale-enGB.luaLocales/MultiBotAceLocale-enUS.luaMultiBot.tocUI/MultiBotFormationUI.lua
| state.formationSeq = (tonumber(state.formationSeq) or 0) + 1 | ||
| local token = tostring(math.floor(safeNow() * 1000)) .. "-formation-" .. tostring(state.formationSeq) | ||
|
|
||
| return Comm.Send("RUN", "FORMATION~" .. scope .. "~" .. urlEncodeField(target) .. "~" .. token .. "~" .. urlEncodeField(command)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not treat a successful transport write as a successful formation change.
Comm.RunFormationCommand returns after Comm.Send. It does not wait for FORMATION_ACK. The UI then changes the root icon and closes the menu before it knows that any bot applied the command. If the server rejects the formation, FORMATION_ACK reports executed == 0, but the fallback does not run and the UI shows a formation that did not apply.
Core/MultiBotComm.lua#L424-L424: expose formation support or command completion separately from send success.UI/MultiBotFormationUI.lua#L32-L49: update the selected icon only after a successful acknowledgement. Use the chat fallback when native formation support is unavailable or rejects the command.
📍 Affects 2 files
Core/MultiBotComm.lua#L424-L424(this comment)UI/MultiBotFormationUI.lua#L32-L49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Core/MultiBotComm.lua` at line 424, The formation flow currently treats
Comm.Send transport success as command success instead of waiting for
FORMATION_ACK. In Core/MultiBotComm.lua lines 424-424, update
Comm.RunFormationCommand to expose native formation support or completion status
separately from send success; in UI/MultiBotFormationUI.lua lines 32-49, update
the selected icon only after an acknowledgement with executed == 1, and invoke
the chat fallback when native support is unavailable or the acknowledgement
rejects the command.
| ["tips.format.circle"] = "Circle Formation\n|cffffffffBots arrange in a circle around you, facing outwards.|r\n\n|cffff0000Left-click to select|r\n|cff999999(Executed by: Raid, Party)|r", | ||
| ["tips.format.chaos"] = "Random Formation\n|cffffffffEach Bot follows you on their own.\nThey line up with you in random locations and face in random directions.|r\n\n|cffff0000Left-click to select|r\n|cff999999(Executed by: Raid, Party)|r", | ||
| ["tips.format.shield"] = "Shield Formation\n|cffffffffBots line up in the front, on the left and right side, facing and aligned in your direction.|r\n\n|cffff0000Left-click to select|r\n|cff999999(Executed by: Raid, Party)|r", | ||
| ["tips.format.far"] = "Far Formation\n|cffffffffBots follow at a distance instead of crowding you, closing back in only when they fall further than AiPlayerbot.FarDistance (20 yards by default).\n\nUseful while stealthed or scouting.|r\n\n|cffff0000Left-click to select|r\n|cff999999(Executed by: Bridge)|r", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the execution attribution for the fallback path.
The tooltip says Executed by: Bridge, but UI/MultiBotFormationUI.lua uses MultiBot.SelectToGroup when native execution is unavailable. State that the command can execute through the bridge or group chat.
Locales/MultiBotAceLocale-enGB.lua#L365-L365: revise the execution attribution.Locales/MultiBotAceLocale-enUS.lua#L365-L365: apply the same revised attribution.
📍 Affects 2 files
Locales/MultiBotAceLocale-enGB.lua#L365-L365(this comment)Locales/MultiBotAceLocale-enUS.lua#L365-L365
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Locales/MultiBotAceLocale-enGB.lua` at line 365, Revise the execution
attribution in the tips.format.far tooltip to state that the command executes
through the bridge or group chat. Apply the same wording in
Locales/MultiBotAceLocale-enGB.lua at line 365 and
Locales/MultiBotAceLocale-enUS.lua at line 365.
This was referenced Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit