Skip to content

Warlock: migrate strategy controls to Chatless bridge - #52

Merged
Wishmaster117 merged 2 commits into
mainfrom
Warlock-migrate-strategy-controls-to-Chatless-bridge
Aug 8, 2026
Merged

Warlock: migrate strategy controls to Chatless bridge#52
Wishmaster117 merged 2 commits into
mainfrom
Warlock-migrate-strategy-controls-to-Chatless-bridge

Conversation

@Wishmaster117

@Wishmaster117 Wishmaster117 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Migrates Warlock Stones, Pets, Curses and Soulstones from direct chat commands to the Chatless bridge strategy mutation path. Also removes invalid/unused Warlock controls, compacts the UI layout, and preserves manual Playerbots diagnostic commands such as nc ?, co ? and ss ?. Playerbots remains unchanged and read-only.

Summary by CodeRabbit

  • New Features

    • Added /mbdebug enchant [bot] to request weapon-enchant diagnostics.
    • Added structured reporting for weapon-enchant status, errors, and validation results.
    • Warlock stone, soulstone, pet, and curse selections now apply add/remove actions together.
  • Improvements

    • Selections update only after successful actions and clear correctly when toggled off.
    • Repositioned Warlock controls and updated combat-button placement.
    • Retained DPS Assist and DPS AOE controls.
  • Removed

    • Removed unused Warlock buff and generic DPS-related controls.

Migrates Warlock Stones, Pets, Curses and Soulstones from direct chat commands to the Chatless bridge strategy mutation path.
Also removes invalid/unused Warlock controls, compacts the UI layout, and preserves manual Playerbots diagnostic commands such as nc ?, co ? and ss ?.
Playerbots remains unchanged and read-only.
@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR updates Warlock selectors to use combined bridge-aware actions and adds a structured weapon-enchant diagnostic flow through /mbdebug enchant.

Changes

Warlock bridge-backed selectors

Layer / File(s) Summary
Warlock selectors and layout
Strategies/MultiBotWarlock.lua, Core/MultiBotEngine.lua, README.md, docs/ROADMAP.md
Stone, soulstone, pet, and curse selectors use combined actions. Local state updates after successful dispatch. Obsolete controls are removed and remaining controls are repositioned.

Weapon-enchant diagnostics

Layer / File(s) Summary
Weapon-enchant request and response handling
Core/MultiBotComm.lua
The bridge sends WEAPON_ENCHANT requests and validates response fields, status values, tokens, and numeric ranges.
Weapon-enchant debug command
Core/MultiBotHandler.lua, README.md, docs/ROADMAP.md
/mbdebug enchant [bot] resolves a bot, submits the diagnostic request, and displays structured enchantment results or errors.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant DebugCommand
  participant MultiBotComm
  participant Bridge
  participant MultiBotHandler
  DebugCommand->>MultiBotComm: request weapon-enchant debug
  MultiBotComm->>Bridge: send WEAPON_ENCHANT request
  Bridge-->>MultiBotComm: return validated response
  MultiBotComm->>MultiBotHandler: deliver parsed result or error
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the primary change: migrating Warlock strategy controls to the chatless bridge.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch Warlock-migrate-strategy-controls-to-Chatless-bridge

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 68ec4c8a9e

ℹ️ 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".

Comment thread Strategies/MultiBotWarlock.lua Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
Core/MultiBotEngine.lua (1)

531-535: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Clarify the multi-return API contract.

MultiBot.ActionToTargetOrGroup reaches return MultiBot.ActionToTarget(...) when targeting a unit. Call sites still treat it as a simple button action and do not consume the transport value, while the function returns (true, "bridge") only in the targeted bot path and single true values in group paths. Either document and preserve the two-value return contract, or normalize all code paths to the value form callers can handle.

🤖 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/MultiBotEngine.lua` around lines 531 - 535, Normalize the return
contract of MultiBot.ActionToTargetOrGroup so every path returns the single
action-success value expected by its callers. Update the targeted-unit path that
delegates to MultiBot.ActionToTarget and remove or consume its transport value,
while preserving successful group behavior and existing failure results.
🤖 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.

Nitpick comments:
In `@Core/MultiBotEngine.lua`:
- Around line 531-535: Normalize the return contract of
MultiBot.ActionToTargetOrGroup so every path returns the single action-success
value expected by its callers. Update the targeted-unit path that delegates to
MultiBot.ActionToTarget and remove or consume its transport value, while
preserving successful group behavior and existing failure results.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b3aa15a6-3007-4f4a-ae0f-d4bd70c4159b

📥 Commits

Reviewing files that changed from the base of the PR and between 68ec4c8 and cb69b78.

📒 Files selected for processing (6)
  • Core/MultiBotComm.lua
  • Core/MultiBotEngine.lua
  • Core/MultiBotHandler.lua
  • README.md
  • Strategies/MultiBotWarlock.lua
  • docs/ROADMAP.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • Strategies/MultiBotWarlock.lua

@Wishmaster117
Wishmaster117 merged commit 8c08274 into main Aug 8, 2026
5 checks passed
@Wishmaster117
Wishmaster117 deleted the Warlock-migrate-strategy-controls-to-Chatless-bridge branch August 8, 2026 14:59
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.

1 participant