fix: prevent CLM-mutated self pattern from claiming other looters (#186)#187
Conversation
Anchor configured loot/money patterns and parse other-player loot before self loot, so broad CleanerLootMessages-style self formats like "+ %s" cannot match other-player lines like "+ Name : [Item]". Adds regression tests covering CLM-mutated globals plus normal Blizzard self/other loot lines.
📝 WalkthroughWalkthroughThe PR fixes incorrect loot attribution in multi-player scenarios by reordering pattern matching to scan "other looter" patterns before "self" patterns, preventing false self-attribution. A comprehensive test suite validates the fix across both Blizzard and CleanerLootMessages formats. ChangesLoot Attribution Fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
🤖 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 `@DragonToast/Listeners/LootListener_Shared.lua`:
- Line 117: The pattern returned by Utils.BuildPattern(...) is being anchored
(true -> BuildLocalizedPattern adds ^...$) which makes LootListener_Shared's
msg:match(...) fail when chat has extra trailing/leading text; fix by removing
anchoring or allowing suffixes: either call
Utils.BuildPattern(patternConfig.globalString or patternConfig.fallbackString,
false) so BuildLocalizedPattern doesn't wrap ^...$, or alter the matching in
LootListener_Shared to use string.find/partial matching or to trim known
trailing payload before using msg:match; update references to
Utils.BuildPattern, BuildLocalizedPattern, and the msg:match call accordingly.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 321bd6f2-f9b7-4b31-8ac3-6a5f028342d8
📒 Files selected for processing (2)
DragonToast/Listeners/LootListener_Shared.luaspec/LootListener_spec.lua
| end | ||
|
|
||
| return Utils.BuildPattern(patternConfig.globalString or patternConfig.fallbackString) | ||
| return Utils.BuildPattern(patternConfig.globalString or patternConfig.fallbackString, true) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect BuildPattern definition + how it uses the 2nd (anchor) arg
fd -i listenerutils.lua --exec rg -nP -C8 'BuildPattern'Repository: Xerrion/DragonToast
Length of output: 2310
🏁 Script executed:
#!/bin/bash
# Find BuildLocalizedPattern implementation + its anchor handling
fd -i listenerutils.lua --exec rg -nP -C6 'BuildLocalizedPattern' . || true
fd -i listenerutils.lua --exec rg -nP -C6 'anchor' . || trueRepository: Xerrion/DragonToast
Length of output: 50376
🏁 Script executed:
#!/bin/bash
# Inspect LootListener_Shared around the BuildPattern call and find where the pattern is used.
nl -ba DragonToast/Listeners/LootListener_Shared.lua | sed -n '80,160p'
nl -ba DragonToast/Listeners/LootListener_Shared.lua | sed -n '160,260p' || true
rg -n "patternConfig|BuildPattern|fallbackString|globalString" DragonToast/Listeners/LootListener_Shared.lua
rg -n "BuildPattern\(" DragonToast/Listeners -S
rg -n "\:match\(" DragonToast/Listeners/LootListener_Shared.luaRepository: Xerrion/DragonToast
Length of output: 2661
🏁 Script executed:
#!/bin/bash
# Print relevant portion of LootListener_Shared.lua around BuildConfiguredPattern
sed -n '1,220p' DragonToast/Listeners/LootListener_Shared.lua
# Find where the generated patterns are applied (match/Find/parse)
rg -n --fixed-string "BuildConfiguredPattern" DragonToast/Listeners/LootListener_Shared.lua
rg -n --fixed-string "BuildPattern(" DragonToast/Listeners/LootListener_Shared.lua
rg -n "match%(" DragonToast/Listeners/LootListener_Shared.lua
rg -n "string%.match" DragonToast/Listeners/LootListener_Shared.lua
rg -n "pattern.*:match|:match" DragonToast/Listeners/LootListener_Shared.luaRepository: Xerrion/DragonToast
Length of output: 8609
🏁 Script executed:
#!/bin/bash
# Show the functions around where msg:match(patternPair...) and msg:match(moneyPattern.pattern) are used.
sed -n '220,360p' DragonToast/Listeners/LootListener_Shared.lua
# Find where msg is defined/sanitized in LootListener_Shared.lua
rg -n "local msg|msg\s*=" DragonToast/Listeners/LootListener_Shared.lua
rg -n "IsIndexableChatMessage|indexable|sanity guard" DragonToast/Listeners/LootListener_Shared.lua
rg -n "Parse|Handle|OnEvent|LOOT" DragonToast/Listeners/LootListener_Shared.lua | head -n 50Repository: Xerrion/DragonToast
Length of output: 6291
Confirm Utils.BuildPattern(..., true) anchors (and trailing text will break matching)
Utils.BuildPattern forwards true into BuildLocalizedPattern, which wraps the generated pattern with ^...$ (DragonToast/Core/ListenerUtils.lua). LootListener_Shared then applies that anchored pattern directly to the full chat payload via msg:match(...) (no trimming/suffix handling), so any trailing characters/whitespace not covered by the Blizzard global string or fallback string will prevent matches.
🤖 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 `@DragonToast/Listeners/LootListener_Shared.lua` at line 117, The pattern
returned by Utils.BuildPattern(...) is being anchored (true ->
BuildLocalizedPattern adds ^...$) which makes LootListener_Shared's
msg:match(...) fail when chat has extra trailing/leading text; fix by removing
anchoring or allowing suffixes: either call
Utils.BuildPattern(patternConfig.globalString or patternConfig.fallbackString,
false) so BuildLocalizedPattern doesn't wrap ^...$, or alter the matching in
LootListener_Shared to use string.find/partial matching or to trim known
trailing payload before using msg:match; update references to
Utils.BuildPattern, BuildLocalizedPattern, and the msg:match call accordingly.
There was a problem hiding this comment.
@coderabbitai Anchoring is intentional for this fix — issue #186 is specifically caused by the unanchored self pattern (+ %s once CleanerLootMessages mutates LOOT_ITEM_SELF) silently matching a substring of other-player lines like + Name : [Item]. Anchoring forces full-line matches, which is the correct contract here: CHAT_MSG_LOOT payloads are full Blizzard global-string renders (or full CLM-formatted lines), not embedded substrings. The new spec/LootListener_spec.lua cases exercise both Blizzard and CLM payload shapes end-to-end and stay green, so the trailing-text failure mode you flagged is not present in real chat payloads. Keeping the anchor.
There was a problem hiding this comment.
@Xerrion, understood — the contract makes sense. CHAT_MSG_LOOT delivers fully-rendered strings, so anchoring is the correct tool here: it's what prevents the CLM-mutated broad self pattern from swallowing other-player lines via substring matching, and the end-to-end spec coverage confirms the real payload shapes all stay within the anchored boundaries. My concern doesn't apply. Thanks for the detailed context!
(ᵔ ᴥ ᵔ)
✏️ Learnings added
Learnt from: Xerrion
Repo: Xerrion/DragonToast PR: 187
File: DragonToast/Listeners/LootListener_Shared.lua:117-117
Timestamp: 2026-06-01T09:15:55.957Z
Learning: In DragonToast (`DragonToast/Listeners/LootListener_Shared.lua`), `BuildConfiguredPattern` calls `Utils.BuildPattern(..., true)` to produce anchored (`^...$`) patterns. This anchoring is intentional: `CHAT_MSG_LOOT` payloads are always fully-rendered Blizzard global-string lines or full CLM-formatted lines, never embedded substrings. Without anchoring, the CLM-mutated self pattern (e.g., `+ %s`) can match as a substring of other-player lines like `+ Name : [Item]`, causing mis-attribution (issue `#186`). Do not flag the `true` anchor argument as unnecessary or suggest removing it.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
1 participant
Closes #186
Root cause
CleanerLootMessages (and similar addons) mutate Blizzard's loot globals such as
LOOT_ITEM_SELFinto very broad formats like"+ %s". DragonToast's loot parser converted those%sformats into Lua patterns without anchoring them and tried the self-loot patterns before the other-player patterns. As a result, an other-player loot line like"+ Name : [Item]"matched the CLM-mutated self pattern first, and the loot was mis-attributed to "You" instead of the actual looter.Fix
Anchor every configured loot/money pattern derived from the localized globals, and parse other-player loot lines before self loot lines. Anchoring prevents the broad self format from swallowing other-player lines, and the reordered parse order makes the intended match win even on clients where CLM has rewritten the globals. Regression tests cover both the CLM-mutated globals and normal Blizzard self/other loot lines.
Verification
mise run lint- passmise run test- pass (63 successes / 0 failures)Follow-up (non-blocking)
Reviewer suggested adding money-parsing regression tests in a future PR.
Summary by CodeRabbit
Bug Fixes
Tests