p2a-outfit-chatless - #54
Conversation
|
Warning Review limit reached
Next review available in: 34 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 bridge now negotiates outfit support through ChangesOutfit capability flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Player
participant MultiBotOutfitUI
participant MultiBotComm
participant Bridge
Player->>MultiBotOutfitUI: Request outfit list or command
MultiBotOutfitUI->>MultiBotComm: Submit operation when bridge is connected and capable
MultiBotComm->>Bridge: Send outfit request with persistence and creation flags
Bridge-->>MultiBotComm: Return command result
MultiBotComm-->>MultiBotOutfitUI: Forward result metadata
MultiBotOutfitUI-->>Player: Display localized feedback
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![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: 5f3ac177ff
ℹ️ 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 MultiBot.allowLegacyChatFallback ~= true then | ||
| if bridgeState then | ||
| bridgeState.lastError = "OUTFIT_CAPABILITY_UNAVAILABLE" | ||
| end | ||
| return false |
There was a problem hiding this comment.
If a list was loaded and the bridge subsequently disconnects or loses OUTFIT_V1, clicking Equip, Replace, Update, Reset, or New reaches this branch under the default no-fallback configuration. Every caller ignores the false return, and this branch only updates the internal bridge.lastError, so the visible status remains unchanged and the click appears to succeed or do nothing without explaining that no command was sent; set an unavailable/failure status before returning.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
UI/MultiBotOutfitUI.lua (1)
972-1009: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winShow an explicit failure when the outfit request cannot be sent.
RequestListmarks the UI as loading immediately, but then skips the bridge/API/whisper path andFinishListreplaces the status withloadedorno_outfits. Use the setbridgeState.lastErrorfor the failure case, such as"OUTFIT_CAPABILITY_UNAVAILABLE"when legacy chat fallback is disabled, so users do not see a silent timeout.🤖 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 `@UI/MultiBotOutfitUI.lua` around lines 972 - 1009, The RequestList failure path must explicitly update the waiting UI instead of allowing the delayed FinishList callback to report loaded or no_outfits. When no bridge/API request succeeds and legacy chat fallback is disabled, set bridgeState.lastError to "OUTFIT_CAPABILITY_UNAVAILABLE" and ensure the corresponding wait button reflects the failure before returning false.Core/MultiBotComm.lua (1)
2616-2640: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPass an explicit creation flag for outpost update feedback.
OutfitUI:CreateFromCurrent()runsoutfitName .. " update"butComm.ApplyOutfitCommandPayload()passescommand.persistto the UI, whileshowOutfitCommandFeedback()uses that value aswasCreate. This makes feedback for future non-persist create commands selectfeedback_updateinstead offeedback_created. Pass a separate creation indicator throughRunOutfitCommand,RunOutfitCommand(), andHandleBridgeCommandResult()instead of reusing persistence status.🤖 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` around lines 2616 - 2640, Pass a dedicated creation flag through the outfit command flow: update Core/MultiBotComm.lua lines 2616-2640 in Comm.ApplyOutfitCommandPayload and UI/MultiBotOutfitUI.lua lines 1074-1133 across RunOutfitCommand, CreateFromCurrent, and HandleBridgeCommandResult so feedback receives whether the command created an outfit independently of command.persist. Use that creation flag as wasCreate in showOutfitCommandFeedback, preserving persistence solely for storage behavior and selecting feedback_created for create commands, including non-persist ones.
🤖 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.
Outside diff comments:
In `@Core/MultiBotComm.lua`:
- Around line 2616-2640: Pass a dedicated creation flag through the outfit
command flow: update Core/MultiBotComm.lua lines 2616-2640 in
Comm.ApplyOutfitCommandPayload and UI/MultiBotOutfitUI.lua lines 1074-1133
across RunOutfitCommand, CreateFromCurrent, and HandleBridgeCommandResult so
feedback receives whether the command created an outfit independently of
command.persist. Use that creation flag as wasCreate in
showOutfitCommandFeedback, preserving persistence solely for storage behavior
and selecting feedback_created for create commands, including non-persist ones.
In `@UI/MultiBotOutfitUI.lua`:
- Around line 972-1009: The RequestList failure path must explicitly update the
waiting UI instead of allowing the delayed FinishList callback to report loaded
or no_outfits. When no bridge/API request succeeds and legacy chat fallback is
disabled, set bridgeState.lastError to "OUTFIT_CAPABILITY_UNAVAILABLE" and
ensure the corresponding wait button reflects the failure before returning
false.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c7bacc4f-c452-43a0-bf52-08742a87a04a
📒 Files selected for processing (10)
Core/MultiBotComm.luaLocales/MultiBotAceLocale-deDE.luaLocales/MultiBotAceLocale-enGB.luaLocales/MultiBotAceLocale-enUS.luaLocales/MultiBotAceLocale-esES.luaLocales/MultiBotAceLocale-frFR.luaLocales/MultiBotAceLocale-koKR.luaLocales/MultiBotAceLocale-ruRU.luaLocales/MultiBotAceLocale-zhCN.luaUI/MultiBotOutfitUI.lua
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdaaea7f40
ℹ️ 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".
| and bridgeState.connected == true | ||
| and bridgeState.outfitCapable == true | ||
|
|
||
| if not bridgeOutfitCapable and MultiBot.allowLegacyChatFallback ~= true then |
There was a problem hiding this comment.
When bot A's outfits are displayed and the user opens bot B while the bridge is disconnected or has not advertised OUTFIT_V1, RequestList(B) has already changed self.botName and the window title before this early return, but it leaves A's entries and selectedName visible. The action buttons therefore associate A's outfits with B; if the bridge reconnects before another refresh, an Equip/Replace/Update/Reset click can send an A outfit name to B. Preserve entries only for an unavailable same-bot refresh, and clear them when the requested bot changes.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c536bc541
ℹ️ 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".
| self.entries = previousEntries | ||
| self.selectedName = previousSelectedName |
There was a problem hiding this comment.
When switching from bot A to bot B while the bridge appears capable, a failed RequestOutfits(B) restores A's entries and selection even though self.botName and the title remain B. The action buttons can then send A's outfit names to B if a later command succeeds; preserve the old data only for a same-bot refresh, and otherwise leave B's list empty. Fresh evidence beyond the earlier offline-switch report is this newly added send-failure rollback, which independently recreates the cross-bot state mismatch.
Useful? React with 👍 / 👎.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes