Skip to content

Commit 3698c9a

Browse files
TimothyLukeclaude
andcommitted
#1983 Editor fixes relating to version CRUD tasks.
GSE.ReplaceSequence stored the caller's own table into GSE.Library, so from the first Save of a session the editor's working sequence and the Library entry were the same object. Every per-version handler carries a "mirror into the Library so the tree updates before Save" block, and post-Save that block was writing into the same array a second time: Delete removed two versions, New Version inserted two, drag reorder moved twice. The extra version also left the tree pointing at an index the reloaded sequence did not have, which is the nil-index error on the way back into the editor. The Library now holds a clone, which is the invariant those mirror blocks were written against — fixed at the source rather than by guarding each mirror site. New Version left the Save button disabled. The macro-length gate is the only thing that disables Save, and the newversion branch refreshed it BEFORE inserting, so it measured the previously selected version; if that one was over the limit, a brand-new copy of Default inherited a disabled Save and nothing re-evaluated it until macro text happened to change. GUIDrawMacroEditor does not refresh the gate itself, so the branch now does it against the new index. Version-delete metadata rules, which were wrong in both directions: - MetaData.Default was decremented on EVERY delete, so removing version 5 while Default was 4 silently moved Default to 3 and the sequence ran a macro the author never chose. Only references AFTER the deleted version move now. - A context override pointing AT the deleted version was silently repointed at Default. It now blocks the delete and names the entries to change first, the same way Default already did. The rule and the key list move to Storage.lua as GSE.VersionReferencesInUse and GSE.ShiftVersionReferencesAfterDelete, with the keys derived from contextVersionPriority instead of hand-listed. The editor had three separate copies of that list; a context added to the runtime and missed in one of them is a reference silently left pointing at the wrong version. Drops the ten "X setting changed to Default." strings, dead now that the silent repoint is gone. No other locale carried them. spec/versionrefs_spec.lua covers the three delete cases, the ten keys, string indexes, junk input, and that the Library no longer aliases the caller. 163 busted tests, luacheck clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017wmCKbEwmEgWW8SpWnXJhC
1 parent 34b29a5 commit 3698c9a

5 files changed

Lines changed: 323 additions & 122 deletions

File tree

GSE/API/Storage.lua

Lines changed: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -514,21 +514,35 @@ function GSE.SnapshotDependentMacros(sequence)
514514
end
515515

516516
--- Replace a current version of a Macro
517+
--- Store a sequence into the Library and the saved variables.
518+
--
519+
-- The Library entry is a CLONE, never the caller's own table. The editor keeps
520+
-- its working copy in editframe.Sequence and hands that same table here on
521+
-- Save; storing it directly made the two the same object, and from the first
522+
-- Save of a session onwards every per-version operation ran twice — once as
523+
-- the editor's own edit, then again through the "mirror into the Library so
524+
-- the tree updates before Save" block in each handler, which was by then
525+
-- writing into the same array. Delete removed two versions, New Version
526+
-- inserted two, drag reorder moved twice. The extra version also left the
527+
-- tree pointing at an index the reloaded sequence did not have, which is the
528+
-- nil-index error on the way back into the editor. Cloning restores the
529+
-- invariant those mirror blocks were written against: the Library holds a
530+
-- separate copy that only this function replaces.
517531
function GSE.ReplaceSequence(classid, sequenceName, sequence)
518532
if GSE.SanitizeSequenceEditorMarkup then
519533
GSE.SanitizeSequenceEditorMarkup(sequence)
520534
end
521535
GSE.ComputeSequenceDependencies(sequence)
522536
GSE.SnapshotDependentMacros(sequence)
523537
if GSE.UpdateDeltaFork and GSE.UpdateDeltaFork(sequence) then
524-
GSE.Library[classid][sequenceName] = sequence
538+
GSE.Library[classid][sequenceName] = GSE.CloneSequence(sequence)
525539
GSE:SendMessage(Statics.Messages.SEQUENCE_UPDATED, sequenceName)
526540
return
527541
end
528542
-- Checksum is stamped on export only, not on save, so the stored checksum
529543
-- always reflects the last-exported state rather than the current edit state.
530544
GSESequences[classid][sequenceName] = GSE.EncodeMessage({sequenceName, sequence})
531-
GSE.Library[classid][sequenceName] = sequence
545+
GSE.Library[classid][sequenceName] = GSE.CloneSequence(sequence)
532546
GSE:SendMessage(Statics.Messages.SEQUENCE_UPDATED, sequenceName)
533547
end
534548

