From 42501372c5ee69029fd839917b1b16e941accf6f Mon Sep 17 00:00:00 2001 From: Christopher Hermann Date: Thu, 30 Apr 2026 19:18:55 +0900 Subject: [PATCH] =?UTF-8?q?release:=20v1.3.15-alpha=20=E2=80=94=20Cast=20A?= =?UTF-8?q?ll=20gate=20+=20stop-after-reload=20crash=20fix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cast All gate - Cast All was greyed out whenever the currently-selected character had no spells in the active preset, even when other party members had spells in the same preset. Cast Character was correctly char-scoped. - New BfBot.UI._CanCastAll() walks every portrait slot and matches the cross-party scope of BuildQueueFromPreset; the bbCast button now uses _CanCastAll while bbCastChar keeps the char-scoped _CanCast. Stop-after-reload crash (#38, reported by sov_) - After reloading a save mid-cast, BfBot.Exec._casters[].sprite still held freed CGameSprite pointers from the pre-reload party; pressing Stop dereferenced them via EEex_Action_QueueResponseStringOnAIBase, which segfaults at the engine level. pcall does NOT catch C++ access violations. - Stop() and _Complete() now re-resolve the caster sprite from the current portrait slot in their cleanup loops, so they never deref the freed pointer. BFBTCR is a no-op on targets without an active BFBTCH effect, so the cleanup is safe even when the slot now holds a different character. - BfBot.Exec._HardReset clears state without touching cached sprites. - BfBot.Exec._IsStateStale compares cached caster names against the live portrait sprites and proactively hard-resets execution state from _SafetyTick when party composition changed across the reload, so Cast / Cast Character re-enable themselves on the next safety tick instead of leaving the user stuck pressing Stop. - Userdata equality on EEex_Sprite_GetInPortrait is unreliable: two consecutive calls return distinct userdata wrappers and the __eq metamethod evaluates them as not equal even when they point at the same CGameSprite. The implementation deliberately avoids sprite identity in favor of name comparison + portrait re-resolution. Tests - New BfBot.Test.StaleState phase covers _HardReset, _IsStateStale, Stop / _Complete / _SafetyTick stale recovery, and Stop's re-resolve safety on a same-save reload (8 assertions, all pass). Versions - BfBot.VERSION 1.3.14-alpha → 1.3.15-alpha - setup-buffbot.tp2 VERSION ~v1.3.14-alpha~ → ~v1.3.15-alpha~ Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 6 ++ buffbot/BfBotCor.lua | 2 +- buffbot/BfBotExe.lua | 116 +++++++++++++++++--- buffbot/BfBotTst.lua | 218 +++++++++++++++++++++++++++++++++++++- buffbot/BfBotUI.lua | 26 ++++- buffbot/BuffBot.menu | 2 +- buffbot/setup-buffbot.tp2 | 2 +- 7 files changed, 355 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f07fe4..6db1dda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## v1.3.15-alpha (2026-04-30) + +### Fixed +- **"Cast All" greyed out when the selected character has no preset spells** — the gate fed both action buttons via `BfBot.UI._CanCast()`, which only checked the current character's spell table. On characters with nothing configured for the active preset (e.g. Safana on a buff preset), Cast All was disabled even though other party members had spells in the same preset. Cast All now uses a new `BfBot.UI._CanCastAll()` that mirrors `BuildQueueFromPreset`'s cross-party scope: it falls through to the other portrait slots when the current character is empty. Cast Character keeps the original char-scoped gate. +- **Crash when pressing Stop after reloading a save mid-cast** (#38) — reported by sov_ on Discord. After loading a save while a buff queue was running, only the Stop button was enabled; clicking it triggered an access violation. `BfBot.Exec._casters[].sprite` cached `CGameSprite` userdata from the pre-reload party, and the post-reload save freed those C++ objects — calling `EEex_Action_QueueResponseStringOnAIBase` on the stale userdata segfaulted at the engine level (and `pcall` does not catch C++ access violations). Stop and `_Complete` now re-resolve the caster sprite from the current portrait slot in their cleanup loops, so they never dereference the freed pointer; `BFBTCR` is a no-op on targets without an active `BFBTCH`, so the cleanup is safe even when the slot now holds a different character. A new `_IsStateStale` heuristic compares cached caster names against the live portrait names and proactively hard-resets execution state from `_SafetyTick` when party composition changed across the reload, so the Cast / Cast Character buttons re-enable themselves on the next safety tick instead of leaving the user stuck pressing Stop. Covered by `BfBot.Test.StaleState` (8 assertions). + ## v1.3.14-alpha (2026-04-28) ### Fixed diff --git a/buffbot/BfBotCor.lua b/buffbot/BfBotCor.lua index b2c8e48..9bc9fb5 100644 --- a/buffbot/BfBotCor.lua +++ b/buffbot/BfBotCor.lua @@ -6,7 +6,7 @@ -- Root namespace BfBot = BfBot or {} -BfBot.VERSION = "1.3.14-alpha" +BfBot.VERSION = "1.3.15-alpha" BfBot.MAX_PRESETS = 8 -- ============================================================ diff --git a/buffbot/BfBotExe.lua b/buffbot/BfBotExe.lua index ebd76aa..7582b6c 100644 --- a/buffbot/BfBotExe.lua +++ b/buffbot/BfBotExe.lua @@ -460,15 +460,77 @@ function BfBot.Exec._Advance(slot) BfBot.Exec._ProcessCasterEntry(slot, caster.index + 1) end +--- Reset all execution state without dereferencing cached sprites. +--- Used to recover from save-reload mid-cast (issue #38), where _casters +--- holds freed CGameSprite pointers from the pre-reload party. Calling +--- into the engine on those pointers would segfault — clear the table +--- first, never touch caster.sprite. Caller is responsible for closing +--- the exec log (see Stop / _Complete recovery branches); this keeps the +--- function side-effect free so the in-game test suite can capture its +--- own output to the log around each subtest. +function BfBot.Exec._HardReset() + BfBot.Exec._state = "idle" + BfBot.Exec._casters = {} + BfBot.Exec._activeCasters = 0 + BfBot.Exec._castCount = 0 + BfBot.Exec._skipCount = 0 + BfBot.Exec._totalEntries = 0 + BfBot.Exec._qcMode = 0 +end + +--- Detect stale execution state from a save reload mid-cast. +--- After loading a save while casting, _casters[].sprite still holds +--- freed CGameSprite pointers from the pre-reload party. We can't safely +--- compare sprite identity directly — EEex returns a fresh userdata +--- wrapper per call to EEex_Sprite_GetInPortrait, and `==` falls through +--- to a __eq metamethod that does NOT pointer-compare the wrapped +--- CGameSprite (verified empirically with two consecutive calls returning +--- different wrappers and `==` evaluating to false). +--- +--- Instead, compare the cached character name (a plain string captured +--- at Start time) against the freshly-fetched portrait sprite's name. +--- The fresh sprite is safe to dereference; the cached string never +--- references engine memory. This catches the "different-party-composition +--- reload" case (e.g. user reloads to before recruiting an NPC). It does +--- NOT catch the "same-save reload" case where party composition is +--- unchanged — Stop's cleanup loop must independently re-resolve sprites +--- from the portrait so it doesn't dereference cached caster.sprite. +--- @return boolean: true if state is "running" but at least one caster's +--- cached name no longer matches the current portrait at that slot. +function BfBot.Exec._IsStateStale() + if BfBot.Exec._state ~= "running" then return false end + + for slot, caster in pairs(BfBot.Exec._casters) do + if caster.name then + local fresh = EEex_Sprite_GetInPortrait(slot) + local freshName = fresh and BfBot._GetName(fresh) or nil + if freshName ~= caster.name then return true end + end + end + + return false +end + --- Log execution summary and transition to "done" state. function BfBot.Exec._Complete() - -- Clean up lingering cheat buffs + -- Fast-path recovery from save-reload (issue #38) — see Stop(). + if BfBot.Exec._IsStateStale() then + BfBot.Exec._HardReset() + BfBot._CloseLog() + return + end + + -- Clean up lingering cheat buffs — re-resolve sprite from portrait, + -- never dereference cached caster.sprite (see Stop() rationale). for slot, caster in pairs(BfBot.Exec._casters) do - if caster.cheatApplied and caster.sprite then - pcall(function() - EEex_Action_QueueResponseStringOnAIBase( - 'ReallyForceSpellRES("BFBTCR",Myself)', caster.sprite) - end) + if caster.cheatApplied then + local sprite = EEex_Sprite_GetInPortrait(slot) + if sprite then + pcall(function() + EEex_Action_QueueResponseStringOnAIBase( + 'ReallyForceSpellRES("BFBTCR",Myself)', sprite) + end) + end caster.cheatApplied = false end end @@ -572,19 +634,39 @@ end --- Stop execution mid-queue. function BfBot.Exec.Stop() + -- Fast-path recovery from save-reload mid-cast with party composition + -- change (issue #38): hard-reset to idle without entering the cleanup + -- loop — there's nothing to clean up because the buffs were applied to + -- the previous save's party. + if BfBot.Exec._IsStateStale() then + BfBot._Print("[BuffBot] Stale execution state from save reload — resetting.") + BfBot.Exec._HardReset() + BfBot._CloseLog() + return + end + if BfBot.Exec._state ~= "running" then BfBot._Print("[BuffBot] Not running.") return end BfBot.Exec._state = "stopped" - -- Clean up lingering cheat buffs + -- Clean up lingering cheat buffs. Re-resolve the sprite from the + -- current portrait slot rather than using cached caster.sprite — that + -- userdata wraps a freed CGameSprite pointer if the user reloaded a + -- save mid-cast (issue #38), and pcall does NOT catch the access + -- violation that engine calls would trigger on the freed pointer. + -- This re-resolution is safe even when _IsStateStale missed a same- + -- party reload: BFBTCR is a no-op on targets without an active BFBTCH. for slot, caster in pairs(BfBot.Exec._casters) do - if caster.cheatApplied and caster.sprite then - pcall(function() - EEex_Action_QueueResponseStringOnAIBase( - 'ReallyForceSpellRES("BFBTCR",Myself)', caster.sprite) - end) + if caster.cheatApplied then + local sprite = EEex_Sprite_GetInPortrait(slot) + if sprite then + pcall(function() + EEex_Action_QueueResponseStringOnAIBase( + 'ReallyForceSpellRES("BFBTCR",Myself)', sprite) + end) + end caster.cheatApplied = false end end @@ -614,6 +696,16 @@ function BfBot.Exec._SafetyTick() pcall(BfBot.Innate.RefreshAll) end + -- Proactively recover from save-reload mid-cast (issue #38). The + -- EEex_LuaAction chain that drives _Advance does NOT resume after a + -- save load, so _state stays "running" forever and the UI gates + -- Cast/CastChar off. Detect via portrait-set mismatch and reset so the + -- user sees a clean idle state on next menu open — and so the running + -- branch below falls through to the BFBTCH cleanup loop. + if BfBot.Exec._IsStateStale() then + BfBot.Exec._HardReset() + end + -- If exec engine is actively running, it owns cheat management — don't interfere if BfBot.Exec._state == "running" then return end diff --git a/buffbot/BfBotTst.lua b/buffbot/BfBotTst.lua index 3544b4a..c52b390 100644 --- a/buffbot/BfBotTst.lua +++ b/buffbot/BfBotTst.lua @@ -2090,6 +2090,217 @@ function BfBot.Test.Theming() return fail == 0 end +-- ============================================================ +-- BfBot.Test.StaleState — Save-reload mid-cast recovery (issue #38) +-- ============================================================ +function BfBot.Test.StaleState() + _reset() + P("") + P("========================================") + P(" Stale State Recovery Tests (issue #38)") + P("========================================") + P("") + + -- Precondition: don't run if exec is currently active + if BfBot.Exec.GetState() == "running" then + _warning("Skipping: exec currently running") + return _summary("Stale State") + end + + -- Save state to restore on exit + local savedState = BfBot.Exec._state + local savedCasters = BfBot.Exec._casters + local savedActive = BfBot.Exec._activeCasters + local savedLastTick = BfBot.Exec._lastSafetyTick + + local function _restore() + BfBot.Exec._state = savedState + BfBot.Exec._casters = savedCasters + BfBot.Exec._activeCasters = savedActive + BfBot.Exec._lastSafetyTick = savedLastTick + end + + -- A non-userdata sentinel for the "freed pointer" simulation. The + -- "stale-test" name is what _IsStateStale compares against the live + -- portrait sprite's name — guaranteed not to match a real character. + local STALE = "STALE_TEST_SENTINEL" + + local function _poison(cheatApplied) + BfBot.Exec._state = "running" + BfBot.Exec._casters = { + [0] = { + sprite = STALE, + cheatApplied = cheatApplied and true or false, + queue = {}, + index = 0, + done = false, + name = "stale-test", + cheatBoundary = 0, + }, + } + BfBot.Exec._activeCasters = 1 + end + + -- Re-open the log between subtests because Stop()/_Complete()'s stale + -- branch closes the log handle (correct production behavior — caller + -- of Stop owned the log via Start). The test runner needs the log open + -- to capture _summary() output after those subtests run. + local function _reopenLog() + if BfBot._OpenLogAppend then BfBot._OpenLogAppend(BfBot._logFile) end + end + + -- ---- Test 1: _HardReset exists and clears state ---- + P(" [1] _HardReset clears state without dereferencing cached sprites") + if type(BfBot.Exec._HardReset) ~= "function" then + _nok("_HardReset is not a function") + _restore() + return _summary("Stale State") + end + + _poison(true) + local hrOk, hrErr = pcall(BfBot.Exec._HardReset) + if hrOk + and BfBot.Exec._state == "idle" + and next(BfBot.Exec._casters) == nil + and BfBot.Exec._activeCasters == 0 + then + _ok("_HardReset cleared state to idle and emptied _casters") + else + _nok(string.format("_HardReset incomplete: ok=%s err=%s state=%s casters_empty=%s active=%s", + tostring(hrOk), tostring(hrErr), tostring(BfBot.Exec._state), + tostring(next(BfBot.Exec._casters) == nil), tostring(BfBot.Exec._activeCasters))) + end + + -- ---- Test 2: _IsStateStale ---- + P("") + P(" [2] _IsStateStale detection") + if type(BfBot.Exec._IsStateStale) ~= "function" then + _nok("_IsStateStale is not a function") + _restore() + return _summary("Stale State") + end + + -- 2a: idle state is never stale (regardless of _casters) + BfBot.Exec._state = "idle" + BfBot.Exec._casters = {} + _check(BfBot.Exec._IsStateStale() == false, "idle state is not stale") + + -- 2b: running with cached name matching current portrait name → not stale + local sprite = EEex_Sprite_GetInPortrait(0) + if sprite then + local realName = BfBot._GetName(sprite) + BfBot.Exec._state = "running" + BfBot.Exec._casters = { [0] = { sprite = sprite, name = realName, cheatApplied = false } } + BfBot.Exec._activeCasters = 1 + _check(BfBot.Exec._IsStateStale() == false, + "running with cached name matching portrait[0] is not stale") + else + _warning("No sprite in portrait 0; skipping fresh-name match case") + end + + -- 2c: running with cached name that does not match portrait → stale + _poison(false) + _check(BfBot.Exec._IsStateStale() == true, + "running with cached name 'stale-test' does not match portrait → stale") + + -- ---- Test 3: Stop() with stale state hard-resets without engine call ---- + P("") + P(" [3] Stop() recovers from stale state") + _poison(true) -- cheatApplied=true would trigger BFBTCR cleanup loop in buggy code + local stopOk, stopErr = pcall(BfBot.Exec.Stop) + _reopenLog() -- Stop's stale branch closed the log + if stopOk + and BfBot.Exec._state == "idle" + and next(BfBot.Exec._casters) == nil + then + _ok("Stop() with stale state hard-reset to idle (no engine call attempted)") + else + _nok(string.format("Stop() with stale state: ok=%s err=%s state=%s casters_empty=%s", + tostring(stopOk), tostring(stopErr), tostring(BfBot.Exec._state), + tostring(next(BfBot.Exec._casters) == nil))) + end + + -- ---- Test 4: _Complete() with stale state hard-resets ---- + P("") + P(" [4] _Complete() recovers from stale state") + _poison(true) + BfBot.Exec._activeCasters = 0 -- _Complete is only called when all casters finished + local cmpOk, cmpErr = pcall(BfBot.Exec._Complete) + _reopenLog() -- _Complete's stale branch closed the log + if cmpOk + and BfBot.Exec._state == "idle" + and next(BfBot.Exec._casters) == nil + then + _ok("_Complete() with stale state hard-reset to idle") + else + _nok(string.format("_Complete() with stale state: ok=%s err=%s state=%s casters_empty=%s", + tostring(cmpOk), tostring(cmpErr), tostring(BfBot.Exec._state), + tostring(next(BfBot.Exec._casters) == nil))) + end + + -- ---- Test 5: _SafetyTick proactively resets stale state ---- + P("") + P(" [5] _SafetyTick proactively resets stale state") + _poison(false) + BfBot.Exec._lastSafetyTick = 0 -- force tick to run past rate limit + local stOk, stErr = pcall(BfBot.Exec._SafetyTick) + if stOk + and BfBot.Exec._state == "idle" + and next(BfBot.Exec._casters) == nil + then + _ok("_SafetyTick reset stale running state to idle") + else + _nok(string.format("_SafetyTick stale reset: ok=%s err=%s state=%s casters_empty=%s", + tostring(stOk), tostring(stErr), tostring(BfBot.Exec._state), + tostring(next(BfBot.Exec._casters) == nil))) + end + + -- ---- Test 6: Stop() with name-match (same-save reload) re-resolves + -- sprite from portrait instead of dereferencing cached pointer. + -- This is the critical safety property: even when _IsStateStale misses + -- the case (party composition unchanged), Stop must not pass the cached + -- caster.sprite to engine functions. We poison caster.sprite with a + -- string sentinel and set caster.name to match the current portrait; + -- _IsStateStale returns false, the cleanup loop runs, and it must + -- re-resolve from EEex_Sprite_GetInPortrait(slot) without crashing. + P("") + P(" [6] Stop() with same-party reload re-resolves sprite from portrait") + if sprite then -- captured in Test 2b + local realName = BfBot._GetName(sprite) + BfBot.Exec._state = "running" + BfBot.Exec._casters = { + [0] = { + sprite = STALE, -- simulated freed pointer + name = realName, -- matches portrait → not detected as stale + cheatApplied = true, -- forces cleanup loop to engage + queue = {}, index = 0, done = false, cheatBoundary = 0, + }, + } + BfBot.Exec._activeCasters = 1 + local rrOk, rrErr = pcall(BfBot.Exec.Stop) + _reopenLog() -- Stop's normal exit closed the log + local entry = BfBot.Exec._casters[0] + if rrOk + and BfBot.Exec._state == "stopped" + and entry and entry.cheatApplied == false + then + _ok("Stop() did not deref STALE sentinel; cleanup ran on fresh portrait sprite") + else + _nok(string.format("Stop() with cached sentinel + name match: ok=%s err=%s state=%s cheatApplied=%s", + tostring(rrOk), tostring(rrErr), tostring(BfBot.Exec._state), + tostring(entry and entry.cheatApplied))) + end + else + _warning("No sprite in portrait 0; skipping re-resolve safety case") + end + + -- ---- Cleanup ---- + _restore() + + P("") + return _summary("Stale State") +end + -- ============================================================ -- BfBot.Test.RunAll — Full test suite -- ============================================================ @@ -2172,6 +2383,10 @@ function BfBot.Test.RunAll() local themingOk = BfBot.Test.Theming() P("") + -- Phase 15: Stale State Recovery (issue #38) + local staleOk = BfBot.Test.StaleState() + P("") + -- Summary P("========================================") P(" Fields: " .. (fieldsOk and "PASS" or "FAIL")) @@ -2190,11 +2405,12 @@ function BfBot.Test.RunAll() P(" Movable Panel: " .. (movPanelOk and "PASS" or "FAIL")) P(" Duration Recursion: " .. (durRecOk and "PASS" or "FAIL")) P(" Theming: " .. (themingOk and "PASS" or "FAIL")) + P(" Stale State: " .. (staleOk and "PASS" or "FAIL")) P("========================================") P("Log written to: " .. BfBot._logFile) BfBot._CloseLog() - return fieldsOk and classOk and scanOk and persistOk and qcOk and ovrOk and exportOk and scanRefOk and tgtOk and combatOk and subwinOk and lockOk and lockOrderOk and movPanelOk and durRecOk and themingOk + return fieldsOk and classOk and scanOk and persistOk and qcOk and ovrOk and exportOk and scanRefOk and tgtOk and combatOk and subwinOk and lockOk and lockOrderOk and movPanelOk and durRecOk and themingOk and staleOk end -- ============================================================ diff --git a/buffbot/BfBotUI.lua b/buffbot/BfBotUI.lua index 8b64394..b7ae754 100644 --- a/buffbot/BfBotUI.lua +++ b/buffbot/BfBotUI.lua @@ -1296,11 +1296,35 @@ function BfBot.UI._IsPresetSelected(idx) return BfBot.UI._presetIdx == idx end ---- Can we start casting? (exec idle + spells exist) +--- Can we start casting for the current character? (exec idle + current char has preset spells) function BfBot.UI._CanCast() return BfBot.Exec.GetState() ~= "running" and #buffbot_spellTable > 0 end +--- Can we start "Cast All"? (exec idle + any party member has preset spells) +--- Mirrors BuildQueueFromPreset's cross-party scope so the gate doesn't grey +--- out when only the currently-selected char has nothing configured. +function BfBot.UI._CanCastAll() + if BfBot.Exec.GetState() == "running" then return false end + if #buffbot_spellTable > 0 then return true end + local presetIdx = BfBot.UI._presetIdx + for slot = 0, 5 do + if slot ~= BfBot.UI._charSlot then + local sprite = EEex_Sprite_GetInPortrait(slot) + if sprite then + local config = BfBot.Persist.GetConfig(sprite) + if config and config.presets then + local preset = config.presets[presetIdx] + if preset and preset.spells and next(preset.spells) then + return true + end + end + end + end + end + return false +end + --- Is execution currently running? function BfBot.UI._IsRunning() return BfBot.Exec.GetState() == "running" diff --git a/buffbot/BuffBot.menu b/buffbot/BuffBot.menu index 7570f84..85371ea 100644 --- a/buffbot/BuffBot.menu +++ b/buffbot/BuffBot.menu @@ -721,7 +721,7 @@ menu { name "bbCast" enabled "buffbot_isOpen" - clickable lua "BfBot.UI._CanCast()" + clickable lua "BfBot.UI._CanCastAll()" action "BfBot.UI.Cast()" text lua "buffbot_castLabel" text style "bb_button" diff --git a/buffbot/setup-buffbot.tp2 b/buffbot/setup-buffbot.tp2 index b714c52..5dd185a 100644 --- a/buffbot/setup-buffbot.tp2 +++ b/buffbot/setup-buffbot.tp2 @@ -1,6 +1,6 @@ BACKUP ~weidu_external/backup/buffbot~ AUTHOR ~Chrizhermann (github.com/Chrizhermann/bg-eeex-buffbot)~ -VERSION ~v1.3.14-alpha~ +VERSION ~v1.3.15-alpha~ BEGIN ~BuffBot: In-Game Buff Automation~ DESIGNATED 0