Skip to content
Closed
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
19 changes: 19 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: CI

on:
push:
pull_request:

jobs:
checks:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Setup Lua 5.1
run: |
sudo apt-get update
sudo apt-get install -y lua5.1
- name: Lua syntax checks
run: ./scripts/check_lua_syntax.sh
- name: Locale parity checks
run: ./scripts/check_locale_parity.py
2 changes: 1 addition & 1 deletion AutoJunkDestroyer-Classic.toc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
## Title: AutoJunkDestroyer
## Author: Milestorme
## Notes: Destroy junk items quickly (full localization + updated libs).
## Version: 1.1.10
## Version: 1.2.0
## Category: Bags & Inventory
## IconTexture: Interface/AddOns/AutoJunkDestroyer/AutoJunkDestroyer.tga
## X-Localizations: enUS, deDE, frFR, esES, esMX, itIT, ptBR, ruRU, koKR, zhCN, zhTW
Expand Down
2 changes: 1 addition & 1 deletion AutoJunkDestroyer-TBC.toc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
## Title: AutoJunkDestroyer
## Author: Milestorme
## Notes: Destroy junk items quickly (full localization + updated libs).
## Version: 1.1.10
## Version: 1.2.0
## Category: Bags & Inventory
## IconTexture: Interface/AddOns/AutoJunkDestroyer/AutoJunkDestroyer.tga
## X-Localizations: enUS, deDE, frFR, esES, esMX, itIT, ptBR, ruRU, koKR, zhCN, zhTW
Expand Down
155 changes: 14 additions & 141 deletions AutoJunkDestroyer.lua
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
-- Author: Milestorme
-- Description: Destroy junk items when bags are full easily
-- Safe, BG-aware, works with any number of bags
-- Version: 1.1.10
-- Version: 1.2.0
-------------------------------------------------
-- FUNCTION INDEX
-------------------------------------------------
Expand Down Expand Up @@ -40,18 +40,12 @@
-- LDB.OnClick() -> Minimap click handler
-- LDB.OnTooltipShow() -> Minimap tooltip text

-- Slash Commands
-- /ajd pause -> Toggle pause
-- /ajd toggle -> Toggle popup visibility
-- /ajd button [reset] -> Show/reset popup position
-- /ajd minimap [show|hide|lock|unlock|reset|pos]

-- Event Handling
-- PLAYER_LOGIN -> Initialization
-- PLAYER_ENTERING_WORLD -> BG state detection
-- PLAYER_REGEN_DISABLED -> Combat start
-- PLAYER_REGEN_ENABLED -> Combat end
-- BAG_UPDATE -> Bag change handling
-- BAG_UPDATE_DELAYED -> Bag change handling
-- PLAYER_LOGOUT -> Persist saved data
-------------------------------------------------

Expand Down Expand Up @@ -83,13 +77,20 @@ local BG_EXIT_DELAY = 0.5
-- If we leave a BG while in combat, we defer enabling until combat ends.
local pendingEnableAfterCombat = false
local warnedWaitingForCombat = false
local DEBUG = false

-------------------------------------------------
-- Utility
-------------------------------------------------
local function Print(msg)
-- notes: Unified chat output helper for consistent addon prefix formatting.
DEFAULT_CHAT_FRAME:AddMessage(L["CFF00FF00_DC869E"] .. (L["ADDON_NAME"] or L["ADDON_NAME"]) .. L["R_7FC9D5"] .. msg)
DEFAULT_CHAT_FRAME:AddMessage(L["CFF00FF00_DC869E"] .. L["ADDON_NAME"] .. L["R_7FC9D5"] .. msg)
end

local function DebugPrint(msg)
if DEBUG then
Print("[debug] " .. tostring(msg))
end
end

local function IsInBattleground()
Expand Down Expand Up @@ -615,6 +616,8 @@ local function InitAceDB()
AJD_SanitizeAceDBSV(AutoJunkDestroyerIconDB)

-- Install a pre-logout guard by wrapping AceDB.frame OnEvent so we sanitize BEFORE AceDB cleans up.
-- Compatibility note: this depends on stable AceDB internals (frame/GetScript/SetScript); if those
-- internals change, we skip wrapping and continue safely without crashing.
if not AceDB.__AJD_PreLogoutWrapped and AceDB.frame and AceDB.frame.GetScript and AceDB.frame.SetScript then
local frame = AceDB.frame
local orig = frame:GetScript("OnEvent")
Expand All @@ -631,6 +634,8 @@ local function InitAceDB()
if orig then return orig(self, event, ...) end
end)
AceDB.__AJD_PreLogoutWrapped = true
else
DebugPrint("AceDB pre-logout wrapper skipped (unexpected AceDB frame internals)")
end


Expand Down Expand Up @@ -694,138 +699,6 @@ local function InitAceDB()
Print(L["MSG_MINIMAP_ACEDB_OK"])
end

