Add and optimize requested TokensWorth modules
Merge the audited TokensWorth-derived modules and ClassicAPI-focused optimizations into the default branch.
This commit is contained in:
Dusk-92
@@ -670,3 +670,218 @@ Static audit is complete. Before merging to the stable branch, test:
|
||||
Do not merge this audit branch solely on static analysis. Treat the branch as a
|
||||
test candidate until the in-game matrix has passed without Lua errors or UI
|
||||
regressions.
|
||||
|
||||
|
||||
## TokensWorth requested modules — test branch audit
|
||||
|
||||
Branch: `tokensworth-mods-classicapi-test`
|
||||
|
||||
Seven requested modules were added for runtime testing. **Improved Roll Frames was
|
||||
not duplicated** because an optimized version already exists in
|
||||
ShaguTweaks-ClassicAPI.
|
||||
|
||||
### Mouseover Right / Mouseover Right 2
|
||||
|
||||
The original implementation created one independent mouse-catching overlay for
|
||||
every action button plus the bar itself. The test version uses one invisible
|
||||
reveal area per bar and one 100 ms controller that only runs while the bar is
|
||||
visible. When hidden, there is no polling.
|
||||
|
||||
The helper preserves existing bar OnShow/OnHide scripts and tracks the native
|
||||
`SHOW_MULTI_ACTIONBAR_3` / `SHOW_MULTI_ACTIONBAR_4` state through
|
||||
`CVAR_UPDATE`.
|
||||
|
||||
### Hide Macro Text
|
||||
|
||||
One-time UI update only. No ClassicAPI replacement is useful for this FrameXML
|
||||
operation and no periodic work is installed.
|
||||
|
||||
### Unit Frame Abbreviated Names
|
||||
|
||||
The original target-of-target implementation recalculated the name on every
|
||||
rendered frame. The test version uses ClassicAPI event validation and
|
||||
`UNIT_TARGET` when available. Only environments without that event use a
|
||||
250 ms fallback ticker.
|
||||
|
||||
### Movable Unit Frames Extended
|
||||
|
||||
The permanent per-frame Ctrl+Shift check was removed. The module uses
|
||||
`ShaguTweaks.API.IsShiftKeyDown`, `API.IsControlKeyDown` and
|
||||
`MODIFIER_STATE_CHANGED` when ClassicAPI exposes modifier-state events.
|
||||
|
||||
The grid is created lazily on the first unlock. Positions are stored in the
|
||||
existing `ShaguTweaks_config["MoveUnitframesExtended"]` table.
|
||||
|
||||
For the requested Turtle layout, the debuff anchor is **BuffButton32** instead
|
||||
of the upstream BuffButton16.
|
||||
|
||||
### Cursor Tooltip
|
||||
|
||||
The original global `GameTooltip_SetDefaultAnchor` replacement was removed.
|
||||
The test version keeps the native function intact through the ShaguTweaks safe
|
||||
post-hook helper. Cursor tracking runs only while a default-anchored tooltip is
|
||||
shown.
|
||||
|
||||
### Hide Combat Tooltip
|
||||
|
||||
The original combat-long per-frame Shift polling was removed. ClassicAPI's
|
||||
modifier event updates the tooltip only when modifier state actually changes.
|
||||
A 50 ms combat-only fallback exists for environments without that event.
|
||||
|
||||
The GameTooltip Show method is post-hooked so tooltips opened after entering
|
||||
combat immediately inherit the correct hidden/Shift-visible state.
|
||||
|
||||
### Runtime validation required
|
||||
|
||||
Before merging this branch, test:
|
||||
|
||||
- login and `/reload` with the seven new modules disabled
|
||||
- each module individually enabled
|
||||
- both Mouseover Right modules together
|
||||
- toggle the two right actionbars in Interface Options while mouseover modules are enabled
|
||||
- stance/bonus actionbar changes with Hide Macro Text enabled
|
||||
- long NPC target names and target-of-target changes
|
||||
- Ctrl+Shift dragging of party frames, minimap, BuffButton0, BuffButton32 and TempEnchant1
|
||||
- relog/reload persistence of moved positions
|
||||
- Cursor Tooltip alone, Hide Combat Tooltip alone, and both enabled together
|
||||
- combat tooltip Shift reveal/re-hide behavior
|
||||
- interaction with DragonflightUI-Reforged's injected ShaguTweaks Extras list
|
||||
- Lua errors and visible FPS regressions
|
||||
|
||||
Static code review is complete; this branch remains a runtime test candidate.
|
||||
|
||||
|
||||
### Cursor Tooltip — focused audit / optimization
|
||||
|
||||
A focused audit compared the test implementation with the TokensWorth upstream
|
||||
module.
|
||||
|
||||
Findings:
|
||||
|
||||
- the feature genuinely needs frame-by-frame cursor coordinates while the
|
||||
tooltip is visible; there is no ClassicAPI event that can replace pointer
|
||||
tracking
|
||||
- the previous test implementation showed its cursor tracker as soon as
|
||||
`GameTooltip_SetDefaultAnchor` ran, even though default anchoring normally
|
||||
happens before `GameTooltip:Show()`
|
||||
- `ClearAllPoints()` on the cursor tracker every rendered frame was
|
||||
unnecessary
|
||||
- moving the tracker while the mouse coordinates were unchanged caused
|
||||
avoidable layout work
|
||||
- replacing `GameTooltip_SetDefaultAnchor` for every possible tooltip caller
|
||||
could affect non-`GameTooltip` frames unnecessarily
|
||||
|
||||
The optimized test version now:
|
||||
|
||||
- starts its `OnUpdate` work only while the actual `GameTooltip` is shown
|
||||
- performs one immediate cursor-position update before Show to avoid a first
|
||||
frame jump
|
||||
- skips `SetPoint` when cursor coordinates and UI scale have not changed
|
||||
- reuses the same CENTER anchor without per-frame `ClearAllPoints`
|
||||
- keeps the original default-anchor helper for non-`GameTooltip` callers
|
||||
- keeps `SetClampedToScreen(true)` so the cursor tooltip remains inside the
|
||||
visible UI area
|
||||
- retains the intentional default-anchor replacement because a post-hook alone
|
||||
was not reliable on Vanilla/Turtle layout code
|
||||
|
||||
No ClassicAPI-specific replacement is useful here. `GetCursorPosition`,
|
||||
tooltip ownership and frame anchoring are native UI operations. The optimized
|
||||
design therefore keeps the unavoidable cursor-following `OnUpdate`, but only
|
||||
for the time where that work is actually needed.
|
||||
|
||||
|
||||
## Remaining TokensWorth modules — focused audit
|
||||
|
||||
The remaining requested TokensWorth-derived modules were reviewed against their
|
||||
upstream implementations and the current ClassicAPI/ShaguTweaks architecture.
|
||||
|
||||
### Mouseover Right / Mouseover Right 2 — optimized
|
||||
|
||||
Both options continue to use one shared helper instead of duplicating the
|
||||
upstream implementation.
|
||||
|
||||
Upstream creates one mouse-catching overlay for each of the 12 buttons plus an
|
||||
additional bar overlay for each actionbar. The ClassicAPI test version keeps one
|
||||
hotspot and one controller per bar.
|
||||
|
||||
The focused audit additionally:
|
||||
|
||||
- caches the native actionbar visibility flag from `CVAR_UPDATE` /
|
||||
`PLAYER_ENTERING_WORLD` instead of looking it up on every watcher tick
|
||||
- replaces repeated `GetTime()` deadline checks with a simple accumulated
|
||||
two-second idle timer
|
||||
- removes a duplicate watcher restart when the hidden bar is revealed
|
||||
- leaves the watcher disabled while the bar is hidden
|
||||
- preserves any pre-existing bar `OnShow` / `OnHide` scripts
|
||||
|
||||
The remaining 100 ms watcher only exists while the relevant actionbar is
|
||||
actually visible and waiting to auto-hide. No ClassicAPI API can replace the
|
||||
required mouse-over state check.
|
||||
|
||||
### Hide Macro Text — retained as-is
|
||||
|
||||
No hot-path issue was found.
|
||||
|
||||
The module performs a single pass over the native action-button FontStrings and
|
||||
sets their alpha to zero. It installs no event handlers, no hooks and no
|
||||
`OnUpdate`.
|
||||
|
||||
Using ClassicAPI would add abstraction without benefit because this is a
|
||||
one-time FrameXML presentation change. The current implementation is already
|
||||
effectively zero-cost after enable.
|
||||
|
||||
### Unit Frame Abbreviated Names — further optimized
|
||||
|
||||
The upstream module recalculates target-of-target text on every rendered frame.
|
||||
|
||||
The ClassicAPI version already prefers validated `UNIT_TARGET` and
|
||||
`UNIT_NAME_UPDATE` events, with a 250 ms fallback only where `UNIT_TARGET`
|
||||
is unavailable.
|
||||
|
||||
The focused audit additionally:
|
||||
|
||||
- caches the raw unit name and abbreviated result
|
||||
- skips repeated abbreviation/string work while the underlying unit name is
|
||||
unchanged
|
||||
- skips `FontString:SetText` when the displayed text is already correct
|
||||
- limits the legacy fallback to cases where `targettarget` actually exists
|
||||
|
||||
### Movable Unit Frames Extended — hardened
|
||||
|
||||
The upstream module polls Ctrl+Shift every rendered frame. The ClassicAPI
|
||||
version uses `MODIFIER_STATE_CHANGED` plus the shared modifier helpers, with a
|
||||
100 ms fallback only for environments without ClassicAPI modifier events.
|
||||
|
||||
The focused audit found a correctness issue in the first test conversion:
|
||||
merely pressing and releasing Ctrl+Shift saved absolute positions for every
|
||||
managed frame, even if the user had not dragged them. On later logins this
|
||||
could turn untouched Blizzard-managed frames into explicit absolute-position
|
||||
frames.
|
||||
|
||||
The hardened version now:
|
||||
|
||||
- saves a position only after that specific frame was actually dragged
|
||||
- preserves and restores the original drag scripts
|
||||
- preserves and restores the original mouse-enabled state
|
||||
- preserves the original movable state
|
||||
- restores the original user-placed state for untouched frames
|
||||
- only marks a frame user-placed when a real drag starts
|
||||
- keeps the grid lazily created on first unlock
|
||||
- retains the requested Turtle WoW `BuffButton32` debuff anchor
|
||||
|
||||
### Current status
|
||||
|
||||
The focused static audit found no additional change worth making to
|
||||
`actionbar-mouseover-bar-right.lua`,
|
||||
`actionbar-mouseover-bar-right2.lua` or `actionbar-hide-macro.lua` beyond
|
||||
their shared/helper behavior.
|
||||
|
||||
Runtime validation is still required before merge, especially for:
|
||||
|
||||
- enabling/disabling the two native right actionbars while mouseover hiding is
|
||||
active
|
||||
- repeated reveal/hide cycles with action buttons clicked normally
|
||||
- abbreviated target-of-target names during rapid target switching
|
||||
- Ctrl+Shift without dragging anything, followed by relog/reload
|
||||
- dragging each supported frame individually, followed by relog/reload
|
||||
- default party/buff/minimap layout remaining unchanged for frames never moved
|
||||
|
||||
Reference in New Issue
Block a user