Fix Toolkit Overriding Semantics
- 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
This commit is contained in:
+23
-17
@@ -809,13 +809,7 @@ func collectBaseDataset(dataset nativeDataset) (nativeCollectedDataset, error) {
|
|||||||
lockModified = true
|
lockModified = true
|
||||||
return override, nil
|
return override, nil
|
||||||
}
|
}
|
||||||
cloned := make(map[string]any, len(override)-1)
|
return override, nil
|
||||||
for field, value := range override {
|
|
||||||
if field != "id" {
|
|
||||||
cloned[field] = value
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return cloned, nil
|
|
||||||
}
|
}
|
||||||
validExplicitIDForNewKey := func(key string, rowID int) bool {
|
validExplicitIDForNewKey := func(key string, rowID int) bool {
|
||||||
if key == "" {
|
if key == "" {
|
||||||
@@ -4063,18 +4057,23 @@ func resolveOverrideTarget(datasetName string, override map[string]any, rowByID
|
|||||||
}
|
}
|
||||||
|
|
||||||
func overrideRequestsNullRow(override map[string]any) bool {
|
func overrideRequestsNullRow(override map[string]any) bool {
|
||||||
rawKey, ok := override["key"]
|
value, ok := override["null"]
|
||||||
if !ok {
|
if !ok {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
if rawKey == nil {
|
switch typed := value.(type) {
|
||||||
return true
|
case bool:
|
||||||
}
|
return typed
|
||||||
typed, ok := rawKey.(string)
|
case string:
|
||||||
if !ok {
|
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 false
|
||||||
}
|
}
|
||||||
return strings.TrimSpace(typed) == nullValue
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func nullifyOverrideRow(row map[string]any, columns []string, rowByKey map[string]map[string]any) {
|
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
|
return changed, nil
|
||||||
}
|
}
|
||||||
newKey, ok := newKeyValue.(string)
|
newKey, ok := newKeyValue.(string)
|
||||||
if !ok || newKey == "" {
|
if !ok || newKey == "" || strings.TrimSpace(newKey) == nullValue {
|
||||||
delete(row, "key")
|
delete(row, "key")
|
||||||
return changed, nil
|
return changed, nil
|
||||||
}
|
}
|
||||||
|
conflictingID := -1
|
||||||
if conflicting, ok := rowByKey[newKey]; ok && conflicting != nil {
|
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
|
rowHasID := rowID != 0 || row["id"] != nil
|
||||||
if !conflictingHasID || !rowHasID || conflictingID != rowID {
|
if !conflictingHasID || !rowHasID || conflictingID != rowID {
|
||||||
delete(conflicting, "key")
|
delete(conflicting, "key")
|
||||||
@@ -4135,7 +4136,12 @@ func updateOverrideRowKey(datasetName string, row map[string]any, expanded map[s
|
|||||||
rowByKey[newKey] = row
|
rowByKey[newKey] = row
|
||||||
if existingID, ok := lockData[newKey]; ok {
|
if existingID, ok := lockData[newKey]; ok {
|
||||||
if existingID != rowID {
|
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 {
|
} else {
|
||||||
lockData[newKey] = rowID
|
lockData[newKey] = rowID
|
||||||
|
|||||||
@@ -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) {
|
func TestBuildCanonicalSkillsMatchesReference(t *testing.T) {
|
||||||
root := testProjectRoot(t)
|
root := testProjectRoot(t)
|
||||||
mkdirAll(t, filepath.Join(root, "topdata", "data"))
|
mkdirAll(t, filepath.Join(root, "topdata", "data"))
|
||||||
|
|||||||
Reference in New Issue
Block a user