-------------------------------------------------
-- Slash Commands
-------------------------------------------------
SLASH_AUTOJUNKDESTROYER1 = "/ajd"
SlashCmdList.AUTOJUNKDESTROYER = function(msg)
-- notes: Slash command router for /ajd.
-- notes: Supports: pause, toggle, threshold, button [reset], minimap [hide/show/lock/unlock/reset/pos]
msg = (msg or ""):lower()

if msg == "pause" then
userPaused = not userPaused
paused = userPaused or inBattleground
Print(userPaused and L["MSG_PAUSED"] or L["MSG_RESUMED"])
UpdateButtonVisibility(true)
return
end

if msg:match("^threshold") then
EnsureSV()
local arg = msg:match("^threshold%s*(.*)$") or ""
if arg == "" then
Print(string.format(L["MSG_THRESHOLD_CURRENT"], math.floor(GetBagUsageThreshold() * 100 + 0.5)))
return
end

local v = tonumber(arg)
if not v then
Print(L["MSG_THRESHOLD_USAGE"])
return
end

-- Allow "90" or "0.90"
if v > 1.0 then v = v / 100 end
if v < 0.50 then v = 0.50 end
if v > 0.99 then v = 0.99 end

AutoJunkDestroyerDB.settings.bagUsageThreshold = v
Print(string.format(L["MSG_THRESHOLD_SET"], math.floor(v * 100 + 0.5)))
ScheduleBagRefresh(0)
return
end

if msg:match("^button") then
-- notes: /ajd button -> prints saved popup position; /ajd button reset -> clears saved position.
local arg = msg:match("^button%s*(.*)$") or ""
arg = arg:lower()

if arg == "reset" then
ResetPopupButtonPosition()
Print(L["MSG_POPUP_RESET"])
else
EnsureSV()
local p = AutoJunkDestroyerDB.popupButtonPos
if p then
if p.point then
Print("PopupPos (saved): xOfs=" .. tostring(p.x) .. " yOfs=" .. tostring(p.y) .. " (" .. tostring(p.point) .. ")")
else
Print("PopupPos (saved): x=" .. tostring(p.x) .. " y=" .. tostring(p.y))
end
else
local l, t = button:GetLeft(), button:GetTop()
Print("PopupPos not saved yet. Current left=" .. tostring(l) .. " top=" .. tostring(t))
end
end
return
end

if msg:match("^minimap") then
-- notes: /ajd minimap controls LibDBIcon state (hide/show/lock/unlock/reset/pos).
if not db then
Print(L["MSG_MINIMAP_DB_NOT_READY"])
return
end

local arg = msg:match("^minimap%s*(.*)$") or ""
arg = arg:lower()

if arg == "hide" then
db.profile.minimap.hide = true
icon:Hide("AutoJunkDestroyer")
Print(L["MSG_MINIMAP_ICON_HIDDEN"])
elseif arg == "show" then
db.profile.minimap.hide = false
icon:Show("AutoJunkDestroyer")
Print(L["MSG_MINIMAP_ICON_SHOWN"])
elseif arg == "lock" then
db.profile.minimap.lock = true
icon:Lock("AutoJunkDestroyer")
Print(L["MSG_MINIMAP_LOCKED"])
elseif arg == "unlock" then
db.profile.minimap.lock = false
icon:Unlock("AutoJunkDestroyer")
Print(L["MSG_MINIMAP_UNLOCKED"])
elseif arg == "reset" then
db.profile.minimap.minimapPos = 220
icon:Refresh("AutoJunkDestroyer", db.profile.minimap)
Print(L["MSG_MINIMAP_RESET"])
elseif arg == "pos" or arg == "" then
Print("MinimapPos (saved): " .. tostring(db.profile.minimap.minimapPos) ..
" | hide=" .. tostring(db.profile.minimap.hide) ..
" | lock=" .. tostring(db.profile.minimap.lock))
else
Print("/ajd minimap reset")
end
return
end

if msg == "toggle" then
-- notes: /ajd toggle shows/hides the popup delete button (only when addon is active).
if paused or inBattleground or InCombat() then
Print(L["MSG_DISABLED_RIGHT_NOW"])
button:Hide()
return
end

if button:IsShown() then
button:Hide()
Print(L["MSG_BUTTON_HIDDEN_MINIMAP"])
else
button:Show()
UpdateButtonText()
Print(L["MSG_BUTTON_SHOWN_MINIMAP"])
end
return
end

-- notes: Default help output.
Print("/ajd pause")
Print("/ajd toggle")
Print("/ajd threshold 90")
Print("/ajd minimap reset")
end

-------------------------------------------------
-- Events
Expand Down
19 changes: 19 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,25 @@
# Changelog


## 1.2.0
### Added
- Added explicit `/ajd resume` and `/ajd status` slash commands.
- Added localized slash-command aliases for core and minimap subcommands across non-English locales.
- Added dedicated `Commands.lua` module to separate slash routing from core logic.
- Added CI workflow and scripts for Lua syntax checks and locale key parity.