@@ -1038,6 +1052,70 @@ local contextVersionPriority = {
10381052
{ metaKey = "Party", flag = "inParty", valueKey = "Party" },
10391053
}
10401054

1055+
--- Every MetaData key that holds a VERSION NUMBER, derived from
1056+
-- contextVersionPriority above rather than repeated. PVP appears twice in
1057+
-- that table (once routing to Arena), so dedupe on valueKey.
1058+
local contextVersionKeys = {}
1059+
do
1060+
local seen = {}
1061+
for _, entry in ipairs(contextVersionPriority) do
1062+
if entry.valueKey and not seen[entry.valueKey] then
1063+
seen[entry.valueKey] = true
1064+
contextVersionKeys[#contextVersionKeys + 1] = entry.valueKey
1065+
end
1066+
end
1067+
end
1068+
1069+
--- The context keys, in priority order. Callers must not mutate the result.
1070+
function GSE.GetContextVersionKeys()
1071+
return contextVersionKeys
1072+
end
1073+
1074+
--- Which MetaData entries point AT `version`.
1075+
-- Returns a list of key names, empty when nothing references it. Deleting a
1076+
-- version that something points at is refused rather than silently repointed:
1077+
-- an author who set Raid to version 4 chose that, and moving it to Default
1078+
-- behind their back changes which macro fires in a raid.
1079+
function GSE.VersionReferencesInUse(metadata, version)
1080+
local inUse = {}
1081+
if type(metadata) ~= "table" then return inUse end
1082+
version = tonumber(version)
1083+
if not version then return inUse end
1084+
if tonumber(metadata.Default) == version then inUse[#inUse + 1] = "Default" end
1085+
for _, key in ipairs(contextVersionKeys) do
1086+
if tonumber(metadata[key]) == version then inUse[#inUse + 1] = key end
1087+
end
1088+
return inUse
1089+
end
1090+
1091+
--- Move every version reference down one slot after `version` was deleted.
1092+
--
1093+
-- Only references AFTER the deleted version move — what they point at slid
1094+
-- down by one. A reference BEFORE it cannot be affected: deleting a later
1095+
-- version does not renumber earlier ones. A reference AT it never reaches
1096+
-- here, because VersionReferencesInUse blocks the delete first.
1097+
--
1098+
-- Default used to be decremented unconditionally, so deleting version 5 while
1099+
-- Default was 4 silently moved Default to 3 — the sequence then ran a macro
1100+
-- the author never selected.
1101+
function GSE.ShiftVersionReferencesAfterDelete(metadata, version)
1102+
if type(metadata) ~= "table" then return end
1103+
version = tonumber(version)
1104+
if not version then return end
1105+
local function shift(value)
1106+
local n = tonumber(value)
1107+
if n and n > version then return n - 1 end
1108+
return value
1109+
end
1110+
metadata.Default = shift(metadata.Default)
1111+
for _, key in ipairs(contextVersionKeys) do
1112+
if not GSE.isEmpty(metadata[key]) then
1113+
metadata[key] = shift(metadata[key])
1114+
end
1115+
end
1116+
end
1117+
1118+
10411119
local function isPVESoloContext()
10421120
return not (
10431121
GSE.inScenario or

GSE/Localization/ModL_enUS.lua

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,6 @@ L[
264264
] = true
265265
L["Macro Version %d deleted."] = true
266266
L["This change will not come into effect until you save this macro."] = true
267-
L["PVP setting changed to Default."] = true
268267
L["Delete"] = true
269268
L["Cancel"] = true
270269
L["Addin Version %s contained versions for the following sequences:"] = true
@@ -353,9 +352,6 @@ L["Gnome Sequencer: Compress a Sequence String."] = true
353352
L["Compress Sequence from Forums"] = true
354353
L["Sequence to Compress."] = true
355354
L["Compress"] = true
356-
L["Heroic setting changed to Default."] = true
357-
L["Dungeon setting changed to Default."] = true
358-
L["Party setting changed to Default."] = true
359355
L[
360356
"Macro found by the name %sPVP%s. Rename this macro to a different name to be able to use it. WOW has a global object called PVP that is referenced instead of this macro."
361357
] = true
@@ -365,7 +361,6 @@ L["Random - It will select .... a spell, any spell"] = true
365361

366362
-- GSE 2.3.00
367363
L["The GUI has not been loaded. Please activate this plugin amongst WoW's addons to use the GSE GUI."] = true
368-
L["Arena setting changed to Default."] = true
369364
L["Arena"] = true
370365
L["Local Macro"] = true
371366
L["Updated Macro"] = true
@@ -388,8 +383,6 @@ L[" was imported as a new macro."] = true
388383
L["New Sequence Name"] = true
389384

390385
-- GSE 2.3.09
391-
L["Mythic+ setting changed to Default."] = true
392-
L["Timewalking setting changed to Default."] = true
393386

394387
-- GSE 2.4.01
395388
L[
@@ -441,8 +434,6 @@ L["Hide Minimap Icon"] = true
441434
L["Hide Minimap Icon for LibDataBroker (LDB) data text."] = true
442435

443436
-- GSE 2.4.15 - Missing translations
444-
L["Raid setting changed to Default."] = true
445-
L["Mythic setting changed to Default."] = true
446437

447438
-- GSE 2.5.0
448439
L["The milliseconds being used in key click delay."] = true
@@ -457,7 +448,6 @@ L["Version"] = true
457448

458449
-- 2.5.9
459450
L["The version of this macro to use in Delves and Scenarios."] = true
460-
L["Delves and Scenarios setting changed to Default."] = true
461451

462452
-- 2.6.01
463453
L["Variables"] = true
@@ -951,3 +941,6 @@ L["The delay in seconds between Out of Combat Queue Polls. The Out of Combat Qu
951941
L["The following people donate monthly via Patreon for the ongoing maintenance and development of GSE. Their support is greatly appreciated."] = true
952942
L["These options combine to allow you to reset a sequence while it is running. These options are Cumulative ie they add to each other. Options Like LeftClick and RightClick won't work together very well."] = true
953943
L["This is a common WoW setting used by all addons; it controls when your action buttons respond. On: they react when you press the key (key-down). Off: they react when you release it (key-up). GSE now works either way -- Actionbar Overrides and keybinds fire a single step in both states. With this on, GSE keybinds also fire on key-down for a faster response. Changes apply immediately out of combat (or on your next rebind if toggled mid-combat)."] = true
944+
L["Version %d is in use by: %s. Point %s at another version on the Configuration tab before deleting this one."] = true
945+
L["it"] = true
946+
L["them"] = true

GSE_GUI/Editor.lua

Lines changed: 27 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -5973,6 +5973,29 @@ function GSE.CreateEditor()
59735973
)
59745974
return
59755975
end
5976+
5977+
-- A context override pointing AT this version blocks the delete
5978+
-- the same way Default does. Silently repointing it at Default
5979+
-- (what this used to do) changes which macro fires in that
5980+
-- context without telling the author — they set Raid to version
5981+
-- 4 deliberately. Make them repoint it first. The key list and
5982+
-- the rule live in Storage.lua, derived from
5983+
-- contextVersionPriority, so the editor cannot drift from the
5984+
-- runtime's idea of which contexts exist.
5985+
local blocking = GSE.VersionReferencesInUse(sequence.MetaData, version)
5986+
if #blocking > 0 then
5987+
GSE.Print(
5988+
string.format(
5989+
L["Version %d is in use by: %s. Point %s at another version on the Configuration tab before deleting this one."],
5990+
version,
5991+
table.concat(blocking, ", "),
5992+
#blocking == 1 and L["it"] or L["them"]
5993+
),
5994+
Statics.DebugModules["Editor"]
5995+
)
5996+
return
5997+
end
5998+
59765999
GSE.UI.ShowConfirmDialog({
59776000
owner = editframe,
59786001
title = L["Delete Version"],
@@ -5987,113 +6010,11 @@ function GSE.CreateEditor()
59876010
cancelText = L["Cancel"],
59886011
onConfirm = function()
59896012
local printtext = L["Macro Version %d deleted."]
5990-
if sequence.MetaData.PVP == version then
5991-
sequence.MetaData.PVP = sequence.MetaData.Default
5992-
printtext = printtext .. " " .. L["PVP setting changed to Default."]
5993-
end
5994-
if sequence.MetaData.Arena == version then
5995-
sequence.MetaData.Arena = sequence.MetaData.Default
5996-
printtext = printtext .. " " .. L["Arena setting changed to Default."]
5997-
end
5998-
if sequence.MetaData.Raid == version then
5999-
sequence.MetaData.Raid = sequence.MetaData.Default
6000-
printtext = printtext .. " " .. L["Raid setting changed to Default."]
6001-
end
6002-
if sequence.MetaData.Mythic == version then
6003-
sequence.MetaData.Mythic = sequence.MetaData.Default
6004-
printtext = printtext .. " " .. L["Mythic setting changed to Default."]
6005-
end
6006-
if sequence.MetaData.Heroic == version then
6007-
sequence.MetaData.Heroic = sequence.MetaData.Default
6008-
printtext = printtext .. " " .. L["Heroic setting changed to Default."]
6009-
end
6010-
if sequence.MetaData.Dungeon == version then
6011-
sequence.MetaData.Dungeon = sequence.MetaData.Default
6012-
printtext = printtext .. " " .. L["Dungeon setting changed to Default."]
6013-
end
6014-
if sequence.MetaData.Party == version then
6015-
sequence.MetaData.Party = sequence.MetaData.Default
6016-
printtext = printtext .. " " .. L["Party setting changed to Default."]
6017-
end
6018-
if sequence.MetaData.MythicPlus == version then
6019-
sequence.MetaData.MythicPlus = sequence.MetaData.Default
6020-
printtext = printtext .. " " .. L["Mythic+ setting changed to Default."]
6021-
end
6022-
if sequence.MetaData.Timewalking == version then
6023-
sequence.MetaData.Timewalking = sequence.MetaData.Default
6024-
printtext = printtext .. " " .. L["Timewalking setting changed to Default."]
6025-
end
6026-
if sequence.MetaData.Scenario == version then
6027-
sequence.MetaData.Scenario = sequence.MetaData.Default
6028-
printtext = printtext .. " " .. L["Delves and Scenarios setting changed to Default."]
6029-
end
60306013

6031-
if sequence.MetaData.Default > 1 then
6032-
sequence.MetaData.Default = tonumber(sequence.MetaData.Default) - 1
6033-
else
6034-
sequence.MetaData.Default = 1
6035-
end
6014+
-- Only references after the deleted version move; see
6015+
-- GSE.ShiftVersionReferencesAfterDelete for the three cases.
6016+
GSE.ShiftVersionReferencesAfterDelete(sequence.MetaData, version)
60366017

6037-
if
6038-
not GSE.isEmpty(sequence.MetaData.PVP) and sequence.MetaData.PVP > 1 and
6039-
sequence.MetaData.PVP >= version
6040-
then
6041-
sequence.MetaData.PVP = tonumber(sequence.MetaData.PVP) - 1
6042-
end
6043-
if
6044-
not GSE.isEmpty(sequence.MetaData.Arena) and sequence.MetaData.Arena > 1 and
6045-
sequence.MetaData.Arena >= version
6046-
then
6047-
sequence.MetaData.Arena = tonumber(sequence.MetaData.Arena) - 1
6048-
end
6049-
if
6050-
not GSE.isEmpty(sequence.MetaData.Raid) and sequence.MetaData.Raid > 1 and
6051-
sequence.MetaData.Raid >= version
6052-
then
6053-
sequence.MetaData.Raid = tonumber(sequence.MetaData.Raid) - 1
6054-
end
6055-
if
6056-
not GSE.isEmpty(sequence.MetaData.Mythic) and sequence.MetaData.Mythic > 1 and
6057-
sequence.MetaData.Mythic >= version
6058-
then
6059-
sequence.MetaData.Mythic = tonumber(sequence.MetaData.Mythic) - 1
6060-
end
6061-
if
6062-
not GSE.isEmpty(sequence.MetaData.MythicPlus) and sequence.MetaData.MythicPlus > 1 and
6063-
sequence.MetaData.MythicPlus >= version
6064-
then
6065-
sequence.MetaData.MythicPlus = tonumber(sequence.MetaData.MythicPlus) - 1
6066-
end
6067-
if
6068-
not GSE.isEmpty(sequence.MetaData.Timewalking) and sequence.MetaData.Timewalking > 1 and
6069-
sequence.MetaData.Timewalking >= version
6070-
then
6071-
sequence.MetaData.Timewalking = tonumber(sequence.MetaData.Timewalking) - 1
6072-
end
6073-
if
6074-
not GSE.isEmpty(sequence.MetaData.Heroic) and sequence.MetaData.Heroic > 1 and
6075-
sequence.MetaData.Heroic >= version
6076-
then
6077-
sequence.MetaData.Heroic = tonumber(sequence.MetaData.Heroic) - 1
6078-
end
6079-
if
6080-
not GSE.isEmpty(sequence.MetaData.Dungeon) and sequence.MetaData.Dungeon > 1 and
6081-
sequence.MetaData.Dungeon >= version
6082-
then
6083-
sequence.MetaData.Dungeon = tonumber(sequence.MetaData.Dungeon) - 1
6084-
end
6085-
if
6086-
not GSE.isEmpty(sequence.MetaData.Party) and sequence.MetaData.Party > 1 and
6087-
sequence.MetaData.Party >= version
6088-
then
6089-
sequence.MetaData.Party = tonumber(sequence.MetaData.Party) - 1
6090-
end
6091-
if
6092-
not GSE.isEmpty(sequence.MetaData.Scenario) and sequence.MetaData.Scenario > 1 and
6093-
sequence.MetaData.Scenario >= version
6094-
then
6095-
sequence.MetaData.Scenario = tonumber(sequence.MetaData.Scenario) - 1
6096-
end
60976018
table.remove(sequence.Versions, version)
60986019

60996020
-- Mirror the deletion into the Library display cache so the
@@ -6110,11 +6031,7 @@ function GSE.CreateEditor()
61106031
table.remove(libSeq.Versions, version)
61116032
if libSeq.MetaData then
61126033
libSeq.MetaData.Default = sequence.MetaData.Default
6113-
local contextKeys = {
6114-
"Raid", "Arena", "Mythic", "MythicPlus", "PVP",
6115-
"Heroic", "Dungeon", "Timewalking", "Party", "Scenario",
6116-
}
6117-
for _, ck in ipairs(contextKeys) do
6034+
for _, ck in ipairs(GSE.GetContextVersionKeys()) do
61186035
libSeq.MetaData[ck] = sequence.MetaData[ck]
61196036
end
61206037
end

GSE_GUI/Editor_Tree.lua

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1292,6 +1292,17 @@ local function onClick_Sequences(editframe, container, group, unique, path, key,
12921292
end
12931293

12941294
editframe.GUIDrawMacroEditor(contentcontainer, newVersionIndex, table.concat(path, "\001"))
1295+
1296+
-- Recompute the Save gate for the version we just drew. The refresh
1297+
-- higher up in this branch ran BEFORE the insert, so it measured the
1298+
-- previously selected version: if that one was over the macro-length
1299+
-- limit, the brand-new version — a copy of Default, comfortably under
1300+
-- it — inherited a disabled Save button, and nothing re-evaluated it
1301+
-- until the author happened to edit macro text. GUIDrawMacroEditor
1302+
-- does not refresh this itself.
1303+
if editframe.RefreshMacroLimitSaveState then
1304+
editframe:RefreshMacroLimitSaveState(newVersionIndex)
1305+
end
12951306
editframe:SetTitle(
12961307
L["Sequence Editor"] .. ": " .. sequencename .. " (" .. L["New"] .. " " .. L["Version"] .. ")"
12971308
)

0 commit comments

Comments
 (0)