Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .luacheckrc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ read_globals = {
-- Lua globals
"table", "string", "math", "pairs", "ipairs", "type", "tostring", "tonumber",
"select", "unpack", "print", "format", "wipe", "sort", "tinsert", "tremove",
"strsplit", "strtrim", "strlower",
"strsplit", "strtrim", "strlower", "strupper",

-- Lua builtins (used or anticipated)
"pcall",
Expand Down
105 changes: 105 additions & 0 deletions Core/Init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ local DB_DEFAULTS = {
profile = {
enabled = true,
verbose = false,
overlay = {
anchor = "BOTTOM",
offsetX = 0,
offsetY = 2,
fontSize = 10,
abbreviateNumbers = true,
},
},
}

Expand Down Expand Up @@ -69,6 +76,96 @@ end
-------------------------------------------------------------------------------
-- Slash Command Router
-------------------------------------------------------------------------------

-- Valid anchor point identifiers accepted by the config command
local VALID_ANCHORS = {
BOTTOM = true, TOP = true, CENTER = true,
LEFT = true, RIGHT = true,
BOTTOMLEFT = true, BOTTOMRIGHT = true,
TOPLEFT = true, TOPRIGHT = true,
}

local function HandleConfigCommand(addon, args)
local cfg = addon.db.profile.overlay
local setting = args[2] and strlower(args[2]) or ""
local value = args[3]

if setting == "" then
addon:Print("Overlay settings:")
addon:Print(string.format(" anchor: %s", cfg.anchor))
addon:Print(string.format(" offsetX: %d", cfg.offsetX))
addon:Print(string.format(" offsetY: %d", cfg.offsetY))
addon:Print(string.format(" fontSize: %d", cfg.fontSize))
addon:Print(string.format(" abbreviate: %s", cfg.abbreviateNumbers and "on" or "off"))
return
end

if setting == "anchor" then
local v = value and strupper(value) or ""
if VALID_ANCHORS[v] then
cfg.anchor = v
addon:Print("Overlay anchor set to: " .. v)
else
addon:Print("Invalid anchor. Valid values: BOTTOM, TOP, CENTER, LEFT, RIGHT, "
.. "BOTTOMLEFT, BOTTOMRIGHT, TOPLEFT, TOPRIGHT")
return
end

elseif setting == "offsetx" then
local n = tonumber(value)
if n then
cfg.offsetX = n
addon:Print("Overlay offsetX set to: " .. n)
else
addon:Print("Invalid value for offsetX. Expected a number.")
return
end

elseif setting == "offsety" then
local n = tonumber(value)
if n then
cfg.offsetY = n
addon:Print("Overlay offsetY set to: " .. n)
else
addon:Print("Invalid value for offsetY. Expected a number.")
return
end

elseif setting == "fontsize" then
local n = tonumber(value)
if n and n >= 6 and n <= 24 then
cfg.fontSize = n
addon:Print("Overlay font size set to: " .. n)
else
addon:Print("Invalid font size. Expected a number between 6 and 24.")
return
end

elseif setting == "abbreviate" then
local v = value and strlower(value) or ""
if v == "on" or v == "true" or v == "1" then
cfg.abbreviateNumbers = true
addon:Print("Number abbreviation enabled.")
elseif v == "off" or v == "false" or v == "0" then
cfg.abbreviateNumbers = false
addon:Print("Number abbreviation disabled.")
else
addon:Print("Invalid value for abbreviate. Use 'on' or 'off'.")
return
end

else
addon:Print("Unknown config setting '" .. setting
.. "'. Valid settings: anchor, offsetX, offsetY, fontSize, abbreviate")
return
end

-- Apply the updated settings to all existing overlays
if ns.ActionBar and ns.ActionBar.ApplySettings then
ns.ActionBar.ApplySettings()
end
end

function PhDamage:OnSlashCommand(input)
if not ns.Diagnostics then
self:Print("Diagnostics module not loaded.")
Expand All @@ -84,11 +181,19 @@ function PhDamage:OnSlashCommand(input)
local spellInput = table.concat(args, " ", 2)
local linkName = spellInput:match("|Hspell:%d+.-|h%[(.-)%]|h")
ns.Diagnostics.PrintSpell(linkName or spellInput)
elseif cmd == "config" then
HandleConfigCommand(self, args)
elseif cmd == "help" then
self:Print("Usage:")
self:Print(" /phd — Show all spell computations")
self:Print(" /phd state — Show current player state snapshot")
self:Print(" /phd spell <name> — Detailed breakdown for one spell")
self:Print(" /phd config — Show overlay display settings")
self:Print(" /phd config anchor <point> — Set text anchor (BOTTOM, TOP, CENTER, ...)")
self:Print(" /phd config offsetX <n> — Set horizontal offset (default 0)")
self:Print(" /phd config offsetY <n> — Set vertical offset (default 2)")
self:Print(" /phd config fontSize <n> — Set font size 6-24 (default 10)")
self:Print(" /phd config abbreviate on|off — Toggle k/M number shortening (default on)")
self:Print(" /phd help — Show this help")
else
ns.Diagnostics.PrintAll()
Expand Down
48 changes: 44 additions & 4 deletions Presentation/ActionBar.lua
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,15 @@ local function BuildSpellIDMap()
end

-------------------------------------------------------------------------------
-- FormatNumber - delegated to shared formatting module
-- FormatNumber - delegated to shared formatting module, honours abbreviation setting
-------------------------------------------------------------------------------
local FormatNumber = function(n) return ns.Format.FormatNumber(n) end
local FormatNumber = function(n)
local cfg = ns.Addon and ns.Addon.db and ns.Addon.db.profile and ns.Addon.db.profile.overlay
if cfg and cfg.abbreviateNumbers == false then
return ns.Format.FormatNumberFull(n)
end
return ns.Format.FormatNumber(n)
end

-------------------------------------------------------------------------------
-- ResolveSpellID(button)
Expand Down Expand Up @@ -96,6 +102,23 @@ local function ResolveSpellID(button)
return spellIDToBase[spellID]
end

-------------------------------------------------------------------------------
-- ApplyOverlayAppearance(fontString, button)
-- Applies font size and anchor point from the current DB profile to an overlay.
-- Falls back to sensible defaults when the DB is not yet available.
-------------------------------------------------------------------------------
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")
Comment on lines +110 to +119
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -name "ActionBar.lua" -type f

Repository: 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"
fi

Repository: 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"
fi

Repository: Xerrion/PhDamage

Length of output: 1327


🏁 Script executed:

# Search for SavedVariables configuration and db schema
rg "SavedVariables|profile.*overlay" --max-count 20

Repository: 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 -50

Repository: Xerrion/PhDamage

Length of output: 2188


🏁 Script executed:

# Search for db initialization and defaults setup
rg "ns\.Addon\.db|defaults" --max-count 30

Repository: Xerrion/PhDamage

Length of output: 410


🏁 Script executed:

# Search for db defaults initialization
rg "db\.defaults|db\.profile|overlay.*=" -B 3 -A 3 | head -100

Repository: 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"
fi

Repository: 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
fi

Repository: Xerrion/PhDamage

Length of output: 6678


🏁 Script executed:

# Find all calls to ApplyOverlayAppearance
rg "ApplyOverlayAppearance" -B 2 -A 2

Repository: Xerrion/PhDamage

Length of output: 1474


🏁 Script executed:

# Check if there's SavedVariables migration logic or similar
rg "OnUpdate|MigrateDB|Cleanup" -A 3

Repository: 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 2

Repository: 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.

end

-------------------------------------------------------------------------------
-- GetOrCreateOverlay(button)
-- Lazily creates and returns the FontString overlay for a button.
Expand All @@ -105,12 +128,29 @@ local function GetOrCreateOverlay(button)
return button.phDamageText
end

local fontString = button:CreateFontString(nil, "OVERLAY", "NumberFontNormalSmall")
fontString:SetPoint("BOTTOM", button, "BOTTOM", 0, 2)
local fontString = button:CreateFontString(nil, "OVERLAY")
ApplyOverlayAppearance(fontString, button)
button.phDamageText = fontString
return fontString
end

-------------------------------------------------------------------------------
-- ActionBar.ApplySettings()
-- Re-applies font/position settings to all existing overlays and forces a
-- full refresh so the displayed values are re-rendered immediately.
-------------------------------------------------------------------------------
function ActionBar.ApplySettings()
for _, button in ipairs(allButtons) do
if button.phDamageText then
ApplyOverlayAppearance(button.phDamageText, button)
end
end
wipe(resultCache)
for _, button in ipairs(allButtons) do
ActionBar.UpdateButton(button)
end
end

-------------------------------------------------------------------------------
-- CalculateForSpell(baseSpellID, playerState)
-- Returns the pipeline result for a spell, using the per-refresh cache.
Expand Down
9 changes: 9 additions & 0 deletions Presentation/Format.lua
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ function Format.FormatNumber(n)
end
end

-------------------------------------------------------------------------------
-- FormatNumberFull(n)
-- Full integer display without "k" abbreviation: 15000 -> "15000"
-------------------------------------------------------------------------------
function Format.FormatNumberFull(n)
if n == nil then return "?" end
return tostring(floor(n + 0.5))
end

-------------------------------------------------------------------------------
-- FormatDPS(n)
-- DPS/HPS with one decimal place, "k" suffix for large values.
Expand Down
38 changes: 38 additions & 0 deletions tests/test_format.lua
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,44 @@ describe("Format Module", function()
end)
end)

