From c66b7dcb2b57aaffbb86ff53df5ff4bc2d9d7c24 Mon Sep 17 00:00:00 2001 From: vickydotbat Date: Thu, 14 May 2026 00:15:58 +0200 Subject: [PATCH] Fix Toolkit Overriding Semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - key: null and key: "****" now mean “remove this row's identity” - only "null": true blanks the whole row - explicit id remains authoritative on id + key overrides - when a row claims a key from another row, the key and lock entry can move with it if the old row loses that key --- internal/topdata/native.go | 40 ++++---- internal/topdata/topdata_test.go | 167 +++++++++++++++++++++++++++++++ 2 files changed, 190 insertions(+), 17 deletions(-) diff --git a/internal/topdata/native.go b/internal/topdata/native.go index 5c0bfd1..8a6131e 100644 --- a/internal/topdata/native.go +++ b/internal/topdata/native.go @@ -809,13 +809,7 @@ func collectBaseDataset(dataset nativeDataset) (nativeCollectedDataset, error) { lockModified = true return override, nil } - cloned := make(map[string]any, len(override)-1) - for field, value := range override { - if field != "id" { - cloned[field] = value - } - } - return cloned, nil + return override, nil } validExplicitIDForNewKey := func(key string, rowID int) bool { if key == "" { @@ -4063,18 +4057,23 @@ func resolveOverrideTarget(datasetName string, override map[string]any, rowByID } func overrideRequestsNullRow(override map[string]any) bool { - rawKey, ok := override["key"] + value, ok := override["null"] if !ok { return false } - if rawKey == nil { - return true - } - typed, ok := rawKey.(string) - if !ok { + switch typed := value.(type) { + case bool: + return typed + case string: + trimmed := strings.TrimSpace(strings.ToLower(typed)) + return trimmed == "true" || trimmed == "1" || trimmed == "yes" + case int: + return typed != 0 + case float64: + return typed != 0 + default: return false } - return strings.TrimSpace(typed) == nullValue } func nullifyOverrideRow(row map[string]any, columns []string, rowByKey map[string]map[string]any) { @@ -4119,12 +4118,14 @@ func updateOverrideRowKey(datasetName string, row map[string]any, expanded map[s return changed, nil } newKey, ok := newKeyValue.(string) - if !ok || newKey == "" { + if !ok || newKey == "" || strings.TrimSpace(newKey) == nullValue { delete(row, "key") return changed, nil } + conflictingID := -1 if conflicting, ok := rowByKey[newKey]; ok && conflicting != nil { - conflictingID, conflictingHasID := conflicting["id"].(int) + var conflictingHasID bool + conflictingID, conflictingHasID = conflicting["id"].(int) rowHasID := rowID != 0 || row["id"] != nil if !conflictingHasID || !rowHasID || conflictingID != rowID { delete(conflicting, "key") @@ -4135,7 +4136,12 @@ func updateOverrideRowKey(datasetName string, row map[string]any, expanded map[s rowByKey[newKey] = row if existingID, ok := lockData[newKey]; ok { if existingID != rowID { - return false, fmt.Errorf("dataset %s: key %q is locked to id %d and cannot be rewritten to id %d", datasetName, newKey, existingID, rowID) + if conflictingID >= 0 && existingID == conflictingID { + lockData[newKey] = rowID + changed = true + } else { + return false, fmt.Errorf("dataset %s: key %q is locked to id %d and cannot be rewritten to id %d", datasetName, newKey, existingID, rowID) + } } } else { lockData[newKey] = rowID diff --git a/internal/topdata/topdata_test.go b/internal/topdata/topdata_test.go index 7ad07fb..bcccfc7 100644 --- a/internal/topdata/topdata_test.go +++ b/internal/topdata/topdata_test.go @@ -8169,6 +8169,173 @@ func TestBuildSupportsCanonicalSkills(t *testing.T) { } } +func TestBuildGeneratedSkillFocusUsesOverriddenCanonicalSkillKey(t *testing.T) { + root := testProjectRoot(t) + writeFeatGeneratedHarness(t, root, map[string]string{ + "skill_focus.json": `{ + "family": "skill_focus", + "family_key": "skillfocus", + "template": "masterfeats:skillfocus", + "name_prefix": "Skill Focus", + "label_prefix": "FEAT_SKILL_FOCUS", + "constant_prefix": "FEAT_SKILL_FOCUS", + "default_fields": { + "CRValue": "0.5", + "ReqSkillMinRanks": "1", + "ALLCLASSESCANUSE": "1", + "TOOLSCATEGORIES": "6", + "PreReqEpic": "0", + "ReqAction": "1" + }, + "child_ref_field": "REQSKILL", + "child_source": { + "dataset": "skills", + "predicate": "accessible" + } +}` + "\n", + "greater_skill_focus.json": `{ + "family": "greater_skill_focus", + "family_key": "greaterskillfocus", + "template": "masterfeats:greaterskillfocus", + "name_prefix": "Greater Skill Focus", + "label_prefix": "FEAT_GREATER_SKILL_FOCUS", + "constant_prefix": "FEAT_GREATER_SKILL_FOCUS", + "default_fields": { + "CRValue": "0.2", + "ReqSkillMinRanks": "15", + "ALLCLASSESCANUSE": "1", + "TOOLSCATEGORIES": "6", + "PreReqEpic": "0", + "ReqAction": "1" + }, + "child_ref_field": "REQSKILL", + "child_source": { + "dataset": "skills", + "predicate": "accessible" + } +}` + "\n", + }, map[string]int{ + "feat:skillfocus_intimidate": 192, + "feat:greaterskillfocus_intimidate": 1305, + }) + writeFile(t, filepath.Join(root, "topdata", "data", "skills", "base.json"), `{ + "output": "skills.2da", + "columns": ["Label", "Name", "Description", "Icon", "Untrained", "KeyAbility", "ArmorCheckPenalty", "AllClassesCanUse", "Category", "MaxCR", "Constant", "HostileSkill", "HideFromLevelUp"], + "rows": [ + {"id": 18, "key": "skills:taunt", "Label": "Taunt", "Name": "280", "Description": "356", "Icon": "isk_taunt", "Untrained": "1", "KeyAbility": "CHA", "ArmorCheckPenalty": "0", "AllClassesCanUse": "1", "Category": "****", "MaxCR": "****", "Constant": "SKILL_TAUNT", "HostileSkill": "1", "HideFromLevelUp": "0"} + ] +}`+"\n") + writeFile(t, filepath.Join(root, "topdata", "data", "skills", "lock.json"), `{ + "skills:intimidate": 18 +}`+"\n") + mkdirAll(t, filepath.Join(root, "topdata", "data", "skills", "modules")) + writeFile(t, filepath.Join(root, "topdata", "data", "skills", "modules", "ovr_taunttointimidate.json"), `{ + "overrides": [ + { + "id": 18, + "key": "skills:intimidate", + "Label": "Intimidate", + "Constant": "SKILL_INTIMIDATE", + "HideFromLevelUp": "0" + } + ] +}`+"\n") + + result, err := BuildNative(testProject(root), nil) + if err != nil { + t.Fatalf("BuildNative failed: %v", err) + } + got, err := os.ReadFile(filepath.Join(result.Output2DADir, "feat.2da")) + if err != nil { + t.Fatalf("read feat.2da: %v", err) + } + text := string(got) + if !strings.Contains(text, "192\tFEAT_SKILL_FOCUS_INTIMIDATE\t") { + t.Fatalf("expected generated skill focus intimidate row, got:\n%s", text) + } + if !strings.Contains(text, "1305\tFEAT_GREATER_SKILL_FOCUS_INTIMIDATE\t") { + t.Fatalf("expected generated greater skill focus intimidate row, got:\n%s", text) + } + if strings.Contains(text, "FEAT_SKILL_FOCUS_TAUNT") || strings.Contains(text, "FEAT_GREATER_SKILL_FOCUS_TAUNT") { + t.Fatalf("expected generated feat families to follow overridden canonical skill key, got:\n%s", text) + } +} + +func TestOverrideRequestsNullRowUsesExplicitNullFlag(t *testing.T) { + if overrideRequestsNullRow(map[string]any{"key": nil}) { + t.Fatal("key: null should not blank a row") + } + if overrideRequestsNullRow(map[string]any{"key": "****"}) { + t.Fatal(`key: "****" should not blank a row`) + } + if !overrideRequestsNullRow(map[string]any{"null": true}) { + t.Fatal(`null: true should blank a row`) + } +} + +func TestBuildSkillsKeyNullRemovesIdentityWithoutNullingRow(t *testing.T) { + root := testProjectRoot(t) + mkdirAll(t, filepath.Join(root, "topdata", "data", "skills", "modules")) + writeFile(t, filepath.Join(root, "topdata", "base_dialog.json"), "{}\n") + writeFile(t, filepath.Join(root, "topdata", "data", "skills", "base.json"), `{ + "output": "skills.2da", + "columns": ["Label", "Name", "Description", "Icon", "Untrained", "KeyAbility", "ArmorCheckPenalty", "AllClassesCanUse", "Category", "MaxCR", "Constant", "HostileSkill", "HideFromLevelUp"], + "rows": [ + {"id": 18, "key": "skills:taunt", "Label": "Taunt", "Name": "280", "Description": "356", "Icon": "isk_taunt", "Untrained": "1", "KeyAbility": "CHA", "ArmorCheckPenalty": "0", "AllClassesCanUse": "1", "Category": "****", "MaxCR": "****", "Constant": "SKILL_TAUNT", "HostileSkill": "1", "HideFromLevelUp": "0"}, + {"id": 24, "key": "skills:intimidate", "Label": "Intimidate", "Name": "8756", "Description": "8786", "Icon": "isk_X2Inti", "Untrained": "1", "KeyAbility": "CHA", "ArmorCheckPenalty": "0", "AllClassesCanUse": "1", "Category": "****", "MaxCR": "****", "Constant": "SKILL_INTIMIDATE", "HostileSkill": "0", "HideFromLevelUp": "0"} + ] +}`+"\n") + writeFile(t, filepath.Join(root, "topdata", "data", "skills", "lock.json"), "{}\n") + writeFile(t, filepath.Join(root, "topdata", "data", "skills", "modules", "ovr_taunttointimidate.json"), `{ + "overrides": [ + { + "id": 18, + "key": "skills:intimidate", + "Label": "Intimidate", + "Constant": "SKILL_INTIMIDATE", + "HideFromLevelUp": "0" + } + ] +}`+"\n") + writeFile(t, filepath.Join(root, "topdata", "data", "skills", "modules", "rmv_removedskills.json"), `{ + "overrides": [ + { + "id": 24, + "key": null, + "Label": null, + "Constant": null, + "HideFromLevelUp": "1" + } + ] +}`+"\n") + mkdirAll(t, filepath.Join(root, "reference")) + writeFile(t, filepath.Join(root, "reference", "build.py"), "print('ok')\n") + + result, err := BuildNative(testProject(root), nil) + if err != nil { + t.Fatalf("BuildNative failed: %v", err) + } + lockRaw, err := os.ReadFile(filepath.Join(root, "topdata", "data", "skills", "lock.json")) + if err != nil { + t.Fatalf("read skills lock: %v", err) + } + lockText := string(lockRaw) + if !strings.Contains(lockText, `"skills:intimidate": 18`) { + t.Fatalf("expected remapped intimidate lock entry, got:\n%s", lockText) + } + if strings.Contains(lockText, `"skills:taunt"`) { + t.Fatalf("expected taunt to be removed from lock, got:\n%s", lockText) + } + got, err := os.ReadFile(filepath.Join(result.Output2DADir, "skills.2da")) + if err != nil { + t.Fatalf("read skills.2da: %v", err) + } + text := string(got) + if !strings.Contains(text, "24\t****\t8756\t8786\tisk_X2Inti\t1\tCHA\t0\t1\t****\t****\t****\t0\t1\n") { + t.Fatalf("expected id 24 row to remain present but de-identified, got:\n%s", text) + } +} + func TestBuildCanonicalSkillsMatchesReference(t *testing.T) { root := testProjectRoot(t) mkdirAll(t, filepath.Join(root, "topdata", "data"))