### Changed
- Removed slash command support (`/ajd ...`) and standardized control via minimap left/right-click interactions.
- Localized remaining hardcoded slash-command output strings (popup position, minimap position, and help text).
- Translated newly added slash/minimap/popup status keys into each locale language (instead of English placeholders).
- Improved AceDB pre-logout wrapping notes/guards for forward compatibility diagnostics.
- Updated README slash command docs for pause/resume/status behavior.

### Fixed
- Cleaned minor code/comment polish issues (`Print` addon-name redundancy and event comment naming).
- Fixed slash alias normalization to avoid UTF-8 breakage by lowercasing ASCII only in command parsing.


## 1.1.10
### Fixed
- Consolidated SavedVariables initialization so shard button defaults are always set alongside the bag threshold.
Expand Down
111 changes: 111 additions & 0 deletions CODE_REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
# AutoJunkDestroyer Code Review

Date: 2026-02-14

## Scope
- `AutoJunkDestroyer.lua`
- `Locales/enUS.lua` plus locale parity spot checks
- `README.md`
- `AutoJunkDestroyer-Classic.toc`, `AutoJunkDestroyer-TBC.toc`

---

## Executive summary
The addon is generally solid: defensive state handling around battleground/combat transitions is clear, bag-refresh debouncing is sensible, and the AceDB data separation plus migration hardening is a strong stability improvement. The highest-value follow-up items are mostly polish and consistency fixes (command/docs drift, localization consistency, and reducing long-file complexity).

---

## Findings and recommendations

## 1) Command/documentation drift: `/ajd resume` is documented but not implemented
**Severity:** Medium
**Impact:** User confusion and support churn.

### Evidence
- README documents `/ajd pause` and `/ajd resume`.
- Slash command parser only handles `pause` (toggle), not `resume`.

### Recommendation
- Add explicit command handling for `resume` (and optionally `status`) so docs and behavior match.
- Alternatively, update README to reflect that `/ajd pause` toggles both states.

---

## 2) Some user-facing strings bypass localization
**Severity:** Low-Medium
**Impact:** Incomplete localization experience for non-English users.

### Evidence
- Several chat outputs are hardcoded English debug/status strings (e.g., popup position and minimap status printouts) rather than locale keys.

### Recommendation
- Move remaining hardcoded `Print("...")` strings into locale keys and reuse `L[...]` for all user-visible text.

---

## 3) Single-file complexity is high; split into modules
**Severity:** Medium (maintainability)
**Impact:** Harder future changes, higher regression risk.

### Evidence
- `AutoJunkDestroyer.lua` currently combines event routing, UI, deletion logic, AceDB migration/hardening, slash commands, and shard utilities.

### Recommendation
- Split into focused files (e.g., `Core.lua`, `UI.lua`, `Commands.lua`, `Shard.lua`, `Minimap.lua`, `Migration.lua`).
- Keep startup wiring in one entrypoint file.

---

## 4) AceDB wrapper approach is effective but high-risk if upstream internals change
**Severity:** Medium
**Impact:** Potential compatibility risk with future AceDB internals.

### Evidence
- The code wraps `AceDB.frame`'s `OnEvent` to sanitize DB state before logout.

### Recommendation
- Keep this guard, but add an in-code note about AceDB version expectations and a safety no-op path if frame/script shapes change.
- Add lightweight self-check logging only in debug mode.

---

## 5) Add automated static checks in CI (syntax + locale key parity)
**Severity:** Medium (quality process)
**Impact:** Prevents accidental breakage before release.

### Evidence
- Project currently relies on manual validation; locale correctness is important and easy to regress.

### Recommendation
- Add a simple CI job to run:
- Lua syntax check for addon source files.
- Locale key parity verification against `Locales/enUS.lua`.
- Keep checks lightweight to match addon repo simplicity.

---

## 6) Small correctness/polish opportunities
**Severity:** Low

### Observations
- `Print()` uses `(L["ADDON_NAME"] or L["ADDON_NAME"])`, which is redundant.
- Inline comments mention some older event names while code registers `BAG_UPDATE_DELAYED`.

### Recommendation
- Clean tiny redundancies/comments during next maintenance pass.

---

## What is already strong
- Good combat/BG gating and deferred re-enable behavior.
- Debounced bag refresh strategy.
- Sensible SavedVariables clamping/defaults.
- Dedicated `AutoJunkDestroyerIconDB` and migration cleanup around AceDB shape corruption.

---

## Suggested roadmap (minimal risk)
1. Align slash commands and README (`resume` + optional `status`).
2. Finish localization sweep for remaining hardcoded chat strings.
3. Add lightweight CI checks for syntax and locale parity.
4. Incremental modular split of `AutoJunkDestroyer.lua` with no feature changes.
Loading