Repository navigation
[feature] Show "can not be disenchanted" on legendary items - #37
Conversation
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly summarizes the main change: showing that legendary items cannot be disenchanted. |
| Description check | |
The description references issue #4, which matches the pull request objective and changeset. |
| Linked Issues check | |
Issue #4 (direct) asks that "Cannot be disenchanted" appear on legendary weapons for consistency. At the reviewed head, AddDisenchantInfo.lua handles Enum.ItemQuality.Legendary in a dedicated branch. … |
| Out of Scope Changes check | |
The whole-PR diff (merge base 95dd2a1 to head 808d475) changes only AddDisenchantInfo.lua and AddDisenchantInfo.test.lua. The new legendary branch applies to legendary armor and weapons. The issue nam… |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi… |
Full details: Linked Issues check
-
Explanation
Issue
#4(direct) asks that "Cannot be disenchanted" appear on legendary weapons for consistency. At the reviewed head, AddDisenchantInfo.lua handles Enum.ItemQuality.Legendary in a dedicated branch. For armor and weapon items, it calls tooltip:AddLine(ITEM_DISENCHANT_NOT_DISENCHANTABLE), then tooltip:Show(), then returns. The legendary check was removed from the early-return group, so legendary weapons now reach this branch. AddDisenchantInfo.test.lua adds a test that asserts AddLine is called with "Cannot be disenchanted" and Show is called for a legendary weapon. The string is defined as a test global. In the game client, ITEM_DISENCHANT_NOT_DISENCHANTABLE is the expected global string. This repository does not define it outside the test, so the production text depends on the client global. The issue has no non-coding requirements. The coding requirement is met.
Full details: Out of Scope Changes check
-
Explanation
The whole-PR diff (merge base 95dd2a1 to head 808d475) changes only AddDisenchantInfo.lua and AddDisenchantInfo.test.lua. The new legendary branch applies to legendary armor and weapons. The issue names only legendary weapons. Armor uses the same tooltip path and the same class gate, so the armor case is closely related to the requested consistency change and does not add an unrelated feature. Legendary items in other classes, such as containers, still return without a tooltip. A test covers that case and the code path is unchanged for them. The test changes replace the earlier test that expected no tooltip for legendary armor. These test changes support the implemented behavior. No unrelated changes were found.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
Fixes #4