describe("FormatNumberFull", function()
it("returns '?' for nil", function()
assert.are.equal("?", Format.FormatNumberFull(nil))
end)

it("returns '0' for 0", function()
assert.are.equal("0", Format.FormatNumberFull(0))
end)

it("returns '1' for 1", function()
assert.are.equal("1", Format.FormatNumberFull(1))
end)

it("returns '500' for 500", function()
assert.are.equal("500", Format.FormatNumberFull(500))
end)

it("returns '1500' for 1500 (no abbreviation)", function()
assert.are.equal("1500", Format.FormatNumberFull(1500))
end)

it("returns '10000' for 10000 (no abbreviation)", function()
assert.are.equal("10000", Format.FormatNumberFull(10000))
end)

it("returns '100000' for 100000 (no abbreviation)", function()
assert.are.equal("100000", Format.FormatNumberFull(100000))
end)

it("returns '0' for 0.4 (floors to 0)", function()
assert.are.equal("0", Format.FormatNumberFull(0.4))
end)

it("returns '1' for 0.6 (rounds up)", function()
assert.are.equal("1", Format.FormatNumberFull(0.6))
end)
end)

describe("FormatDPS", function()
it("returns '?' for nil", function()
assert.are.equal("?", Format.FormatDPS(nil))
Expand Down
Loading