From b5a3a715a4b78bf48193ba8a69886dbae19ed5d4 Mon Sep 17 00:00:00 2001 From: vickydotbat Date: Sun, 17 May 2026 14:43:09 +0200 Subject: [PATCH] Sorting changes --- README.md | 17 +++-- internal/app/app_test.go | 6 +- internal/pipeline/pipeline_test.go | 9 ++- internal/project/project.go | 59 +++++++++----- internal/project/project_test.go | 40 +++++++++- ..._PART_MODELS_IN_2DA_GENERATION_CONTRACT.md | 12 ++- internal/topdata/generated_assets.go | 4 +- internal/topdata/parts_discovery.go | 18 ++++- internal/topdata/top_package.go | 71 +++++++++++++++++ internal/topdata/topdata_test.go | 76 +++++++++++++++++++ 10 files changed, 276 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index e2589c3..76403ba 100644 --- a/README.md +++ b/README.md @@ -182,12 +182,12 @@ generated_assets: kind: trailing_numeric_suffix group_from: first_path_segment parts_rows: + null_undiscovered_rows: true row_defaults: COSTMODIFIER: "0" acbonus: default: - strategy: descending_row_id_sort_key - max_row_id: 999 + strategy: ascending_row_id_sort_key divisor: 100 format: "%.2f" datasets: @@ -270,10 +270,15 @@ because this path does not produce TLK packages. For `parts_rows` generated assets, `parts_rows.acbonus` can normalize final ACBONUS values before module overrides are applied. The -`descending_row_id_sort_key` strategy writes toolset ordering values from row -IDs, and per-dataset policies such as `chest: fixed 0.00` can override that -default. Authored source ACBONUS values are not authoritative when this policy -is configured; files under `parts/modules` remain the final override layer. +`ascending_row_id_sort_key` strategy writes toolset ordering values from row +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. 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/app/app_test.go b/internal/app/app_test.go index 5eba59e..ba118fe 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -31,7 +31,11 @@ topdata: writeFile(t, filepath.Join(root, ".cache", "2da", "repadjust.2da"), "2DA V2.0\n\n Label\n0 TEST_LABEL\n") writeFile(t, filepath.Join(root, "build", "sow_tlk.tlk"), "compiled tlk") writeFile(t, filepath.Join(root, "topdata", "assets", "gui", "testicon.png"), "icon-data") - writeFile(t, filepath.Join(root, "topdata", "data", "repadjust", "base.json"), "{ this is intentionally invalid json }\n") + writeFile(t, filepath.Join(root, "topdata", "data", "repadjust", "base.json"), `{ + "output": "repadjust.2da", + "columns": ["Label"], + "rows": [{"id": 0, "Label": "TEST_LABEL"}] +}`+"\n") sourceTime := time.Now().Add(-2 * time.Hour) outputTime := time.Now().Add(-1 * time.Hour) diff --git a/internal/pipeline/pipeline_test.go b/internal/pipeline/pipeline_test.go index a4637d3..fc87227 100644 --- a/internal/pipeline/pipeline_test.go +++ b/internal/pipeline/pipeline_test.go @@ -2248,10 +2248,11 @@ func TestBuildHAKsGeneratesParts2DAAssetsFromLocalModels(t *testing.T) { "group_from": "first_path_segment" }, "parts_rows": { + "null_undiscovered_rows": true, "row_defaults": {"COSTMODIFIER": "0"}, "acbonus": { "default": { - "strategy": "descending_row_id_sort_key", + "strategy": "ascending_row_id_sort_key", "max_row_id": 999, "divisor": 100, "format": "%.2f" @@ -2277,12 +2278,14 @@ func TestBuildHAKsGeneratesParts2DAAssetsFromLocalModels(t *testing.T) { } `) mustWriteFile(t, filepath.Join(root, "assets", "part", "belt", "pfa0_belt018.mdl"), "belt") + mustWriteFile(t, filepath.Join(root, "assets", "part", "belt", "pfa0_belt117.mdl"), "belt") mustWriteFile(t, filepath.Join(root, "topdata", "data", "parts", "belt.json"), `{ "output": "parts_belt.2da", "columns": ["COSTMODIFIER", "ACBONUS"], "rows": [ {"id": 0, "COSTMODIFIER": 1, "ACBONUS": "0.25"}, - {"id": 17, "COSTMODIFIER": 0, "ACBONUS": "0.90"} + {"id": 17, "COSTMODIFIER": 0, "ACBONUS": "0.90"}, + {"id": 117, "COSTMODIFIER": 0, "ACBONUS": "****"} ] } `) @@ -2324,7 +2327,7 @@ func TestBuildHAKsGeneratesParts2DAAssetsFromLocalModels(t *testing.T) { t.Fatalf("read generated parts 2da: %v", err) } partsText := string(partsRaw) - for _, want := range []string{"0\t1\t9.99", "17\t0\t9.82", "18\t0\t0.75"} { + for _, want := range []string{"0\t****\t****", "17\t****\t****", "18\t0\t0.75", "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 749f32d..9db2cfd 100644 --- a/internal/project/project.go +++ b/internal/project/project.go @@ -297,9 +297,10 @@ type AutogenConsumerConfig struct { } type PartsRowsConfig struct { - RowDefaults map[string]string `json:"row_defaults" yaml:"row_defaults"` - ACBonus PartsRowsACBonusConfig `json:"acbonus" yaml:"acbonus"` - Datasets map[string]PartsRowsDatasetConfig `json:"datasets" yaml:"datasets"` + 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"` } type PartsRowsDatasetConfig struct { @@ -1395,6 +1396,9 @@ func validateGeneratedConfig(cfg GeneratedConfig) []error { } func partsRowsConfigConfigured(cfg PartsRowsConfig) bool { + if cfg.NullUndiscoveredRows { + return true + } if len(cfg.RowDefaults) > 0 { return true } @@ -1412,26 +1416,43 @@ func partsRowsConfigConfigured(cfg PartsRowsConfig) bool { func validatePartsRowsConfig(fieldPrefix string, cfg PartsRowsConfig) []error { var failures []error - failures = append(failures, validatePartsRowsACBonusPolicy(fieldPrefix+".acbonus.default", cfg.ACBonus.Default, false)...) - for dataset, policy := range cfg.ACBonus.Datasets { - dataset = strings.TrimSpace(dataset) - if dataset == "" { - failures = append(failures, fmt.Errorf("%s.acbonus.datasets contains an empty dataset key", fieldPrefix)) - continue + if partsRowsProjectACBonusConfigured(cfg) { + failures = append(failures, validatePartsRowsACBonusPolicy(fieldPrefix+".acbonus.default", cfg.ACBonus.Default, false)...) + for dataset, policy := range cfg.ACBonus.Datasets { + dataset = strings.TrimSpace(dataset) + if dataset == "" { + failures = append(failures, fmt.Errorf("%s.acbonus.datasets contains an empty dataset key", fieldPrefix)) + continue + } + failures = append(failures, validatePartsRowsACBonusPolicy(fieldPrefix+".acbonus.datasets."+dataset, policy, true)...) } - failures = append(failures, validatePartsRowsACBonusPolicy(fieldPrefix+".acbonus.datasets."+dataset, policy, true)...) - } - for dataset, datasetCfg := range cfg.Datasets { - dataset = strings.TrimSpace(dataset) - if dataset == "" { - failures = append(failures, fmt.Errorf("%s.datasets contains an empty dataset key", fieldPrefix)) - continue + for dataset, datasetCfg := range cfg.Datasets { + dataset = strings.TrimSpace(dataset) + if dataset == "" { + failures = append(failures, fmt.Errorf("%s.datasets contains an empty dataset key", fieldPrefix)) + continue + } + failures = append(failures, validatePartsRowsACBonusPolicy(fieldPrefix+".datasets."+dataset+".acbonus", datasetCfg.ACBonus, true)...) } - failures = append(failures, validatePartsRowsACBonusPolicy(fieldPrefix+".datasets."+dataset+".acbonus", datasetCfg.ACBonus, true)...) } return failures } +func partsRowsProjectACBonusConfigured(cfg PartsRowsConfig) bool { + if strings.TrimSpace(cfg.ACBonus.Default.Strategy) != "" { + return true + } + if len(cfg.ACBonus.Datasets) > 0 { + return true + } + for _, dataset := range cfg.Datasets { + if strings.TrimSpace(dataset.ACBonus.Strategy) != "" { + return true + } + } + return false +} + func validatePartsRowsACBonusPolicy(fieldPrefix string, policy PartsRowsACBonusPolicy, allowEmpty bool) []error { strategy := strings.TrimSpace(policy.Strategy) if strategy == "" { @@ -1441,9 +1462,9 @@ func validatePartsRowsACBonusPolicy(fieldPrefix string, policy PartsRowsACBonusP return []error{fmt.Errorf("%s.strategy is required", fieldPrefix)} } switch strategy { - case "descending_row_id_sort_key": + case "ascending_row_id_sort_key", "descending_row_id_sort_key": var failures []error - if policy.MaxRowID <= 0 { + if strategy == "descending_row_id_sort_key" && policy.MaxRowID <= 0 { failures = append(failures, fmt.Errorf("%s.max_row_id must be greater than 0", fieldPrefix)) } if policy.Divisor <= 0 { diff --git a/internal/project/project_test.go b/internal/project/project_test.go index 08801c6..606b3de 100644 --- a/internal/project/project_test.go +++ b/internal/project/project_test.go @@ -885,7 +885,7 @@ func TestValidateLayoutAcceptsGeneratedTopData2DAConfig(t *testing.T) { PartsRows: PartsRowsConfig{ RowDefaults: map[string]string{"COSTMODIFIER": "0"}, ACBonus: PartsRowsACBonusConfig{ - Default: PartsRowsACBonusPolicy{Strategy: "descending_row_id_sort_key", MaxRowID: 999, Divisor: 100, Format: "%.2f"}, + Default: PartsRowsACBonusPolicy{Strategy: "ascending_row_id_sort_key", Divisor: 100, Format: "%.2f"}, Datasets: map[string]PartsRowsACBonusPolicy{ "chest": {Strategy: "fixed", Value: "0.00"}, }, @@ -907,6 +907,44 @@ 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 80a0efd..9fc73df 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 @@ -98,6 +98,10 @@ For rows generated from discovered existing models, set: - `COSTMODIFIER = 0` - `ACBONUS = 0.00` +Rows without discovered existing models may be intentionally emitted as dense +2DA null rows when configured. In that mode, missing rows must have +`COSTMODIFIER = ****` and `ACBONUS = ****` in the final generated 2DA. + ### 5. Override precedence If file-based overrides exist, they must take precedence over discovered defaults. @@ -132,9 +136,11 @@ The superseded implementation was correct only if all of the following were true - `COSTMODIFIER = 0` - `ACBONUS = 0.00` -6. File overrides are applied after discovery and take precedence. -7. The final output includes all eligible existing models present in the manifest. -8. Discovery does not require a sibling local `sow-assets` checkout or a Git clone. +6. When configured, rows without discovered models are emitted as dense 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. --- diff --git a/internal/topdata/generated_assets.go b/internal/topdata/generated_assets.go index 2019d36..3c52367 100644 --- a/internal/topdata/generated_assets.go +++ b/internal/topdata/generated_assets.go @@ -101,7 +101,9 @@ func buildGenerated2DAAssetGroup(p *project.Project, cfg project.GeneratedTopDat return nil, err } outputPath := filepath.Join(outputDir, dataset.Dataset.OutputName) - if err := write2DA(compiled, outputPath, dataset.Dataset.Kind == nativeDatasetBase); err != nil { + denseRows := dataset.Dataset.Kind == nativeDatasetBase || + (consumer.Mode == "parts_rows" && isPartsDataset(dataset.Dataset.Name)) + if err := write2DA(compiled, outputPath, denseRows); err != nil { return nil, err } results = append(results, Generated2DAAsset{ diff --git a/internal/topdata/parts_discovery.go b/internal/topdata/parts_discovery.go index 90afd8b..67cb6f6 100644 --- a/internal/topdata/parts_discovery.go +++ b/internal/topdata/parts_discovery.go @@ -507,6 +507,12 @@ func configuredPartsRowsACBonusValue(rowID int, datasetName string, cfg project. return "", false } switch strings.TrimSpace(policy.Strategy) { + case "ascending_row_id_sort_key": + format := strings.TrimSpace(policy.Format) + if format == "" { + format = "%.2f" + } + return fmt.Sprintf(format, float64(rowID)/float64(policy.Divisor)), true case "descending_row_id_sort_key": format := strings.TrimSpace(policy.Format) if format == "" { @@ -655,9 +661,17 @@ func augmentWithAutogeneratedPartsWithConfig(collected []nativeCollectedDataset, } } - // Add rows for discovered IDs that don't already exist + // Add rows for discovered IDs that don't already exist. newRows := make([]map[string]any, 0, len(dataset.Rows)) - newRows = append(newRows, 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) + } for rowID := range ids { if row, exists := existingRows[rowID]; exists { diff --git a/internal/topdata/top_package.go b/internal/topdata/top_package.go index d398e0e..f839b8c 100644 --- a/internal/topdata/top_package.go +++ b/internal/topdata/top_package.go @@ -493,6 +493,9 @@ func packagedBuildResult(p *project.Project) (BuildResult, error) { if files2DA == 0 { return BuildResult{}, fmt.Errorf("topdata build output missing: run build-topdata first") } + if err := validateCompiled2DAOutputCatalog(p, output2DA); err != nil { + return BuildResult{}, err + } newestSource, newestPath, err := newestTopDataInput(p, time.Now()) if err != nil { @@ -515,6 +518,74 @@ func packagedBuildResult(p *project.Project) (BuildResult, error) { }, nil } +func validateCompiled2DAOutputCatalog(p *project.Project, output2DA string) error { + expected, err := expectedCompiled2DAOutputs(p) + if err != nil { + return err + } + actual, err := compiled2DAOutputNames(output2DA) + if err != nil { + return err + } + + missing := make([]string, 0) + for outputName := range expected { + if _, ok := actual[outputName]; !ok { + missing = append(missing, outputName) + } + } + stale := make([]string, 0) + for outputName := range actual { + if _, ok := expected[outputName]; !ok { + stale = append(stale, outputName) + } + } + slices.Sort(missing) + slices.Sort(stale) + if len(missing) > 0 { + return fmt.Errorf("topdata build output is missing compiled 2da %s; run build-topdata first", strings.Join(missing, ", ")) + } + if len(stale) > 0 { + return fmt.Errorf("topdata build output contains stale compiled 2da %s; run build-topdata first", strings.Join(stale, ", ")) + } + return nil +} + +func expectedCompiled2DAOutputs(p *project.Project) (map[string]struct{}, error) { + dataDir := filepath.Join(p.TopDataSourceDir(), "data") + datasets, err := discoverNativeDatasets(dataDir) + if err != nil { + return nil, err + } + registryDatasets, err := collectGeneratedRegistryDatasets(dataDir) + if err != nil { + return nil, err + } + outputs := make(map[string]struct{}, len(datasets)+len(registryDatasets)) + for _, dataset := range datasets { + outputs[dataset.OutputName] = struct{}{} + } + for _, dataset := range registryDatasets { + outputs[dataset.Dataset.OutputName] = struct{}{} + } + return outputs, nil +} + +func compiled2DAOutputNames(output2DA string) (map[string]struct{}, error) { + entries, err := os.ReadDir(output2DA) + if err != nil { + return nil, fmt.Errorf("read compiled 2da output dir %s: %w", output2DA, err) + } + outputs := make(map[string]struct{}) + for _, entry := range entries { + if entry.IsDir() || !strings.EqualFold(filepath.Ext(entry.Name()), ".2da") { + continue + } + outputs[entry.Name()] = struct{}{} + } + return outputs, nil +} + func currentCompiledBuildResult(p *project.Project) (BuildResult, bool, error) { nativeResult, err := packagedBuildResult(p) if err == nil { diff --git a/internal/topdata/topdata_test.go b/internal/topdata/topdata_test.go index a16edf6..d235df1 100644 --- a/internal/topdata/topdata_test.go +++ b/internal/topdata/topdata_test.go @@ -10582,6 +10582,37 @@ func TestNormalizePartsRowsACBonusUsesConfiguredPolicyBeforeOverrides(t *testing } } +func TestNormalizePartsRowsACBonusSupportsAscendingRowIDSortKey(t *testing.T) { + collected := []nativeCollectedDataset{ + { + 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": "****"}, + }, + }, + } + cfg := project.PartsRowsConfig{ + ACBonus: project.PartsRowsACBonusConfig{ + Default: project.PartsRowsACBonusPolicy{Strategy: "ascending_row_id_sort_key", Divisor: 100, Format: "%.2f"}, + }, + } + + got, err := normalizePartsRowsACBonus(collected, cfg) + if err != nil { + t.Fatalf("normalizePartsRowsACBonus failed: %v", err) + } + + beltRows := rowsByID(got[0].Rows) + if got := beltRows[1]["ACBONUS"]; got != "0.01" { + t.Fatalf("expected low belt row to get lower sort key 0.01, got %v", got) + } + if got := beltRows[117]["ACBONUS"]; got != "1.17" { + t.Fatalf("expected high belt row to get higher sort key 1.17, got %v", got) + } +} + func rowsByID(rows []map[string]any) map[int]map[string]any { out := map[int]map[string]any{} for _, row := range rows { @@ -11509,6 +11540,51 @@ func TestBuildAndPackageNoOpsWhenOutputsAreCurrent(t *testing.T) { } } +func TestBuildAndPackageRebuildsWhenDatasetWasDeleted(t *testing.T) { + root := topPackageTestProject(t) + mkdirAll(t, filepath.Join(root, "topdata", "data", "skills")) + writeFile(t, filepath.Join(root, "topdata", "data", "skills", "base.json"), `{ + "output": "skills.2da", + "columns": ["Label"], + "rows": [{"id": 0, "Label": "ATHLETICS"}] +}`+"\n") + + proj := testProject(root) + proj.Config.TopData.ReferenceBuilder = "" + proj.Config.Autogen.Consumers = nil + + result, err := BuildAndPackage(proj, nil) + if err != nil { + t.Fatalf("initial BuildAndPackage failed: %v", err) + } + stale2DA := filepath.Join(result.Output2DADir, "repadjust.2da") + if _, err := os.Stat(stale2DA); err != nil { + t.Fatalf("expected initial repadjust output: %v", err) + } + + sourceTime := time.Now().Add(-4 * time.Hour) + outputTime := time.Now().Add(-10 * time.Minute) + if err := os.RemoveAll(filepath.Join(root, "topdata", "data", "repadjust")); err != nil { + t.Fatalf("remove deleted dataset: %v", err) + } + setTopDataSourceTimes(t, root, sourceTime) + setBuildOutputTimes(t, result, outputTime) + + incremental, err := BuildAndPackage(proj, nil) + if err != nil { + t.Fatalf("incremental BuildAndPackage failed: %v", err) + } + if incremental.Mode == "incremental-noop" { + t.Fatalf("expected deleted dataset to force rebuild, got %q", incremental.Mode) + } + if _, err := os.Stat(stale2DA); !os.IsNotExist(err) { + t.Fatalf("expected stale deleted dataset output to be pruned, got %v", err) + } + if _, err := os.Stat(filepath.Join(result.Output2DADir, "skills.2da")); err != nil { + t.Fatalf("expected remaining dataset output: %v", err) + } +} + func TestBuildPackageFailsWhenAutogenManifestCacheIsStale(t *testing.T) { root := topPackageTestProject(t) proj := testProject(root)