From 92770bbaa0fd72526e7fa3369f3297de55bf7eda Mon Sep 17 00:00:00 2001 From: vickydotbat Date: Sun, 17 May 2026 15:33:21 +0200 Subject: [PATCH] Fix base parts not generating --- README.md | 11 +++--- internal/pipeline/pipeline_test.go | 10 ++++- internal/project/project.go | 10 ++--- internal/project/project_test.go | 38 ------------------- ..._PART_MODELS_IN_2DA_GENERATION_CONTRACT.md | 10 +++-- internal/topdata/parts_discovery.go | 14 ++----- internal/topdata/topdata_test.go | 16 ++++++-- 7 files changed, 38 insertions(+), 71 deletions(-) diff --git a/README.md b/README.md index 76403ba..de40591 100644 --- a/README.md +++ b/README.md @@ -182,7 +182,6 @@ generated_assets: kind: trailing_numeric_suffix group_from: first_path_segment parts_rows: - null_undiscovered_rows: true row_defaults: COSTMODIFIER: "0" acbonus: @@ -274,11 +273,11 @@ ACBONUS values before module overrides are applied. The IDs so higher part rows can sort before lower rows in the toolset; the legacy `descending_row_id_sort_key` strategy remains available for repositories that already depend on it. Per-dataset policies such as `chest: fixed 0.00` can -override the default. When `null_undiscovered_rows` is true, authored part rows -without discovered model IDs are omitted from the collected row set so dense -2DA output writes them as `****` null rows. Authored source ACBONUS values are -not authoritative when this policy is configured; files under `parts/modules` -remain the final override layer. +override the default. Authored source ACBONUS values are not authoritative when +this policy is configured, except that authored `****` values remain null +unless model discovery activates the row. Dense gaps without authored rows or +discovered part models are emitted as `****` null rows. Files under +`parts/modules` remain the final override layer. Extraction is deliberately guarded because it writes source files from binary archives. `extract.archives` is a list of glob patterns relative to diff --git a/internal/pipeline/pipeline_test.go b/internal/pipeline/pipeline_test.go index fc87227..60e4675 100644 --- a/internal/pipeline/pipeline_test.go +++ b/internal/pipeline/pipeline_test.go @@ -2248,7 +2248,6 @@ func TestBuildHAKsGeneratesParts2DAAssetsFromLocalModels(t *testing.T) { "group_from": "first_path_segment" }, "parts_rows": { - "null_undiscovered_rows": true, "row_defaults": {"COSTMODIFIER": "0"}, "acbonus": { "default": { @@ -2285,6 +2284,7 @@ func TestBuildHAKsGeneratesParts2DAAssetsFromLocalModels(t *testing.T) { "rows": [ {"id": 0, "COSTMODIFIER": 1, "ACBONUS": "0.25"}, {"id": 17, "COSTMODIFIER": 0, "ACBONUS": "0.90"}, + {"id": 19, "COSTMODIFIER": 0, "ACBONUS": "****"}, {"id": 117, "COSTMODIFIER": 0, "ACBONUS": "****"} ] } @@ -2327,7 +2327,13 @@ func TestBuildHAKsGeneratesParts2DAAssetsFromLocalModels(t *testing.T) { t.Fatalf("read generated parts 2da: %v", err) } partsText := string(partsRaw) - for _, want := range []string{"0\t****\t****", "17\t****\t****", "18\t0\t0.75", "117\t0\t1.17"} { + for _, want := range []string{ + "0\t1\t0.00", + "17\t0\t0.17", + "18\t0\t0.75", + "19\t****\t****", + "117\t0\t1.17", + } { if !strings.Contains(partsText, want) { t.Fatalf("expected generated parts 2da to contain %q after normalization and overrides, got:\n%s", want, partsText) } diff --git a/internal/project/project.go b/internal/project/project.go index 9db2cfd..49c7d8b 100644 --- a/internal/project/project.go +++ b/internal/project/project.go @@ -297,10 +297,9 @@ type AutogenConsumerConfig struct { } type PartsRowsConfig struct { - NullUndiscoveredRows bool `json:"null_undiscovered_rows" yaml:"null_undiscovered_rows"` - RowDefaults map[string]string `json:"row_defaults" yaml:"row_defaults"` - ACBonus PartsRowsACBonusConfig `json:"acbonus" yaml:"acbonus"` - Datasets map[string]PartsRowsDatasetConfig `json:"datasets" yaml:"datasets"` + RowDefaults map[string]string `json:"row_defaults" yaml:"row_defaults"` + ACBonus PartsRowsACBonusConfig `json:"acbonus" yaml:"acbonus"` + Datasets map[string]PartsRowsDatasetConfig `json:"datasets" yaml:"datasets"` } type PartsRowsDatasetConfig struct { @@ -1396,9 +1395,6 @@ func validateGeneratedConfig(cfg GeneratedConfig) []error { } func partsRowsConfigConfigured(cfg PartsRowsConfig) bool { - if cfg.NullUndiscoveredRows { - return true - } if len(cfg.RowDefaults) > 0 { return true } diff --git a/internal/project/project_test.go b/internal/project/project_test.go index 606b3de..bb326c5 100644 --- a/internal/project/project_test.go +++ b/internal/project/project_test.go @@ -907,44 +907,6 @@ func TestValidateLayoutAcceptsGeneratedTopData2DAConfig(t *testing.T) { } } -func TestValidateLayoutAcceptsGeneratedPartsRowsNullUndiscoveredWithoutACBonusPolicy(t *testing.T) { - root := t.TempDir() - mkdirAll(t, filepath.Join(root, "assets")) - mkdirAll(t, filepath.Join(root, "build")) - proj := Project{ - Root: root, - Config: Config{ - Module: ModuleConfig{Name: "Test", ResRef: "testmod"}, - Paths: PathConfig{Assets: "assets"}, - Generated: GeneratedConfig{ - TopData2DA: []GeneratedTopData2DAConfig{ - { - ID: "parts", - Source: "topdata", - Output: "{paths.cache}/generated-assets/parts-2da", - IncludeDatasets: []string{"parts/**"}, - PackageRoot: "part", - Autogen: AutogenConsumerConfig{ - ID: "parts", - Mode: "parts_rows", - Root: "part", - Include: []string{"**/*.mdl"}, - Derive: AutogenDeriveConfig{Kind: "trailing_numeric_suffix", GroupFrom: "first_path_segment"}, - PartsRows: PartsRowsConfig{ - NullUndiscoveredRows: true, - }, - }, - }, - }, - }, - }, - } - - if err := proj.ValidateLayout(); err != nil { - t.Fatalf("ValidateLayout returned error: %v", err) - } -} - func TestValidateLayoutRejectsEscapingGeneratedTopData2DAConfig(t *testing.T) { root := t.TempDir() mkdirAll(t, filepath.Join(root, "assets")) diff --git a/internal/topdata/AUTO-INCLUDE_EXISTING_PART_MODELS_IN_2DA_GENERATION_CONTRACT.md b/internal/topdata/AUTO-INCLUDE_EXISTING_PART_MODELS_IN_2DA_GENERATION_CONTRACT.md index 9fc73df..cb812a5 100644 --- a/internal/topdata/AUTO-INCLUDE_EXISTING_PART_MODELS_IN_2DA_GENERATION_CONTRACT.md +++ b/internal/topdata/AUTO-INCLUDE_EXISTING_PART_MODELS_IN_2DA_GENERATION_CONTRACT.md @@ -136,11 +136,13 @@ The superseded implementation was correct only if all of the following were true - `COSTMODIFIER = 0` - `ACBONUS = 0.00` -6. When configured, rows without discovered models are emitted as dense null +6. Authored base rows remain exposed. Authored `ACBONUS = ****` rows remain + null unless model discovery activates the row. +7. Dense gaps without authored rows or discovered models are emitted as null rows with `COSTMODIFIER = ****` and `ACBONUS = ****`. -7. File overrides are applied after discovery and take precedence. -8. The final output includes all eligible existing models present in the manifest. -9. Discovery does not require a sibling local `sow-assets` checkout or a Git clone. +8. File overrides are applied after discovery and take precedence. +9. The final output includes all eligible existing models present in the manifest. +10. Discovery does not require a sibling local `sow-assets` checkout or a Git clone. --- diff --git a/internal/topdata/parts_discovery.go b/internal/topdata/parts_discovery.go index 67cb6f6..623c0e3 100644 --- a/internal/topdata/parts_discovery.go +++ b/internal/topdata/parts_discovery.go @@ -474,7 +474,9 @@ func normalizePartsRowsACBonus(collected []nativeCollectedDataset, cfg project.P } } rowID, ok := cloned["id"].(int) - if ok { + if isUnsetPartValue(cloned["ACBONUS"]) { + cloned["COSTMODIFIER"] = nullValue + } else if ok { if value, configured := configuredPartsRowsACBonusValue(rowID, dataset.Dataset.Name, cfg); configured { cloned["ACBONUS"] = value } @@ -663,15 +665,7 @@ func augmentWithAutogeneratedPartsWithConfig(collected []nativeCollectedDataset, // Add rows for discovered IDs that don't already exist. newRows := make([]map[string]any, 0, len(dataset.Rows)) - for _, row := range dataset.Rows { - rowID, ok := row["id"].(int) - if cfg.NullUndiscoveredRows && ok { - if _, discovered := ids[rowID]; !discovered { - continue - } - } - newRows = append(newRows, row) - } + newRows = append(newRows, dataset.Rows...) for rowID := range ids { if row, exists := existingRows[rowID]; exists { diff --git a/internal/topdata/topdata_test.go b/internal/topdata/topdata_test.go index d235df1..a4cb5c5 100644 --- a/internal/topdata/topdata_test.go +++ b/internal/topdata/topdata_test.go @@ -10533,6 +10533,7 @@ func TestNormalizePartsRowsACBonusUsesConfiguredPolicyBeforeOverrides(t *testing Rows: []map[string]any{ {"id": 1, "COSTMODIFIER": "5", "ACBONUS": "0.90"}, {"id": 999, "COSTMODIFIER": "0", "ACBONUS": "****"}, + {"id": 1000, "COSTMODIFIER": "0", "ACBONUS": "0.00"}, }, }, { @@ -10569,8 +10570,11 @@ func TestNormalizePartsRowsACBonusUsesConfiguredPolicyBeforeOverrides(t *testing if got := beltRows[1]["ACBONUS"]; got != "9.98" { t.Fatalf("expected authored belt ACBONUS to be replaced with computed sort key 9.98, got %v", got) } - if got := beltRows[999]["ACBONUS"]; got != "0.00" { - t.Fatalf("expected highest belt row to sort at 0.00, got %v", got) + if got := beltRows[999]["ACBONUS"]; got != "****" { + t.Fatalf("expected authored null belt row to remain ****, got %v", got) + } + if got := beltRows[1000]["ACBONUS"]; got != "-0.01" { + t.Fatalf("expected non-null high belt row to use computed sort key, got %v", got) } chestRows := rowsByID(got[1].Rows) if got := chestRows[14]["ACBONUS"]; got != "0.00" { @@ -10588,8 +10592,9 @@ func TestNormalizePartsRowsACBonusSupportsAscendingRowIDSortKey(t *testing.T) { Dataset: nativeDataset{Name: "parts/belt", OutputName: "parts_belt.2da"}, Columns: []string{"COSTMODIFIER", "ACBONUS"}, Rows: []map[string]any{ - {"id": 1, "COSTMODIFIER": "0", "ACBONUS": "****"}, - {"id": 117, "COSTMODIFIER": "0", "ACBONUS": "****"}, + {"id": 1, "COSTMODIFIER": "0", "ACBONUS": "0.00"}, + {"id": 117, "COSTMODIFIER": "0", "ACBONUS": "0.00"}, + {"id": 118, "COSTMODIFIER": "0", "ACBONUS": "****"}, }, }, } @@ -10611,6 +10616,9 @@ func TestNormalizePartsRowsACBonusSupportsAscendingRowIDSortKey(t *testing.T) { if got := beltRows[117]["ACBONUS"]; got != "1.17" { t.Fatalf("expected high belt row to get higher sort key 1.17, got %v", got) } + if got := beltRows[118]["ACBONUS"]; got != "****" { + t.Fatalf("expected authored null belt row to remain ****, got %v", got) + } } func rowsByID(rows []map[string]any) map[int]map[string]any {