Conversation
- Add overlay settings (anchor, offsetX, offsetY, fontSize, abbreviateNumbers) to DB_DEFAULTS profile so preferences survive reloads - Extend /phd config command to read and update each setting live - ApplyOverlayAppearance() applies font (Fonts\ARIALN.TTF + size + OUTLINE) and anchor point from DB whenever an overlay is created or settings change - ActionBar.ApplySettings() re-applies appearance + forces full refresh - FormatNumber local in ActionBar respects abbreviateNumbers flag - Format.FormatNumberFull(n) added for unabbreviated integer display - 10 new unit tests for FormatNumberFull; restore accidentally merged FormatDPS(0)/FormatDPS(1) test cases - Add strupper to .luacheckrc read_globals (WoW global, sibling of strlower) Agent-Logs-Url: https://github.com/Xerrion/PhDamage/sessions/9a5448c4-86c9-462a-837d-1b7098384ed9 Co-authored-by: Xerrion <1850632+Xerrion@users.noreply.github.com>
|
@coderabbitai please review this |
|
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughIntroduces configurable overlay display settings (anchor, offsets, font size, number abbreviation) persisted to addon database. Implements Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 the current code and only fix it if needed.
Inline comments:
In `@Presentation/ActionBar.lua`:
- Around line 110-119: ApplyOverlayAppearance is using cfg.anchor, cfg.offsetX,
cfg.offsetY and cfg.fontSize directly from saved variables, which can be wrong
types and will crash fontString:SetPoint/SetFont; before calling those APIs
validate and normalize: ensure cfg.anchor is in an allowlist of valid anchors
(e.g. "TOP","BOTTOM","LEFT","RIGHT","CENTER", etc.) otherwise fall back to
"BOTTOM"; coerce cfg.offsetX and cfg.offsetY with tonumber() and default to 0
and 2 on failure; coerce cfg.fontSize with tonumber(), clamp it to the safe
range 6–24 and default to 10 if invalid; then call fontString:ClearAllPoints(),
fontString:SetPoint(anchor, button, anchor, offsetX, offsetY) and
fontString:SetFont(...) using the validated values.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b02e3211-abf6-48ea-ba4a-a20bd4f0a38d
📒 Files selected for processing (5)
.luacheckrcCore/Init.luaPresentation/ActionBar.luaPresentation/Format.luatests/test_format.lua
| local function ApplyOverlayAppearance(fontString, button) | ||
| local cfg = ns.Addon and ns.Addon.db and ns.Addon.db.profile and ns.Addon.db.profile.overlay or {} | ||
| local anchor = cfg.anchor or "BOTTOM" | ||
| local offsetX = cfg.offsetX or 0 | ||
| local offsetY = cfg.offsetY or 2 | ||
| local fontSize = cfg.fontSize or 10 | ||
|
|
||
| fontString:ClearAllPoints() | ||
| fontString:SetPoint(anchor, button, anchor, offsetX, offsetY) | ||
| fontString:SetFont("Fonts\\ARIALN.TTF", fontSize, "OUTLINE") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "ActionBar.lua" -type fRepository: Xerrion/PhDamage
Length of output: 89
🏁 Script executed:
# Read the file around the specified lines
if [ -f "Presentation/ActionBar.lua" ]; then
wc -l "Presentation/ActionBar.lua"
fiRepository: Xerrion/PhDamage
Length of output: 91
🏁 Script executed:
# Get context around lines 110-119
if [ -f "Presentation/ActionBar.lua" ]; then
sed -n '100,130p' "Presentation/ActionBar.lua"
fiRepository: Xerrion/PhDamage
Length of output: 1327
🏁 Script executed:
# Search for SavedVariables configuration and db schema
rg "SavedVariables|profile.*overlay" --max-count 20Repository: Xerrion/PhDamage
Length of output: 645
🏁 Script executed:
# Check for any existing validation of overlay config
rg "overlay" -A 5 -B 5 Presentation/ActionBar.lua | head -50Repository: Xerrion/PhDamage
Length of output: 2188
🏁 Script executed:
# Search for db initialization and defaults setup
rg "ns\.Addon\.db|defaults" --max-count 30Repository: Xerrion/PhDamage
Length of output: 410
🏁 Script executed:
# Search for db defaults initialization
rg "db\.defaults|db\.profile|overlay.*=" -B 3 -A 3 | head -100Repository: Xerrion/PhDamage
Length of output: 4084
🏁 Script executed:
# Find Core/Init.lua to see db initialization
if [ -f "Core/Init.lua" ]; then
wc -l "Core/Init.lua"
fiRepository: Xerrion/PhDamage
Length of output: 78
🏁 Script executed:
# Check the Init.lua file for SavedVariables schema
if [ -f "Core/Init.lua" ]; then
cat -n "Core/Init.lua" | head -150
fiRepository: Xerrion/PhDamage
Length of output: 6678
🏁 Script executed:
# Find all calls to ApplyOverlayAppearance
rg "ApplyOverlayAppearance" -B 2 -A 2Repository: Xerrion/PhDamage
Length of output: 1474
🏁 Script executed:
# Check if there's SavedVariables migration logic or similar
rg "OnUpdate|MigrateDB|Cleanup" -A 3Repository: Xerrion/PhDamage
Length of output: 260
🏁 Script executed:
# Look for any other code paths that might set these values outside of the config command
rg "\.anchor\s*=|\.offsetX\s*=|\.offsetY\s*=|\.fontSize\s*=" -B 2 -A 2Repository: Xerrion/PhDamage
Length of output: 1004
Harden persisted overlay values before calling frame APIs.
cfg.anchor, cfg.offsetX, cfg.offsetY, and cfg.fontSize are consumed directly from SavedVariables. The fallback operators (or pattern) don't protect against type mismatches—corrupted data (e.g., cfg.anchor = 123 or cfg.offsetX = "abc") will be truthy and passed to SetPoint/SetFont, causing errors. If a profile has invalid/stale values, overlay updates break. Add runtime normalization: anchor allowlist validation, numeric coercion for offsets, fontSize clamp to 6–24.
Proposed fix
local function ApplyOverlayAppearance(fontString, button)
local cfg = ns.Addon and ns.Addon.db and ns.Addon.db.profile and ns.Addon.db.profile.overlay or {}
- local anchor = cfg.anchor or "BOTTOM"
- local offsetX = cfg.offsetX or 0
- local offsetY = cfg.offsetY or 2
- local fontSize = cfg.fontSize or 10
+ local VALID_ANCHORS = {
+ BOTTOM = true, TOP = true, CENTER = true,
+ LEFT = true, RIGHT = true,
+ BOTTOMLEFT = true, BOTTOMRIGHT = true,
+ TOPLEFT = true, TOPRIGHT = true,
+ }
+
+ local anchor = type(cfg.anchor) == "string" and strupper(cfg.anchor) or "BOTTOM"
+ if not VALID_ANCHORS[anchor] then
+ anchor = "BOTTOM"
+ end
+
+ local offsetX = tonumber(cfg.offsetX) or 0
+ local offsetY = tonumber(cfg.offsetY) or 2
+ local fontSize = tonumber(cfg.fontSize) or 10
+ if fontSize < 6 then fontSize = 6 end
+ if fontSize > 24 then fontSize = 24 end
fontString:ClearAllPoints()
fontString:SetPoint(anchor, button, anchor, offsetX, offsetY)
fontString:SetFont("Fonts\\ARIALN.TTF", fontSize, "OUTLINE")
end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Presentation/ActionBar.lua` around lines 110 - 119, ApplyOverlayAppearance is
using cfg.anchor, cfg.offsetX, cfg.offsetY and cfg.fontSize directly from saved
variables, which can be wrong types and will crash fontString:SetPoint/SetFont;
before calling those APIs validate and normalize: ensure cfg.anchor is in an
allowlist of valid anchors (e.g. "TOP","BOTTOM","LEFT","RIGHT","CENTER", etc.)
otherwise fall back to "BOTTOM"; coerce cfg.offsetX and cfg.offsetY with
tonumber() and default to 0 and 2 on failure; coerce cfg.fontSize with
tonumber(), clamp it to the safe range 6–24 and default to 10 if invalid; then
call fontString:ClearAllPoints(), fontString:SetPoint(anchor, button, anchor,
offsetX, offsetY) and fontString:SetFont(...) using the validated values.
Overlay text was hardcoded to
BOTTOManchor at a fixed font size with no way to reposition or resize it — causing overlap with other UI elements (e.g. Warlock soul shards).Changes
Persistent settings (
db.profile.overlay, survives reloads via AceDB):anchor— WoW anchor point (BOTTOMdefault; all 9 standard points accepted)offsetX/offsetY— pixel nudge from anchor (0/2defaults)fontSize— font size in pt, clamped 6–24 (10default)abbreviateNumbers—true=1.5k,false=1500/phd configslash commands (changes apply immediately, no reload):Presentation/ActionBar.luaApplyOverlayAppearance(fontString, button)reads live DB settings and callsSetFont+SetPoint; replaces the previous hardcodedNumberFontNormalSmalltemplate + fixedBOTTOManchorActionBar.ApplySettings()re-applies appearance to all live overlays and forces a value refresh — called by the config handler on every changePresentation/Format.luaFormat.FormatNumberFull(n)— integer display withoutkabbreviationWarning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
luarocks.org/usr/bin/lua5.1 /usr/bin/lua5.1 /usr/bin/luarocks install busted(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Summary by CodeRabbit
New Features
/phd configslash command to customize overlay appearance, including anchor position, offset, font size (6–24), and number abbreviation toggle.Tests