From 6b38a14e5f31c70f67be09f95c67b69f0a4ccce9 Mon Sep 17 00:00:00 2001 From: xh-forge Date: Fri, 19 Jun 2026 14:35:49 -0700 Subject: [PATCH 1/5] Fix crash importing minion skill with missing activeSkillList --- src/Modules/Build.lua | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/Modules/Build.lua b/src/Modules/Build.lua index 84f03259ea..d1eb48277e 100644 --- a/src/Modules/Build.lua +++ b/src/Modules/Build.lua @@ -2041,20 +2041,22 @@ function buildMode:RefreshSkillSelectControls(controls, mainGroup, suffix) controls.mainSkillMinion.shown = true wipeTable(controls.mainSkillMinionSkill.list) if activeSkill.minion then - for _, minionSkill in ipairs(activeSkill.minion.activeSkillList) do - t_insert(controls.mainSkillMinionSkill.list, minionSkill.activeEffect.grantedEffect.name) - end - controls.mainSkillMinionSkill.selIndex = activeEffect.srcInstance["skillMinionSkill"..suffix] or 1 - controls.mainSkillMinionSkill.shown = true - controls.mainSkillMinionSkill.enabled = #controls.mainSkillMinionSkill.list > 1 - wipeTable(controls.mainSkillMinionSkillStatSet.list) - for _, statSet in ipairs(activeSkill.minion.activeSkillList[controls.mainSkillMinionSkill.selIndex].activeEffect.grantedEffect.statSets) do - t_insert(controls.mainSkillMinionSkillStatSet.list, {label = statSet.label, grantedEffectId = activeEffect.grantedEffect.id}) + if activeSkill.minion.activeSkillList and activeSkill.minion.activeSkillList[1] then + for _, minionSkill in ipairs(activeSkill.minion.activeSkillList) do + t_insert(controls.mainSkillMinionSkill.list, minionSkill.activeEffect.grantedEffect.name) + end + controls.mainSkillMinionSkill.selIndex = activeEffect.srcInstance["skillMinionSkill"..suffix] or 1 + controls.mainSkillMinionSkill.shown = true + controls.mainSkillMinionSkill.enabled = #controls.mainSkillMinionSkill.list > 1 + wipeTable(controls.mainSkillMinionSkillStatSet.list) + for _, statSet in ipairs(activeSkill.minion.activeSkillList[controls.mainSkillMinionSkill.selIndex].activeEffect.grantedEffect.statSets) do + t_insert(controls.mainSkillMinionSkillStatSet.list, {label = statSet.label, grantedEffectId = activeEffect.grantedEffect.id}) + end + local minionStatSetIndexLookup = activeEffect.srcInstance["skillMinionSkillStatSetIndexLookup"..suffix] + controls.mainSkillMinionSkillStatSet.selIndex = minionStatSetIndexLookup and minionStatSetIndexLookup[activeEffect.grantedEffect.id] and minionStatSetIndexLookup[activeEffect.grantedEffect.id][controls.mainSkillMinionSkill.selIndex] or 1 + controls.mainSkillMinionSkillStatSet.shown = true + controls.mainSkillMinionSkillStatSet.enabled = #controls.mainSkillMinionSkillStatSet.list > 1 end - local minionStatSetIndexLookup = activeEffect.srcInstance["skillMinionSkillStatSetIndexLookup"..suffix] - controls.mainSkillMinionSkillStatSet.selIndex = minionStatSetIndexLookup and minionStatSetIndexLookup[activeEffect.grantedEffect.id] and minionStatSetIndexLookup[activeEffect.grantedEffect.id][controls.mainSkillMinionSkill.selIndex] or 1 - controls.mainSkillMinionSkillStatSet.shown = true - controls.mainSkillMinionSkillStatSet.enabled = #controls.mainSkillMinionSkillStatSet.list > 1 else t_insert(controls.mainSkillMinion.list, "") end From b703fcb74178d34d2c39643cc28de96649fe9e94 Mon Sep 17 00:00:00 2001 From: xh-forge Date: Fri, 19 Jun 2026 17:19:21 -0700 Subject: [PATCH 2/5] Add tests for missing minion activeSkillList --- spec/System/TestSkills_spec.lua | 59 +++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/spec/System/TestSkills_spec.lua b/spec/System/TestSkills_spec.lua index 9609d08349..d971adc311 100644 --- a/spec/System/TestSkills_spec.lua +++ b/spec/System/TestSkills_spec.lua @@ -58,6 +58,65 @@ describe("TestSkills", function() assert.are.equals(minionId, build.controls.mainSkillMinion.list[1].minionId) end) + it("does not crash when minion activeSkillList is missing", function() + local srcInstance = { statSet = { }, skillPart = { }, nameSpec = "Minion: Test" } + local activeEffect = { + srcInstance = srcInstance, + grantedEffect = { + id = "TestMinionSkill", + name = "Minion: Test", + statSets = { { label = "Default" } }, + }, + statSet = { skillFlags = { } }, + } + local activeSkill = { + activeEffect = activeEffect, + skillData = { }, + minion = { + -- activeSkillList is absent, reproducing the crash fix in #2243 + } + } + build.skillsTab.socketGroupList[1] = { + displaySkillList = { activeSkill }, + mainActiveSkill = 1, + } + + assert.has_no.errors(function() + build:RefreshSkillSelectControls(build.controls, 1, "") + end) + end) + + it("does not crash when minion activeSkillList is an empty table", function() + local srcInstance = { statSet = { }, skillPart = { }, nameSpec = "Minion: Test" } + local activeEffect = { + srcInstance = srcInstance, + grantedEffect = { + id = "TestMinionSkill", + name = "Minion: Test", + statSets = { { label = "Default" } }, + }, + statSet = { skillFlags = { } }, + } + local activeSkill = { + activeEffect = activeEffect, + skillData = { }, + minion = { + activeSkillList = { } -- empty list, guard must check [1] as well + } + } + build.skillsTab.socketGroupList[1] = { + displaySkillList = { activeSkill }, + mainActiveSkill = 1, + } + + assert.has_no.errors(function() + build:RefreshSkillSelectControls(build.controls, 1, "") + end) + + -- Minion skill dropdown should remain hidden when there are no skills + assert.is_false(build.controls.mainSkillMinionSkill.shown) + end) + it("applies minion skill stat set selections to the selected minion skill only", function() build.skillsTab:PasteSocketGroup("Skeletal Sniper 20/0 1") runCallback("OnFrame") From f16084aaaf14cfe75e55f1dc32fed349607cdb74 Mon Sep 17 00:00:00 2001 From: xh-forge Date: Sat, 20 Jun 2026 08:52:01 -0700 Subject: [PATCH 3/5] Guard activeSkillList in Calcs.lua and add end-to-end minion test - Add nil guards for activeSkill.minion.activeSkillList at two sites in Calcs.lua (lines 469, 578) that would crash with ipairs(nil) during the same frame cycle as the Build.lua fix - Add end-to-end test using a real minion skill (Skeletal Sniper) through the full OnFrame pipeline to verify activeSkillList, mainSkill, and UI controls are properly populated --- spec/System/TestSkills_spec.lua | 24 ++++++++++++++++++++++++ src/Modules/Calcs.lua | 4 ++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/spec/System/TestSkills_spec.lua b/spec/System/TestSkills_spec.lua index d971adc311..62b67dd76d 100644 --- a/spec/System/TestSkills_spec.lua +++ b/spec/System/TestSkills_spec.lua @@ -117,7 +117,31 @@ describe("TestSkills", function() assert.is_false(build.controls.mainSkillMinionSkill.shown) end) + it("populates minion skill list and UI controls after full OnFrame cycle", function() + -- End-to-end test: exercises the complete pipeline including + -- calcs.buildOutput, calcs.perform, createMinionSkills, and + -- RefreshSkillSelectControls in a single OnFrame frame. + build.skillsTab:PasteSocketGroup("Skeletal Sniper 20/0 1") + + assert.has_no.errors(function() + runCallback("OnFrame") + end) + + -- Verify the calculation engine populated the minion correctly + local minion = build.calcsTab.mainEnv.minion + assert.is_not_nil(minion, "minion should be created by the calc engine") + assert.is_not_nil(minion.activeSkillList, "activeSkillList should be populated by createMinionSkills") + assert.is_true(#minion.activeSkillList > 0, "minion should have at least one skill") + assert.is_not_nil(minion.mainSkill, "mainSkill should be selected from activeSkillList") + + -- Verify the UI controls were populated by RefreshSkillSelectControls + assert.is_true(build.controls.mainSkillMinion.shown, "minion dropdown should be visible") + assert.is_true(build.controls.mainSkillMinionSkill.shown, "minion skill dropdown should be visible") + assert.is_true(#build.controls.mainSkillMinionSkill.list > 0, "minion skill dropdown should have entries") + end) + it("applies minion skill stat set selections to the selected minion skill only", function() + build.skillsTab:PasteSocketGroup("Skeletal Sniper 20/0 1") runCallback("OnFrame") diff --git a/src/Modules/Calcs.lua b/src/Modules/Calcs.lua index 12f8169c40..b64aea266d 100644 --- a/src/Modules/Calcs.lua +++ b/src/Modules/Calcs.lua @@ -466,7 +466,7 @@ function calcs.buildOutput(build, mode) for _, skillEffect in ipairs(activeSkill.effectList) do env.skillsUsed[skillEffect.grantedEffect.name] = true end - if activeSkill.minion then + if activeSkill.minion and activeSkill.minion.activeSkillList then for _, activeSkill in ipairs(activeSkill.minion.activeSkillList) do env.skillsUsed[activeSkill.activeEffect.grantedEffect.id] = true end @@ -575,7 +575,7 @@ function calcs.buildOutput(build, mode) addTo(env.tagTypesUsed, tag.type, mod) end end - if activeSkill.minion then + if activeSkill.minion and activeSkill.minion.activeSkillList then for _, activeSkill in pairs(activeSkill.minion.activeSkillList) do for _, mod in ipairs(activeSkill.baseSkillModList) do addModTags(env.minion, mod) From 446457baf35376037ee5224e045f4aaed21cf34b Mon Sep 17 00:00:00 2001 From: xh-forge Date: Sat, 20 Jun 2026 15:31:37 -0700 Subject: [PATCH 4/5] Guard missing minion main skill in tooltips --- spec/System/TestSkills_spec.lua | 16 ++++++++++++++++ src/Classes/SkillsTab.lua | 3 +-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/spec/System/TestSkills_spec.lua b/spec/System/TestSkills_spec.lua index 62b67dd76d..24a3eecc82 100644 --- a/spec/System/TestSkills_spec.lua +++ b/spec/System/TestSkills_spec.lua @@ -140,6 +140,22 @@ describe("TestSkills", function() assert.is_true(#build.controls.mainSkillMinionSkill.list > 0, "minion skill dropdown should have entries") end) + it("does not crash rendering socket tooltip when minion skill selection is missing", function() + build.skillsTab:PasteSocketGroup("Skeletal Sniper 20/0 1") + runCallback("OnFrame") + + local socketGroup = build.skillsTab.socketGroupList[1] + socketGroup.displaySkillList[1].minion.mainSkill = nil + + local tooltip = { + AddLine = function() end, + AddSeparator = function() end, + } + assert.has_no.errors(function() + build.skillsTab:AddSocketGroupTooltip(tooltip, socketGroup) + end) + end) + it("applies minion skill stat set selections to the selected minion skill only", function() build.skillsTab:PasteSocketGroup("Skeletal Sniper 20/0 1") diff --git a/src/Classes/SkillsTab.lua b/src/Classes/SkillsTab.lua index 3c43bfffe3..7a8e47606c 100644 --- a/src/Classes/SkillsTab.lua +++ b/src/Classes/SkillsTab.lua @@ -1329,7 +1329,7 @@ function SkillsTabClass:AddSocketGroupTooltip(tooltip, socketGroup) gemShown[skillEffect.srcInstance] = true end end - if activeSkill.minion then + if activeSkill.minion and activeSkill.minion.mainSkill then tooltip:AddSeparator(10) tooltip:AddLine(16, "^7Active Skill #" .. index .. "'s Main Minion Skill:") local activeEffect = activeSkill.minion.mainSkill.effectList[1] @@ -1557,4 +1557,3 @@ function SkillsTabClass:UpdateGlobalGemCountAssignments() end GlobalGemAssignments["GemGroupCount"] = countSocketGroups end - From e16778be3f70861735fddb69797cdbea60a288af Mon Sep 17 00:00:00 2001 From: xh-forge Date: Sat, 20 Jun 2026 17:20:46 -0700 Subject: [PATCH 5/5] Add integration test for minion skill character import (issue #2243) Reproduce the exact bug scenario: ImportItemsAndSkills with a minion skill gem, then ImportPassiveTreeAndJewels to trigger a full rebuild, then verify OnFrame completes without crashing and that the minion's activeSkillList and mainSkill are properly populated. --- spec/System/TestSkills_spec.lua | 57 +++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/spec/System/TestSkills_spec.lua b/spec/System/TestSkills_spec.lua index 24a3eecc82..ac119b1f15 100644 --- a/spec/System/TestSkills_spec.lua +++ b/spec/System/TestSkills_spec.lua @@ -156,6 +156,63 @@ describe("TestSkills", function() end) end) + it("does not crash when importing a character with a minion main skill and passive tree (issue #2243)", function() + -- Reproduces the exact bug scenario: ImportItemsAndSkills adds a minion + -- skill, ImportPassiveTreeAndJewels triggers a rebuild, and OnFrame must + -- complete without crashing in RefreshSkillSelectControls or Calcs. + local charData = { + level = 50, + class = "Witch2", + league = "Test", + equipment = {}, + skills = { + { + support = false, + typeLine = "Skeletal Sniper", + properties = { + { name = "Level", values = { { "20", 0 } } }, + { name = "Quality", values = { { "+0%", 0 } } }, + }, + }, + }, + } + + build.importTab.controls.charImportItemsClearSkills.state = true + build.importTab.controls.charImportItemsClearItems.state = false + build.importTab:ImportItemsAndSkills(charData) + + -- At this point the minion skill is in socketGroupList but the calc + -- engine hasn't run yet, so activeSkillList may be nil — the bug state. + runCallback("OnFrame") + + -- Now import the passive tree, which sets buildFlag and triggers another + -- full rebuild — this is the step that originally caused the crash. + build.importTab:ImportPassiveTreeAndJewels({ + name = "TestMinionImport", + class = "Witch2", + league = "Test", + level = 50, + jewels = {}, + passives = { + hashes = {}, + specialisations = {}, + skill_overrides = {}, + jewel_data = {}, + quest_stats = {}, + }, + }) + + assert.has_no.errors(function() + runCallback("OnFrame") + end) + + -- Verify the minion skill was properly initialised after the full cycle + local mainEnv = build.calcsTab.mainEnv + assert.is_not_nil(mainEnv.minion, "minion should exist after import") + assert.is_not_nil(mainEnv.minion.activeSkillList, "activeSkillList should be populated") + assert.is_not_nil(mainEnv.minion.mainSkill, "mainSkill should be set") + end) + it("applies minion skill stat set selections to the selected minion skill only", function() build.skillsTab:PasteSocketGroup("Skeletal Sniper 20/0 1")