fix(cdm): FocusKick cast sound goes silent and never re-arms - #1178
Merged
EllesmereGaming merged 7 commits intoAug 4, 2026
Conversation
The focus cast sound stayed dead with the bar fully populated. The probe caught both facts at the same instant: spells=1(1 pos) and sound=NOT CREATED, which cannot both be true if the arming had run. RefreshFocusKickProxies only builds the proxy on its hasContent branch, and it has exactly two callers: setup, and the tail of BuildAllCDMBars. Adding a spell in the options writes assignedSpells without re-arming anything, so a bar that was empty when the last arming ran stays soundless no matter what is added to it afterwards. RefreshFocusCastProxyUnit is what the options page calls, and it opened with 'if not _focusCastProxy then return end' -- so the one path the user can reach by hand was also the one path that could not recover the state. It now runs the full refresh instead of returning, which makes the reporter's own workaround (touch the options) reliable rather than incidental. Known gap, deliberately not guessed at: the spell picker writes assignedSpells in several places in the options file and none of them re-arm. Locating that commit path needs one more test round; this change does not depend on it.
…s unresolved
Second half of the focus cast sound bug, and the one that matches the
original report: sound works, dies after a port or a zone change, and only
returns when something rebuilds the bars.
hasContent read the spell list as:
local sd = ns.GetBarSpellData(FOCUSKICK_BAR_KEY)
local spells = sd and sd.assignedSpells
GetBarSpellData returns nil while the active spec key is unresolved (not
specKey or specKey == "0"), which is precisely the state during a loading
screen. That nil was indistinguishable from an empty bar, so the else
branch ran and unregistered a working proxy. Nothing re-armed afterwards,
because the only two arming callers are setup and the BuildAllCDMBars tail.
Now a nil store is treated as 'unknown', not 'empty': the refresh returns
without touching the proxy and the next rebuild arms it properly.
Confirmed against the probe: a spec change fired BuildAllCDMBars with
spells=1(1 pos) and the proxy went NOT CREATED -> LISTENING, proving the
arming path itself is correct and only its reachability was at fault.
…rrupt Removing the kick from the FocusKick bar empties assignedSpells while focusKickInterruptSpellID keeps pointing at it. The dropdown builds its label map only from spells currently on the bar, so the stored key had no label and the widget fell back to rendering the key itself: the row showed a bare '47528'. The selection now gets a resolved spell name, but deliberately is NOT added to spellOrder, so it renders correctly without appearing as a selectable option on a bar that no longer holds it. The list still reads '(no spells on bar)', which is accurate. Cosmetic only; no change to which spell the kick logic uses.
…ate spell data The previous two commits fixed real defects but not the one the reporter keeps hitting. His probe output after that build was unchanged: spells=1(1 pos) alongside sound=NOT CREATED -- a fully populated bar with no proxy. Arming is demand-gated and effectively one-shot: RefreshFocusKickProxies builds the proxy only on its hasContent branch, and its only callers are setup and the tail of BuildAllCDMBars. GetBarSpellData returns nil until the active spec key resolves, so any pass that runs during that window sees an apparently empty bar, declines to arm, and is never retried. The sound then stays dead for the whole session while the bar looks fine, which is exactly why a spec change 'fixes' it -- that reruns BuildAllCDMBars. Two re-arms, both cheap and idempotent: - when the refresh finds the spell store unresolved it schedules one bounded retry rather than simply returning, - and PLAYER_ENTERING_WORLD schedules a deferred refresh, so a login or a zone change cannot leave the proxy permanently unbuilt. The refresh early-returns on a genuinely empty bar, so neither path installs anything for users who do not use the feature.
…cus bar Requested: play the focus-cast sound without having to put the interrupt icon on the FocusKick bar. The sound handler never needed it. It requires exactly two things, a configured sound and an interrupt spell id to run its 'is my kick ready' cooldown check against, and the bar's assigned spells are only the FALLBACK source for that id -- an explicit focusKickInterruptSpellID satisfies it alone. But arming was gated on hasContent, which demands a positive spell on the bar, so the runtime handler and the gate that installs it disagreed about what the feature needs. Taking the kick off the bar killed a sound that had everything required to keep working. The cast sound now arms on its own condition: a configured sound plus a resolvable spell id, explicit or from the bar. The icon-bearing parts of the family, the anchor proxy and the reminders, keep the original bar-content gate and are unchanged. It also arms during the store-unresolved window, since an explicit spell id lives on the bar data rather than in the per-spec spell store and does not have to wait for the retry.
Last gap in the focus cast sound. The tester's sequence: log in with an empty kick bar (NOT CREATED, correct), add the interrupt spell, sound still dead, take a portal, sound starts working. AddSpellToBar and RemoveSpellFromBar are the chokepoints for assignedSpells and neither re-evaluated the FocusKick proxies, whose arming is gated on exactly that content. Nothing else re-runs it either, so the state stayed wrong until an unrelated rebuild happened to fire -- a spec change, or the PLAYER_ENTERING_WORLD re-arm added earlier, which is precisely why a portal 'fixed' it. Both now call RefreshFocusKickProxies, tagged so the probe shows which edge ran. The remove side is a re-evaluation rather than a teardown: with an explicit interrupt spell the cast sound is meant to survive an empty bar. Guarded on the focuskick key, so every other bar's add/remove is unchanged -- including the ghost-bar removal AddSpellToBar performs internally.
…r change Self-review of the five focus-kick commits before opening the PR. Control flow is byte-for-byte equivalent; only structure and comments move. - The two identical 'if bd and bd.enabled ~= false' guards are merged, so hasContent and soundWanted are derived in one pass over the same data. - The rationale for the sound's weaker requirement now sits where soundWanted is computed, instead of above the hasContent branch it does not describe. - Two comments that had run together in the retry block are separated, so the note about arming during the unresolved window reads against the line it explains. - The inner loop variable and the interrupt-spell pick no longer share the name 'sid'. Two things verified while reviewing, both clean: RefreshFocusCastProxyUnit is only ever called from the options file, so the new call into RefreshFocusKickProxies cannot recurse; and CDMFinishSetup is the documented one-time construction hub guarded by _cdmSetupStarted, so the PLAYER_ENTERING_WORLD frame is created once per session rather than accumulating.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptom
The FocusKick cast sound went silent at random. It came back if the options panel was touched, and reset again after porting or a loading screen. Reported on 8.7.4.
Cause
Arming is demand-gated:
RefreshFocusKickProxiesbuilds the cast-sound proxy only on itshasContentbranch, and it had exactly two callers,CDMFinishSetupand the tail ofBuildAllCDMBars. That makes arming effectively a one-shot, with nothing to re-check when the inputs change afterwards.Three separate routes into the bad state came out of that, each confirmed with a diagnostic build on the reporter's client:
GetBarSpellDatareturns nil while the active spec key is unresolved, which is exactly the state during a loading screen. That nil was indistinguishable from an empty bar, so the else branch ran and unregistered a working proxy. Nothing re-armed afterwards. This is the "sound stops after I port" report.spells=1(1 pos)andsound=NOT CREATEDat the same instant: a fully populated bar with no proxy.AddSpellToBarandRemoveSpellFromBarare the chokepoints forassignedSpellsand neither re-evaluated the proxies. The reporter's own words: "added the spell, still no sound; took a portal and it started working".RefreshFocusCastProxyUnitis what the options page calls, and it opened withif not _focusCastProxy then return end— so the only route a user could reach by hand was also the only one incapable of creating the proxy.Changes
PLAYER_ENTERING_WORLDschedules a deferred refresh, so a login or zone cannot leave the proxy permanently unbuilt.AddSpellToBar/RemoveSpellFromBarre-evaluate the proxies when the focuskick bar's content changes. Guarded on the focuskick key, so every other bar is unaffected, including the ghost-bar removalAddSpellToBarperforms internally.RefreshFocusCastProxyUnitruns the full refresh instead of returning when the proxy is absent.Behaviour change, requested
The cast sound no longer requires the interrupt to be sitting on the FocusKick bar.
The handler never needed it: it requires a configured sound and an interrupt spell id to run its "is my kick ready" cooldown check against, and the bar's assigned spells are only the fallback source for that id. An explicit
focusKickInterruptSpellIDsatisfies it alone. Arming was gated onhasContentregardless, so the runtime handler and the gate that installs it disagreed about what the feature needs, and taking the kick off the bar killed a sound that had everything required to keep working.The cast sound now arms on its own condition. The icon-bearing parts of the family, the anchor proxy and the reminders, keep the original bar-content gate and are unchanged.
Who this affects, given
focusCastSoundKeydefaults to"none"andfocusKickInterruptSpellIDdefaults to nil and is only ever written by the options dropdown:hasContentThe last group had to deliberately configure both, so the sound is almost certainly what they wanted. The one edge worth naming: anyone who silenced the sound by emptying the bar rather than setting the sound to None will hear it again. Setting Focus Cast Sound to None is the intended control.
Note that this makes
focusKickInterruptSpellIDload-bearing after the spell leaves the bar. It is deliberately not cleared on removal.Also
The Interrupt Spell dropdown rendered a bare spell id (
47528) once its spell left the bar, because the label map is built only from spells currently on it. The stored selection now gets a resolved name, but is deliberately not added to the menu order, so it displays correctly without appearing as a selectable option on a bar that no longer holds it.Testing
Verified in game by the reporter across the full sequence: log in with the bar empty, add the spell and the sound works immediately with no portal or spec change, remove the spell and it keeps working, zone and it survives, log back in with the bar still empty and it still works.
Diagnostics used to find this were kept on a separate branch and are not in this diff.