diff --git a/.gitignore b/.gitignore index 6e4578c5..db6d581d 100644 --- a/.gitignore +++ b/.gitignore @@ -410,3 +410,7 @@ pump_2.2.0.zip # Wrangler local dev state (Cloudflare Pages/Workers CLI cache, incl. miniflare) .wrangler/ + +# Subagent-driven-development scratch: per-plan ledgers, task briefs, review +# packages. Working state for an in-flight plan, not a record - git history is. +.superpowers/ diff --git a/CLAUDE.md b/CLAUDE.md index 8c956e2e..60c7c297 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -107,7 +107,16 @@ The `transpile-lua` CI job runs that script, fails if the committed `src/lua` no - Avoid C# constructs the existing code avoids in hot paths under Lua settings: `yield return`, LINQ, named tuples, try/catch, and struct dictionary keys have all been removed for Lua performance. LINQ is worse than a perf problem: `CoreSystem.lua` does not load `Collections.Linq`, so `Invoke-LuaBuild.ps1` never copies `Linq.lua` and LINQ transpiles into calls on a module that was never shipped - a runtime failure inside Factorio, not a build error. - Keep control flow deterministic: Factorio modifies `pairs()` and `math.random()` for determinism, so prefer simple, stable iteration and avoid order-dependent assumptions. -- Syntax-check generated Lua with `for f in src/lua/**/*.lua; luac5.2 -p $f; end` (fish). `luac5.2`/`lua5.2` only validate syntax, not Factorio runtime APIs. +### Checking the generated Lua locally + +Run `tools/check-lua.sh`. It does exactly what the `transpile-lua` CI job does, against the same Lua version, in Docker: + +1. syntax-checks every generated file with `luac` 5.2.4 +2. runs `sample.lua`, which is the step that actually matters + +Those two are not redundant. LINQ transpiles cleanly *and* parses cleanly - it emits `local Linq = System.Linq.Enumerable`, which is nil because `Collections.Linq` is not in the CoreSystem load list. It only fails when the module loads (`attempt to index field 'Linq'`), so the check has to run the planner, not just parse it. That takes well under a second. + +Use the script rather than your own `luac`. Homebrew no longer ships Lua 5.2, so a Mac checkout typically has 5.4 or newer, and parsing 5.2-targeted code with a 5.5 parser proves very little. The image (`nickblah/lua:5.2-alpine`, ~11MB) is Lua 5.2.4 - the exact version the performance log above was measured against. Needs Docker (OrbStack or Docker Desktop); regenerate first with `pwsh src/lua/Invoke-LuaBuild.ps1` if you changed the core. ### Factorio reference @@ -117,6 +126,44 @@ The `transpile-lua` CI job runs that script, fails if the committed `src/lua` no - Lua 5.2 manual: - Prefer official Factorio docs over forum/blog/wiki advice when changing runtime behavior. +### The Factorio oracle (re-capture after a game update) + +The planner hardcodes entity names, item names, direction values, and entity sizes. When Factorio changes any of those, nothing here notices - plans keep generating, they are just wrong. Factorio 2.0 renamed `effectivity-module-N` to `efficiency-module-N` and widened directions from 8-way to 16-way, and both went unnoticed for a long time. + +So don't trust memory or the wiki. The game is the only authority on what the game accepts, and it ships four machine-readable sources: + +| Source | Answers | +| --- | --- | +| `factorio --dump-data` | Every prototype: names, collision boxes, pipe connections, pole supply and wire reach, beacon stats | +| `data/*/migrations/*.json` | Every rename, as a table. This is a complete list, not a guess | +| `doc-html/runtime-api.json` | `defines.*` values, version-stamped to the install | +| `data/changelog.txt` | Behavior changes per patch | + +`tools/capture-factorio-oracle.sh` pulls all four into `test/FactorioTools.Test/OilField/factorio-oracle.json`: + +```bash +tools/capture-factorio-oracle.sh # auto-detects a Steam or /Applications install +tools/capture-factorio-oracle.sh --check # report drift, change nothing, exit 1 on mismatch +tools/capture-factorio-oracle.sh --factorio /path/to/factorio.app +``` + +Notes on using it: + +- **The committed fixture targets the experimental branch**, currently 2.1.14, not stable. That is deliberate: the bug reports come from 2.1 and that is where the game is heading. `captureInfo.factorioVersion` in the fixture records which build it came from. +- **Stable and experimental are not identical.** Comparing 2.0.77 stable against 2.1.14 experimental, everything the planner reads is byte-identical except one thing: the pumpjack's output fluid box went from 2 distinct corners to 4, one per rotation (FFF #442). That difference may mean the hardcoded terminal offsets in `Helpers.cs` are wrong for 2.1 - see issue #81. Capture any second version with `--factorio ` and `--out ` to compare. +- **Re-capture after every Factorio update and commit the diff.** A changed fixture is the signal that a hardcoded constant needs review. `--check` answers "has the game moved past what we committed?" without dirtying the tree. +- **The installed binary is the authority** on which version gets captured. Steam updates it without asking, so it decides and everything else follows. Same convention as `scripts/sync-factorio-refs.sh` in FactorioMapWebUI. +- **It runs with user mods disabled** (`--mod-directory` pointed at an empty directory). Mods rewrite prototypes freely, so a capture that loads them describes one person's modded game rather than Factorio. The script prints which mods loaded; expect only `core base elevated-rails quality recycler space-age`. +- **CI never runs the capture** and needs no Factorio install - it reads the committed fixture. That is why the fixture is committed rather than generated on demand. +- Capture needs `python3` (for JSON trimming) and a Factorio install. Neither is needed to build or test. +- `EntityNames.AaiIndustry` names come from a mod, so they are deliberately absent from a vanilla capture. That is expected, not drift. +- Output is deterministic: two captures of the same install are byte-identical. + +Two related sources, for when the game binary is not the easiest thing to reach: + +- **`wube/factorio-data`** (cloned at `~/GitHub/factorio-data`) is the official prototype source, tagged per version. Its `*/migrations/*.json` files are byte-identical to the installed game's, so renames can be checked with no Factorio install at all. Only the resolved geometry from `--dump-data` genuinely needs the binary. +- **A throwaway mod is the way to generate blueprint fixtures.** Docs describe prototypes, not what the blueprint exporter writes. A mod whose `on_init` calls `stack.set_blueprint_entities{...}` then `helpers.write_file(name, stack.export_stack())`, run headless via `factorio --create --mod-directory `, produces a real blueprint string stamped with the real game version. This is how the direction values and the `mirror` field were established rather than assumed. Check runtime API names against `doc-html/runtime-api.json` first - `game.write_file` became `helpers.write_file` in 2.0. + ## Testing notes - Tests use **xUnit v3 + Verify** (`xunit.v3` + `Verify.XunitV3`). Many tests assert against committed `*.verified.txt` snapshots under `test/FactorioTools.Test/OilField`. When behavior legitimately changes, update snapshots via Verify's accept workflow (received vs verified) rather than editing expected files by hand. A stale committed snapshot **does** fail on CI: `AutoVerify` is off there, so the test itself throws `VerifyException` with the diff (confirmed on a real runner, not just inferred - note that setting `CI`/`GITHUB_ACTIONS` locally does *not* reproduce the build-server detection, so local simulation of this is misleading). A "Check no Verify snapshots drifted" step backs that up by failing if `dotnet test` leaves any `*.verified.*` file dirty, in case the detection ever regresses. Commit regenerated snapshots with your change. diff --git a/docs/superpowers/plans/2026-08-16-factorio-21-fixes.md b/docs/superpowers/plans/2026-08-16-factorio-21-fixes.md new file mode 100644 index 00000000..77959db3 --- /dev/null +++ b/docs/superpowers/plans/2026-08-16-factorio-21-fixes.md @@ -0,0 +1,883 @@ +# Factorio 2.1 Compatibility Fixes Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make the oil field planner speak Factorio 2.1 instead of 1.1, fixing mis-rotated pumpjacks and unrecognized renamed items. + +**Architecture:** The internal `Direction` enum stays 1.1-style four-way, because it is the planner's logical concept and is transpiled to Lua. All version knowledge lives at the serialization boundary. A committed oracle fixture, captured from the game, becomes the thing that fails when Factorio moves again. + +**Tech Stack:** C# / .NET 10, xUnit v3 + Verify, Vue 3 + Pinia + Vitest, CSharp.lua transpilation, .NET WASM. + +**Spec:** `docs/superpowers/specs/2026-08-16-factorio-21-oracle-design.md` + +## Global Constraints + +- Use hyphens, never em dashes or en dashes, in all files. +- Prose written for humans targets Flesch-Kincaid grade 12 max, aim 9-11. +- The core library `src/FactorioTools` must stay free of JSON/serialization dependencies. It is transpiled to Lua; `FactorioTools.Serialization` is not. +- In core library hot paths avoid `yield return`, LINQ, named tuples, try/catch, and struct dictionary keys. LINQ in the core is a runtime failure inside Factorio, not a build error, because `Linq.lua` is never shipped. +- Any change to the core requires regenerating `src/lua` via `src/lua/Invoke-LuaBuild.ps1` and committing it, or the `transpile-lua` CI job fails. +- Build and test under both default flags and `/p:UseLuaSettings=true`. +- Verify snapshots auto-accept locally but fail on CI. Commit regenerated snapshots. Setting `CI=1` locally does NOT reproduce CI behavior, so do not try to simulate it. +- Confirmed encodings, do not re-derive: blueprint version is `major<<48 | minor<<32 | patch<<16 | dev`; the 2.0 threshold is `562949953421312`. Blueprint directions are north omitted, east `4`, south `8`, west `12`. A flipped entity carries `"mirror": true`. + +## Corrections to the spec, found while planning + +Three spec statements were wrong. The plan below is correct; fix the spec in Task 1. + +1. **`FormatVersion` and `ParseVersion` already exist** at `GridToBlueprintString.cs:243` and `:233`, and they already match the confirmed layout. Do not write new ones. +2. **Output is already version-stamped.** `GridToBlueprintString.cs:222` emits `FormatVersion(2, 0, 32, 0)`. The spec's claim that output needs to start stamping a version is wrong. +3. **The fixture needs no `.csproj` change.** `small-list.txt` is `CopyToOutputDirectory: Never` and tests resolve paths through `BaseTest.GetRepositoryRoot()`. Follow that pattern. + +The corpus carries `version: 0` because `CleanBlueprint.cs:34` builds a `new Blueprint` without copying `Version`, and normalize serializes that object. + +--- + +### Task 1: Oracle assertion test + +Locks the oracle in before any behavior changes. Pure test addition. It should FAIL on the current code, which is the point: it reproduces the reported bugs as a test. + +**Files:** +- Create: `test/FactorioTools.Test/OilField/FactorioOracleTest.cs` +- Modify: `docs/superpowers/specs/2026-08-16-factorio-21-oracle-design.md` (apply the three corrections above) + +**Interfaces:** +- Consumes: `BaseTest.GetRepositoryRoot()` (public static, returns repo root), the committed fixture `test/FactorioTools.Test/OilField/factorio-oracle.json`. +- Produces: nothing other tasks consume. + +- [ ] **Step 1: Write the failing test** + +Create `test/FactorioTools.Test/OilField/FactorioOracleTest.cs`: + +```csharp +using System.Text.Json; +using Knapcode.FactorioTools.Data; + +namespace Knapcode.FactorioTools.OilField; + +/// +/// Asserts the planner's hardcoded Factorio facts against an oracle captured from the game +/// itself (tools/capture-factorio-oracle.sh). +/// +/// This reads the COMMITTED fixture, never the game, so CI needs no Factorio install. +/// Re-capture after a Factorio update and commit the diff; a changed fixture failing here +/// is the signal that a constant needs review. +/// +public class FactorioOracleTest : BaseTest +{ + private static readonly string FixturePath = Path.Combine( + GetRepositoryRoot(), "test", "FactorioTools.Test", "OilField", "factorio-oracle.json"); + + private const string ReCaptureHint = + "If Factorio changed, re-run tools/capture-factorio-oracle.sh and review the diff."; + + private static JsonElement Oracle() + { + return JsonDocument.Parse(File.ReadAllText(FixturePath)).RootElement; + } + + private static HashSet Names(JsonElement parent, string property) + { + var element = parent.GetProperty(property); + if (element.ValueKind == JsonValueKind.Object) + { + return element.EnumerateObject().Select(x => x.Name).ToHashSet(); + } + + return element.EnumerateArray().Select(x => x.GetString()!).ToHashSet(); + } + + [Fact] + public void EveryVanillaEntityNameExistsInFactorio() + { + var entities = Names(Oracle(), "entities"); + + var missing = typeof(EntityNames.Vanilla) + .GetFields() + .Select(f => (string)f.GetValue(null)!) + .Where(name => !entities.Contains(name)) + .ToList(); + + Assert.True(missing.Count == 0, $"Not in Factorio: {string.Join(", ", missing)}. {ReCaptureHint}"); + } + + /// + /// EntityNames.AaiIndustry is deliberately NOT checked. Those names come from the AAI + /// Industry mod, and the oracle is captured with mods disabled on purpose, so their + /// absence is expected rather than drift. + /// + [Fact] + public void EveryVanillaModuleNameExistsInFactorio() + { + var modules = Names(Oracle(), "modules"); + + var missing = typeof(ItemNames.Vanilla) + .GetFields() + .Select(f => (string)f.GetValue(null)!) + // "blueprint" is an item, not a module, so it is not in the module list. + .Where(name => name != ItemNames.Vanilla.Blueprint) + .Where(name => !modules.Contains(name)) + .ToList(); + + Assert.True(missing.Count == 0, $"Not a Factorio module: {string.Join(", ", missing)}. {ReCaptureHint}"); + } + + [Theory] + [InlineData(Direction.Up, "north")] + [InlineData(Direction.Right, "east")] + [InlineData(Direction.Down, "south")] + [InlineData(Direction.Left, "west")] + public void InternalDirectionDoublesToTheFactorioValue(Direction direction, string factorioName) + { + var expected = Oracle().GetProperty("directions").GetProperty(factorioName).GetInt32(); + + // The internal enum is deliberately 1.1-style four-way (N=0, E=2, S=4, W=6). + // Factorio 2.0 is 16-way, so the blueprint value is always exactly double. + Assert.Equal(expected, (int)direction * 2); + } + + [Theory] + [InlineData(EntityNames.Vanilla.SmallElectricPole, "supply_area_distance", 2.5)] + [InlineData(EntityNames.Vanilla.MediumElectricPole, "supply_area_distance", 3.5)] + [InlineData(EntityNames.Vanilla.BigElectricPole, "supply_area_distance", 2)] + [InlineData(EntityNames.Vanilla.Substation, "supply_area_distance", 9)] + [InlineData(EntityNames.Vanilla.SmallElectricPole, "maximum_wire_distance", 7.5)] + [InlineData(EntityNames.Vanilla.MediumElectricPole, "maximum_wire_distance", 9)] + [InlineData(EntityNames.Vanilla.BigElectricPole, "maximum_wire_distance", 32)] + [InlineData(EntityNames.Vanilla.Substation, "maximum_wire_distance", 18)] + [InlineData(EntityNames.Vanilla.Beacon, "supply_area_distance", 3)] + public void RawGeometryBehindTheOptionsPresetsIsUnchanged(string entity, string field, double expected) + { + var actual = Oracle().GetProperty("entities").GetProperty(entity).GetProperty(field).GetDouble(); + + Assert.True( + expected == actual, + $"{entity}.{field} moved from {expected} to {actual}, so the OilFieldOptions presets need review. {ReCaptureHint}"); + } +} +``` + +- [ ] **Step 2: Run the test to verify it fails for the right reason** + +Run: `dotnet test test/FactorioTools.Test/FactorioTools.Test.csproj --filter "FullyQualifiedName~FactorioOracleTest"` + +Expected: `EveryVanillaModuleNameExistsInFactorio` FAILS with `Not a Factorio module: effectivity-module-3`. The direction and geometry tests PASS. If the geometry tests fail, stop and investigate; the oracle says they should not. + +The failing module test IS the reported bug reproduced. Do not fix it here. + +- [ ] **Step 3: Apply the three spec corrections** + +In `docs/superpowers/specs/2026-08-16-factorio-21-oracle-design.md`: +- In the C3 section, replace the claim that output should "start writing a real version stamp" with: output already stamps `FormatVersion(2, 0, 32, 0)` at `GridToBlueprintString.cs:222`, and `ParseVersion`/`FormatVersion` already exist at `:233` and `:243` matching the confirmed layout. +- In the C2 section, delete the note claiming a `` csproj entry is needed. Replace with: tests resolve the fixture through `BaseTest.GetRepositoryRoot()`, matching `BasePlannerTest.SmallListFilePath`. +- In the C6 section, add: the corpus carries `version: 0` because `CleanBlueprint.cs:34` builds a `new Blueprint` without copying `Version`. + +- [ ] **Step 4: Commit** + +```bash +git add test/FactorioTools.Test/OilField/FactorioOracleTest.cs docs/superpowers/specs/2026-08-16-factorio-21-oracle-design.md +git commit -m "Assert planner constants against the captured Factorio oracle + +Reads the committed fixture, never the game, so CI needs no Factorio install. +EveryVanillaModuleNameExistsInFactorio fails on effectivity-module-3, which is +the reported bug reproduced as a test. Fixed in the next commit. + +Also corrects three spec claims found while planning: FormatVersion/ParseVersion +already exist, output is already version-stamped, and the fixture needs no +csproj entry." +``` + +The suite is RED after this commit, deliberately. If that is not acceptable in this repo, squash Task 1 and Task 2 together. + +--- + +### Task 2: Rename the module in C# + +**Files:** +- Modify: `src/FactorioTools/Data/ItemNames.cs:7` + +**Interfaces:** +- Consumes: `FactorioOracleTest.EveryVanillaModuleNameExistsInFactorio` from Task 1. +- Produces: `ItemNames.Vanilla.EfficiencyModule3 == "efficiency-module-3"`, consumed by Task 3's Vue values. + +- [ ] **Step 1: Make the minimal change** + +In `src/FactorioTools/Data/ItemNames.cs`, change line 7 from: + +```csharp + public const string EfficiencyModule3 = "effectivity-module-3"; +``` + +to: + +```csharp + // Renamed in Factorio 2.0. "effectivity-module-3" no longer exists, so the game + // silently rejects it. See base/migrations/2.0.0.json in the game data. + public const string EfficiencyModule3 = "efficiency-module-3"; +``` + +The C# identifier `EfficiencyModule3` was already correct; only the string value was stale. + +- [ ] **Step 2: Run the oracle test to verify it passes** + +Run: `dotnet test test/FactorioTools.Test/FactorioTools.Test.csproj --filter "FullyQualifiedName~FactorioOracleTest"` + +Expected: all PASS. + +- [ ] **Step 3: Run the full suite and accept any snapshot changes** + +Run: `dotnet test` + +Expected: snapshots that embed the module name change. Review each diff and confirm the only change is `effectivity-module-3` becoming `efficiency-module-3`. `Score.HasExpectedScore.verified.txt` must NOT change, because module names do not affect plan quality. If the score moves, stop and investigate. + +- [ ] **Step 4: Commit** + +```bash +git add -A +git commit -m "Emit efficiency-module-3, the name Factorio has used since 2.0 + +effectivity-module-3 has not existed since 2.0, so the game silently rejected +it. The C# identifier was already EfficiencyModule3; only the string was stale." +``` + +--- + +### Task 3: Rename in the Vue app, including persisted settings + +A rename alone is not enough. `OilFieldStore` persists to `localStorage`, so a user who ever picked an efficiency module has the dead name saved and would keep sending it forever. + +**Files:** +- Modify: `src/vue/src/components/ModuleSelect.vue:12-14` +- Modify: `src/vue/src/stores/OilFieldStore.ts` +- Test: `src/vue/src/stores/persistence.test.ts` + +**Interfaces:** +- Consumes: `ItemNames.Vanilla.EfficiencyModule3` value from Task 2. +- Produces: a `migrateModuleNames(state)` helper exported from `OilFieldStore.ts`, taking and returning the persisted state object. + +- [ ] **Step 1: Write the failing test** + +Add to `src/vue/src/stores/persistence.test.ts`: + +```typescript +import { migrateModuleNames } from "./OilFieldStore" + +describe("migrateModuleNames", () => { + it("rewrites module names Factorio renamed in 2.0", () => { + const migrated = migrateModuleNames({ + pumpjackModule: "effectivity-module-3", + beaconModule: "effectivity-module", + }) + + expect(migrated.pumpjackModule).toBe("efficiency-module-3") + expect(migrated.beaconModule).toBe("efficiency-module") + }) + + it("leaves current names alone", () => { + const migrated = migrateModuleNames({ + pumpjackModule: "productivity-module-3", + beaconModule: "speed-module-3", + }) + + expect(migrated.pumpjackModule).toBe("productivity-module-3") + expect(migrated.beaconModule).toBe("speed-module-3") + }) +}) +``` + +Adjust the property names to match the real store state shape. Read `OilFieldStore.ts` first and use its actual module property names. + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `cd src/vue && npx vitest run src/stores/persistence.test.ts` + +Expected: FAIL with `migrateModuleNames is not a function`. + +- [ ] **Step 3: Implement the migration** + +In `src/vue/src/stores/OilFieldStore.ts`, add and export: + +```typescript +// Factorio 2.0 renamed the efficiency modules. Anyone who picked one before this fix +// has the dead name in localStorage and would keep sending it to the planner forever, +// so rewrite on load rather than only fixing the dropdown. +const RENAMED_MODULES: Record = { + "effectivity-module": "efficiency-module", + "effectivity-module-2": "efficiency-module-2", + "effectivity-module-3": "efficiency-module-3", +} + +export function migrateModuleNames>(state: T): T { + for (const key of Object.keys(state)) { + const value = state[key] + if (typeof value === "string" && value in RENAMED_MODULES) { + ;(state as Record)[key] = RENAMED_MODULES[value] + } + } + return state +} +``` + +Then call it from the store's persisted-state restore path. `pinia-plugin-persistedstate` exposes `afterHydrate`, so in the `persist` options object add: + +```typescript + afterHydrate: (ctx) => { + migrateModuleNames(ctx.store.$state as Record) + }, +``` + +Read the existing `persist: { ... }` block first and add the hook alongside what is already there. + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `cd src/vue && npx vitest run src/stores/persistence.test.ts` + +Expected: PASS. + +- [ ] **Step 5: Update the dropdown values** + +In `src/vue/src/components/ModuleSelect.vue`, change lines 12-14 from `value="effectivity-module"`, `value="effectivity-module-2"`, `value="effectivity-module-3"` to `value="efficiency-module"`, `value="efficiency-module-2"`, `value="efficiency-module-3"`. The visible labels already read "Efficiency module" and do not change. + +- [ ] **Step 6: Run the whole front-end suite** + +Run: `cd src/vue && npx vitest run && npm run type-check` + +Expected: PASS. + +- [ ] **Step 7: Commit** + +```bash +git add -A +git commit -m "Rename efficiency modules in the Vue app and migrate saved settings + +Changing the dropdown alone would leave every existing user sending +effectivity-module-3, which Factorio has not accepted since 2.0, because the +choice is persisted in localStorage. migrateModuleNames rewrites it on hydrate." +``` + +--- + +### Task 4: Preserve the mirror flag + +Factorio 2.1.7 added pumpjack flipping. Confirmed by round-tripping a blueprint through 2.1.14: a flipped entity carries `"mirror": true`. + +**Files:** +- Modify: `src/FactorioTools/Data/Entity.cs` +- Test: `test/FactorioTools.Test/OilField/ParseBlueprintTest.cs` (create) + +**Interfaces:** +- Consumes: nothing from earlier tasks. +- Produces: `Entity.Mirror` as `bool?`, read by nothing else. Task 6's regression fixtures exercise it. + +- [ ] **Step 1: Write the failing test** + +Create `test/FactorioTools.Test/OilField/ParseBlueprintTest.cs`: + +```csharp +namespace Knapcode.FactorioTools.OilField; + +public class ParseBlueprintTest : BaseTest +{ + /// + /// Generated by a probe mod running inside Factorio 2.1.14: three pumpjacks, the + /// second and third mirrored. See CLAUDE.md for the probe-mod technique. + /// + private const string MirroredPumpjacks = "0eNqN0MkKgzAQBuB3mfNQNC5UX6WU4jKUaU0MSSwVybs36qFQK3gaZvm/w0xQdwNpw8pBOQE3vbJQXiawfFdVN89UJQlK0IPUj6p5gkdg1dIbythfEUg5dkxramnGmxpkTSYc4CaNoHsbAr2a7YBEpwxhXGqQWzbUrNvU4wYUB8B4X0SQbEwfIGcG+uMnB3yx659//PAddiSD9n0ywouMXc6zXBRZKtIiifI4Trz/ADGoghU="; + + [Fact] + public void KeepsTheMirrorFlagOnAFlippedEntity() + { + var blueprint = ParseBlueprint.Execute(MirroredPumpjacks); + + Assert.Collection( + blueprint.Entities, + e => Assert.Null(e.Mirror), + e => Assert.True(e.Mirror), + e => Assert.True(e.Mirror)); + } +} +``` + +The string above is real output from Factorio 2.1.14, already generated, so use it as written. It decodes to version `562954249306113` and: + +``` +{entity_number: 1, direction: 4} +{entity_number: 2, direction: 4, mirror: True} +{entity_number: 3, direction: 8, mirror: True} +``` + +For reference, it came from the probe-mod technique in CLAUDE.md with these entities. Only regenerate if you need to change what is being tested: + +```lua +stack.set_blueprint_entities{ + {entity_number = 1, name = "pumpjack", position = {x = 0.5, y = 0.5}, direction = defines.direction.east}, + {entity_number = 2, name = "pumpjack", position = {x = 10.5, y = 0.5}, direction = defines.direction.east, mirror = true}, + {entity_number = 3, name = "pumpjack", position = {x = 20.5, y = 0.5}, direction = defines.direction.south, mirror = true}, +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `dotnet test test/FactorioTools.Test/FactorioTools.Test.csproj --filter "FullyQualifiedName~ParseBlueprintTest"` + +Expected: FAIL to compile, `Entity` has no `Mirror`. + +- [ ] **Step 3: Add the property** + +In `src/FactorioTools/Data/Entity.cs`, after the `Direction` property: + +```csharp + // Factorio 2.1.7 added pumpjack and burner mining drill flipping, and a flipped entity + // carries "mirror": true (confirmed by round-tripping a blueprint through 2.1.14). + // Parsed so the flag is not silently lost. The planner re-chooses every pumpjack + // orientation itself, so nothing reads this and it is never emitted. + [JsonPropertyName("mirror")] + public bool? Mirror { get; set; } +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `dotnet test test/FactorioTools.Test/FactorioTools.Test.csproj --filter "FullyQualifiedName~ParseBlueprintTest"` + +Expected: PASS. + +- [ ] **Step 5: Run the full suite** + +Run: `dotnet test` + +Expected: PASS with no snapshot changes. `Mirror` is never emitted, so nothing serialized should move. + +- [ ] **Step 6: Commit** + +```bash +git add -A +git commit -m "Parse the mirror flag instead of dropping it + +Factorio 2.1.7 added pumpjack flipping, and a flipped entity carries +mirror: true. Confirmed by round-tripping a blueprint through 2.1.14, since +FFF #442 describes the feature but not its blueprint representation. + +Parsed only. The planner re-chooses every pumpjack orientation itself, so +honoring a flip is a separate question and is out of scope." +``` + +--- + +### Task 5: Version-gated direction parsing, and the corpus, together + +**This task is atomic and cannot be split.** Fixing the parser alone misreads the 1.1-encoded corpus. Re-normalizing the corpus alone produces 2.x values the unfixed parser misreads. Either half on its own turns the suite red, so they land in one commit. + +**Files:** +- Modify: `src/FactorioTools.Serialization/OilField/Steps/ParseBlueprint.cs` +- Modify: `src/FactorioTools.Serialization/OilField/Steps/GridToBlueprintString.cs` (`SerializeBlueprint`) +- Modify: `test/FactorioTools.Test/OilField/small-list.txt` (regenerated) +- Modify: `test/FactorioTools.Test/OilField/big-list.txt` (regenerated) +- Test: `test/FactorioTools.Test/OilField/ParseBlueprintTest.cs` (extend) + +**Do NOT change `CleanBlueprint.cs`.** Carrying the source version through looks right and is wrong: `ToOutputDirection` always emits 2.x values, so a 1.1 input would come out with 2.x directions under a 1.1 stamp, and re-parsing would halve values that were already converted. Stamping belongs next to the code that does the doubling, which is `SerializeBlueprint`. That also keeps the core library unchanged here, which is what the design wants. + +**Interfaces:** +- Consumes: `Entity.Mirror` from Task 4; `GridToBlueprintString.FormatVersion(ushort, ushort, ushort, ushort)` and `ParseVersion(ulong)` which already exist. +- Produces: `ParseBlueprint.ToInternalDirection(Direction raw, ulong version)` returning `Direction`. + +- [ ] **Step 1: Write the failing tests** + +Add to `test/FactorioTools.Test/OilField/ParseBlueprintTest.cs`: + +```csharp + private const ulong Version2_1_14 = 562954249306113UL; // confirmed against a real export + private const ulong Version1_1_0 = 281479271677952UL; // FormatVersion(1, 1, 0, 0) + + [Theory] + // Factorio 2.x blueprints: north omitted, east 4, south 8, west 12. + [InlineData(0, Version2_1_14, Direction.Up)] + [InlineData(4, Version2_1_14, Direction.Right)] + [InlineData(8, Version2_1_14, Direction.Down)] + [InlineData(12, Version2_1_14, Direction.Left)] + // An unstamped blueprint is assumed to be modern, because that is what users paste. + [InlineData(4, 0UL, Direction.Right)] + // Genuine 1.1 blueprints still parse, when they say so. + [InlineData(2, Version1_1_0, Direction.Right)] + [InlineData(6, Version1_1_0, Direction.Left)] + public void ConvertsBlueprintDirectionsToInternalOnes(int raw, ulong version, Direction expected) + { + Assert.Equal(expected, ParseBlueprint.ToInternalDirection((Direction)raw, version)); + } + + [Theory] + [InlineData(2)] // northeast in 2.x, not a cardinal + [InlineData(6)] // southeast in 2.x, not a cardinal + public void RejectsDirectionsAPumpjackCannotHave(int raw) + { + var ex = Assert.Throws( + () => ParseBlueprint.ToInternalDirection((Direction)raw, Version2_1_14)); + + Assert.Contains(raw.ToString(), ex.Message); + } +``` + +- [ ] **Step 2: Run to verify it fails** + +Run: `dotnet test test/FactorioTools.Test/FactorioTools.Test.csproj --filter "FullyQualifiedName~ParseBlueprintTest"` + +Expected: FAIL to compile, no `ToInternalDirection`. + +- [ ] **Step 3: Implement the conversion** + +In `src/FactorioTools.Serialization/OilField/Steps/ParseBlueprint.cs`, add to the class: + +```csharp + /// + /// The blueprint version at which Factorio widened directions from 8-way to 16-way. + /// GridToBlueprintString.FormatVersion(2, 0, 0, 0). Confirmed against a real 2.1.14 + /// export, whose version is 562954249306113 (2.1.14.1). + /// + private const ulong FirstSixteenWayVersion = 562949953421312UL; + + /// + /// Converts a blueprint's direction to the internal 1.1-style four-way + /// (N=0, E=2, S=4, W=6). + /// + /// Factorio 2.0 widened directions to 16-way (N=0, E=4, S=8, W=12). The old values are + /// still legal, so a 2.x east read as 1.1 is not an error, it is a silently rotated + /// pumpjack. Sniffing the values cannot resolve it either: a blueprint whose directions + /// are all in {0, 4} is valid under both readings with different meanings. The version + /// is the only sound signal. + /// + /// A missing or zero version is treated as modern, because that is what users paste + /// today. The trade-off is spelled out in the design doc. + /// + public static Direction ToInternalDirection(Direction direction, ulong version) + { + var raw = (int)direction; + var internalValue = version == 0 || version >= FirstSixteenWayVersion ? raw / 2 : raw; + + if (internalValue != (int)Direction.Up + && internalValue != (int)Direction.Right + && internalValue != (int)Direction.Down + && internalValue != (int)Direction.Left) + { + throw new FactorioToolsException( + $"Blueprint direction {raw} is not one of the four directions a pumpjack can face.", + badInput: true); + } + + return (Direction)internalValue; + } +``` + +- [ ] **Step 4: Run the unit tests to verify they pass** + +Run: `dotnet test test/FactorioTools.Test/FactorioTools.Test.csproj --filter "FullyQualifiedName~ParseBlueprintTest"` + +Expected: PASS. + +- [ ] **Step 5: Apply the conversion when parsing, and carry the version through** + +In `ParseBlueprint.Execute`, just before `return root.Blueprint;`, add: + +```csharp + for (var i = 0; i < root.Blueprint.Entities.Length; i++) + { + var entity = root.Blueprint.Entities[i]; + if (entity.Direction.HasValue) + { + entity.Direction = ToInternalDirection(entity.Direction.Value, root.Blueprint.Version); + } + } +``` + +Then, in `src/FactorioTools.Serialization/OilField/Steps/GridToBlueprintString.cs`, move the direction-doubling loop into `SerializeBlueprint` itself and stamp the version there, rather than only adding a version stamp: + +```csharp + // Every blueprint string this class produces goes through here, so this is the one + // place that converts internal directions to Factorio 2.0's 16-way values and the one + // place that stamps the version saying so. Keeping them together means they can never + // disagree. Before, only the planner path converted, so the normalize path (see + // NormalizeBlueprints and PlanOrchestrator, which serialize a CleanBlueprint result) + // emitted internal 1.1-style directions under version 0. + blueprint.Version = OutputVersion; + for (var i = 0; i < blueprint.Entities.Length; i++) + { + var entity = blueprint.Entities[i]; + if (entity.Direction.HasValue) + { + entity.Direction = ToOutputDirection(entity.Direction.Value); + } + } +``` + +Moving the doubling here, not just the stamp, is what actually fixes D5 (see the spec): the normalize path called `SerializeBlueprint` directly and never went through the planner's own doubling step, so it had been emitting un-doubled 1.1-style direction values under `version: 0` the whole time. `CleanBlueprint.cs` is deliberately left alone; see the note under Files. + +- [ ] **Step 6: Re-normalize both corpus files** + +The sequence matters. Directions are converted on parse now, and multiplied by 2 on serialize, so a re-normalize rewrites 1.1 values into 2.x values. But the corpus has `version: 0`, which the new parser treats as modern, so a straight re-normalize would halve values that are already 1.1. + +So stamp first, in one throwaway pass. Run from the repo root: + +```bash +python3 - <<'PY' +import base64, zlib, json, pathlib +# FormatVersion(1, 1, 0, 0) - say out loud what the corpus already is, so the parser +# stops guessing. After the re-normalize below they come back stamped 2.x. +V = (1 << 48) | (1 << 32) +for name in ["small-list.txt", "big-list.txt"]: + p = pathlib.Path("test/FactorioTools.Test/OilField") / name + out = [] + for line in p.read_text().splitlines(): + s = line.strip() + if not s or s.startswith("#"): + out.append(line) + continue + j = json.loads(zlib.decompress(base64.b64decode(s[1:]))) + j["blueprint"]["version"] = V + raw = json.dumps(j, separators=(",", ":")).encode() + out.append("0" + base64.b64encode(zlib.compress(raw, 9)).decode()) + p.write_text("\n".join(out) + "\n") + print(f"stamped {name}") +PY +``` + +Then re-normalize through the CLI, which reparses (now correctly, as 1.1) and re-emits as 2.x: + +```bash +dotnet run --project src/FactorioTools.Cli -- oil-field normalize +``` + +- [ ] **Step 7: Verify the corpus actually moved to 2.x** + +```bash +python3 - <<'PY' +import base64, zlib, json +for name in ["small-list.txt", "big-list.txt"]: + path = f"test/FactorioTools.Test/OilField/{name}" + dirs, vers = {}, set() + for line in open(path): + s = line.strip() + if not s or s.startswith("#"): + continue + bp = json.loads(zlib.decompress(base64.b64decode(s[1:])))["blueprint"] + vers.add(bp.get("version")) + for e in bp.get("entities", []): + if e.get("name") == "pumpjack": + dirs[e.get("direction")] = dirs.get(e.get("direction"), 0) + 1 + print(name, "directions:", dict(sorted(dirs.items(), key=lambda x: (x[0] is not None, x[0])))) + print(name, "versions:", vers) +PY +``` + +Expected: directions are now a subset of `{None, 4, 8, 12}` with NO `2` and NO `6`. Versions are all `562949953421312` or higher, none `0`. If any `2` or `6` survives, stop; the conversion is not symmetric. + +- [ ] **Step 8: Run the full suite and check the invariant** + +Run: `dotnet test` + +Expected: PASS, and **`Score.HasExpectedScore.verified.txt` unchanged**. Re-normalizing and fixing the parser are inverse operations, so they must cancel. Verify with: + +```bash +git diff --stat test/FactorioTools.Test/OilField/Score.HasExpectedScore.verified.txt +``` + +Expected: no output. If the scoreboard moved, the change is WRONG. Do not accept the diff. Investigate whether the conversion is asymmetric or the corpus rewrite altered something beyond direction. + +- [ ] **Step 9: Commit** + +```bash +git add -A +git commit -m "Read blueprint directions using the blueprint's own version + +Output already multiplied directions by 2 for Factorio 2.0's 16-way encoding, +but input did nothing, so a 2.x east pumpjack (4) parsed as Direction.Down. +That is the reported mis-rotation. + +Sniffing the values cannot fix it: a blueprint whose directions are all in +{0, 4} is valid under both encodings with different meanings. Version is the +only sound signal, so ToInternalDirection gates on it, treating an unstamped +blueprint as modern because that is what users paste. + +The corpus was 1.1-encoded, which is exactly why no test caught this. Both +lists are re-normalized to 2.x. SerializeBlueprint now stamps the version next +to the code that doubles the directions, so the two can never disagree; the +normalize path previously emitted 2.x directions under version 0. + +Score.HasExpectedScore.verified.txt is unchanged, as it must be: the corpus +rewrite and the parser fix are inverse operations." +``` + +--- + +### Task 6: Regression test for the reported bug + +Task 5 proves the conversion is self-consistent. This proves it against a blueprint the game actually produced. + +**Files:** +- Modify: `test/FactorioTools.Test/OilField/ParseBlueprintTest.cs` + +**Interfaces:** +- Consumes: `ParseBlueprint.ToInternalDirection` from Task 5, `Entity.Mirror` from Task 4. +- Produces: nothing. + +- [ ] **Step 1: Confirm the fixture** + +The string in Step 2 is real output from Factorio 2.1.14, already generated, so use it as written. It decodes to version `562954249306113` and: + +``` +{entity_number: 1} <- north, field omitted entirely +{entity_number: 2, direction: 4} <- east +{entity_number: 3, direction: 8} <- south +{entity_number: 4, direction: 12} <- west +``` + +For reference, it came from the probe-mod technique in CLAUDE.md with these entities. Only regenerate if you need to change what is being tested: + +```lua +stack.set_blueprint_entities{ + {entity_number = 1, name = "pumpjack", position = {x = 0.5, y = 0.5}, direction = defines.direction.north}, + {entity_number = 2, name = "pumpjack", position = {x = 10.5, y = 0.5}, direction = defines.direction.east}, + {entity_number = 3, name = "pumpjack", position = {x = 20.5, y = 0.5}, direction = defines.direction.south}, + {entity_number = 4, name = "pumpjack", position = {x = 30.5, y = 0.5}, direction = defines.direction.west}, +} +``` + +- [ ] **Step 2: Write the test** + +```csharp + /// + /// Exported from Factorio 2.1.14: four pumpjacks facing north, east, south, west. + /// This is the case the corpus cannot cover, because the corpus is 1.1-encoded. + /// It is the test that would have caught the original bug report. + /// + private const string FourCardinalPumpjacks = "0eNqN0csKwyAQBdB/mbWU+Eho/JVSSh5Dsa1G1JSG4L/XJIsWkkBWg3rvWTgj1K8erVMmgBxBNZ3xIC8jeHU31Wu6M5VGkGB7bR9V84RIQJkWPyBpvBJAE1RQuLTmw3Azva7RpQBZtQnYzqdCZyY7IdkpJzDMM0ayItgBgv4bBFrlsFmexYbID4hsVzxviOKAyHdFyqZvVAF1An7bIPBG5+dEXrAyF0yUPCso5TF+Abr9jgg="; + + [Fact] + public void ReadsEveryCardinalFromARealFactorio21Blueprint() + { + var blueprint = ParseBlueprint.Execute(FourCardinalPumpjacks); + + Assert.Collection( + blueprint.Entities, + e => Assert.Equal(Direction.Up, e.Direction ?? Direction.Up), + e => Assert.Equal(Direction.Right, e.Direction), + e => Assert.Equal(Direction.Down, e.Direction), + e => Assert.Equal(Direction.Left, e.Direction)); + } +``` + +- [ ] **Step 3: Run to verify it passes** + +Run: `dotnet test test/FactorioTools.Test/FactorioTools.Test.csproj --filter "FullyQualifiedName~ParseBlueprintTest"` + +Expected: PASS. If east reads as `Direction.Down`, Task 5 did not take effect. + +- [ ] **Step 4: Commit** + +```bash +git add -A +git commit -m "Pin the reported bug with a blueprint the game actually produced + +The corpus is 1.1-encoded, so it cannot cover a real 2.x blueprint. This +fixture came out of Factorio 2.1.14 with all four cardinals, which is the case +that was silently rotating." +``` + +--- + +### Task 7: Regenerate Lua, rebuild WASM, verify the build matrix + +The core changed (`Entity.cs` in Task 4, `ItemNames.cs` in Task 2), so generated artifacts must follow or CI fails. `CleanBlueprint.cs` is deliberately untouched, so the core diff is small. + +**Files:** +- Modify: `src/lua/**` (regenerated) +- Modify: `src/vue/public/framework/**` (regenerated, gitignored) + +**Interfaces:** +- Consumes: every earlier task. +- Produces: nothing. + +- [ ] **Step 1: Regenerate the Lua** + +Run: `pwsh src/lua/Invoke-LuaBuild.ps1` + +- [ ] **Step 2: Syntax-check the generated Lua** + +Run (bash): `find src/lua -name '*.lua' -exec luac5.2 -p {} \;` + +Expected: no output. Any output is a syntax error. + +- [ ] **Step 3: Build and test under Lua settings** + +Run: `dotnet build /p:UseLuaSettings=true && dotnet test /p:UseLuaSettings=true` + +Expected: PASS. This catches core changes that break the Lua-safe configuration. + +- [ ] **Step 4: Rebuild the WASM bundle** + +Run: `cd src/vue && npm run build-wasm` + +Then confirm the bundle landed in the right shape, not flattened: + +```bash +test -f src/vue/public/framework/dotnet.js && echo "bundle shape ok" || echo "WRONG: dotnet.js is not in public/framework" +``` + +- [ ] **Step 5: Verify the front end still plans in a real browser** + +Run: `cd src/vue && npm run build && npm run preview` + +`npm run dev` cannot run the WASM planner; use build plus preview. Load the page, paste the four-cardinal blueprint from Task 6, and confirm the plan comes back with pumpjacks facing the right way. + +- [ ] **Step 6: Commit** + +```bash +git add -A +git commit -m "Regenerate the Lua for the 2.1 compatibility fixes + +The core changed (Entity, ItemNames), so the committed Lua has to follow or +transpile-lua fails. Syntax-checked with luac5.2 and tested under +UseLuaSettings=true." +``` + +- [ ] **Step 7: Open the pull request** + +```bash +git push -u origin fix/factorio-21-oracle +gh pr create --title "Speak Factorio 2.1 instead of 1.1" --body "$(cat <<'EOF' +Fixes mis-rotated pumpjacks and unrecognized renamed items reported since Factorio 2.1. + +Both symptoms are one root cause: the planner still spoke Factorio 1.1. + +## What was wrong + +- `effectivity-module-3` has not existed since 2.0, so the game silently rejected it +- directions were multiplied by 2 on output but never divided on input, so a 2.x east pumpjack (4) parsed as south +- the `mirror` flag added for pumpjack flipping in 2.1.7 was dropped on parse +- the test corpus is 1.1-encoded, which is why none of this ever failed a test + +## How it was found + +Rather than trusting the wiki, `tools/capture-factorio-oracle.sh` pulls prototype facts out of the game and commits them as a fixture. `FactorioOracleTest` asserts the planner's constants against it on every build, with no Factorio install needed on CI. Re-capture after a game update and a changed fixture fails the test with a diff. + +The version, direction and mirror encodings were confirmed by round-tripping blueprints through Factorio 2.1.14, and cross-checked against `wube/factorio-data` at tag 2.1.14. + +## The safety check + +Re-normalizing the corpus and fixing the parser are inverse operations, so `Score.HasExpectedScore.verified.txt` is **unchanged**. A moved scoreboard would have meant the change was wrong. + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +EOF +)" +``` + +--- + +## Self-Review + +**Spec coverage.** C1 landed already. C2 is Task 1. C3 is Task 5. C4 is Tasks 2 and 3. C5 is Task 4. C6 is Task 5 step 6. The invariant is Task 5 step 8. Lua and WASM are Task 7. The manual-drift-check decision needs no task. Out-of-scope items correctly have no task. + +**Placeholder scan.** None. The two blueprint fixtures in Tasks 4 and 6 were generated from Factorio 2.1.14 and are embedded as literal strings, verified to decode to exactly what their tests assert. Every task is mechanical. + +**Type consistency.** `ToInternalDirection(Direction, ulong)` is defined in Task 5 step 3 and used with that signature in Task 5 step 1 and Task 6. `Entity.Mirror` is `bool?` in Task 4 and asserted with `Assert.True(e.Mirror)` and `Assert.Null(e.Mirror)`, both valid for `bool?`. `migrateModuleNames` is generic over `T extends Record` in Task 3 and called both ways consistently. + +**Caught in self-review, already fixed above.** Task 5 originally had `CleanBlueprint` carry the source version through. That is wrong: `ToOutputDirection` always emits 2.x values, so a 1.1 input would come out with 2.x directions under a 1.1 stamp, and reparsing would halve values that were already converted. Stamping now happens in `SerializeBlueprint`, next to the doubling, so the two cannot disagree. + +**Known soft spots for the executor.** Task 3 guesses the store's module property names; read `OilFieldStore.ts` and use the real ones. Task 3 also assumes `pinia-plugin-persistedstate` exposes `afterHydrate`; confirm against the installed version and use its documented hook if the name differs. Task 5 step 6's stamping pass rewrites the corpus with Python before the CLI re-normalize; run it on a clean tree so `git checkout` can undo it if the sequence needs a retry. diff --git a/docs/superpowers/specs/2026-08-16-factorio-21-oracle-design.md b/docs/superpowers/specs/2026-08-16-factorio-21-oracle-design.md new file mode 100644 index 00000000..bded0d11 --- /dev/null +++ b/docs/superpowers/specs/2026-08-16-factorio-21-oracle-design.md @@ -0,0 +1,208 @@ +# Speaking Factorio 2.1 to a 2.1 game + +Design for the bugs reported since Factorio 2.1: mis-rotated pumpjacks, and items the game does not recognize. + +## Problem + +Both symptoms have one root cause. The planner still speaks Factorio 1.1. + +Two things changed in Factorio 2.0 that this repo never followed: + +1. `effectivity-module-N` was renamed to `efficiency-module-N`. +2. Directions widened from 8 values to 16. North stayed 0, but east moved from 2 to 4, south from 4 to 8, and west from 6 to 12. + +The second one is the nastier of the pair, because the old values are still *legal*. Emitting `4` for south does not fail - the game reads it as east and rotates the pumpjack. There is no error anywhere, just a wrong blueprint. + +### Why the tests did not catch it + +The committed corpus is 1.1-encoded. Decoding every blueprint in `small-list.txt` and `big-list.txt` gives pumpjack direction values of `{0, 2, 4, 6}`, and both `2` and `6` are impossible for a 2.x pumpjack - they would mean northeast and southeast. Every blueprint also carries `version: 0`. + +So the parser is *correct for the corpus* and wrong for anything a user pastes. The tests and the bug report cannot both be satisfied by the current code, and the tests won. + +### Why an oracle, rather than reading the wiki + +This class of bug is invisible from inside the repo, so the fix has to include a way to notice it next time. Commit `bfef7ba` added that: `tools/capture-factorio-oracle.sh` pulls prototype facts out of the game itself into `test/FactorioTools.Test/OilField/factorio-oracle.json`. + +Pointing that fixture at the current constants reports: + +``` +ItemNames.EfficiencyModule3 "effectivity-module-3" -> efficiency-module-3 +ModuleSelect.vue effectivity-module{,-2,-3} -> efficiency-module* +Direction.Right = 2 2 now means northeast; east is 4 +Direction.Down = 4 4 now means east; south is 8 +Direction.Left = 6 6 now means southeast; west is 12 +``` + +#### Cross-checked against wube's own repo + +The capture was validated against `wube/factorio-data` checked out at tag `2.1.14` (locally at `~/GitHub/factorio-data`). The `base/migrations` directories are byte-identical to the installed game's, and `base/prototypes/item.lua:2615` defines `efficiency-module` directly. Two independent sources, same answer. + +Worth noting so nobody re-raises it: `effectivity` still appears 14 times in 2.1.14 prototypes, but only ever as a **property** name (`distribution_effectivity`, and `effectivity` on vehicles). It is never an item name. The rename touched item, recipe and technology names only, exactly as the migration table says. + +`factorio-data` is also the no-install path to part of the oracle. Migrations and prototype source need no Factorio binary, so only the resolved geometry from `--dump-data` truly requires the game. + +#### Generating blueprints as test fixtures + +Two of the confirmations above came from a throwaway Factorio mod rather than from docs, and the technique is worth recording because it is the only way to get authoritative blueprint fixtures. + +A mod whose `on_init` calls `stack.set_blueprint_entities{...}` and `helpers.write_file(name, stack.export_stack())`, run headless via `factorio --create --mod-directory `, exports whatever blueprint you ask for, stamped with the real game version. That is how the direction table and the `mirror` field above were established. + +(`game.write_file` moved to `helpers.write_file` in 2.0 - a fitting instance of the same problem this document is about. Check names against `doc-html/runtime-api.json` rather than memory.) + +The oracle also **ruled things out**, which narrows the work considerably. Every pole and beacon number still matches the game: supply distances 2.5/3.5/2/9, wire reach 7.5/9/32/18, beacon supply 3 and distribution effectivity 1.5. `PlanUndergroundPipes.MaxUnderground = 11` still agrees with the game's `max_underground_distance: 10` (which counts the gap, not the ends). None of the geometry drifted. + +## Defects in scope + +**D1 - Renamed module.** `src/FactorioTools/Data/ItemNames.cs:7` emits `effectivity-module-3`. `src/vue/src/components/ModuleSelect.vue:12-14` offers all three stale names. + +**D2 - Direction is converted on output but not on input.** `GridToBlueprintString.cs:38` multiplies by 2, which is right. `ParseBlueprint.cs` does nothing, so `InitializeContext.cs:250` reads a raw 2.x value into the 1.1-style enum. An east pumpjack (4) is read as `Direction.Down`; south (8) and west (12) are not valid enum members at all. + +**D3 - Pumpjack flips are dropped.** Changelog 2.1.7 and [FFF #442](https://factorio.com/blog/post/fff-442) added pumpjack flipping. The FFF describes the feature but says nothing about how a flip is represented in a blueprint, so it was tested rather than assumed: a blueprint round-tripped through 2.1.14 writes `"mirror": true` alongside `direction`. `src/FactorioTools/Data/Entity.cs` has no `mirror` property, so the flag vanishes on parse. + +**D4 - The corpus encodes 1.1 directions.** Left alone, it would keep hiding D2. + +**D5 - The normalize path never doubled directions at all.** `ToOutputDirection` (the multiply-by-2 step) lived only in `GridToBlueprintString.Execute`, the planner's own serialization path. `PlanOrchestrator.Normalize` and `NormalizeBlueprints` call `SerializeBlueprint` directly, skipping `Execute` entirely, so the normalize path was emitting internal 1.1-style direction values (`0, 2, 4, 6`) unconverted, and stamping them with `version: 0` on top - a blueprint that looks unversioned-1.1 but is not one, silently wrong for any consumer that trusts the version field. This is a real, user-visible bug: anyone who ran `oil-field normalize` (or the API's `normalize` route) got back a blueprint Factorio would misread. It was found and fixed alongside D2/C3, by moving the direction-doubling loop out of `GridToBlueprintString.Execute` and into `SerializeBlueprint` itself, so every caller - the planner's own path and normalize alike - goes through the same conversion and the same version stamp. It was not called out as its own defect until this correction, because it surfaced while fixing D2 rather than from a separate bug report. + +## Design + +### C1 - Oracle capture (landed in `bfef7ba`) + +Already done. Not repeated here. + +### C2 - Oracle assertion test + +`FactorioOracleTest` reads the **committed fixture**, never the game, so CI needs no Factorio install. It asserts: + +- every `EntityNames.Vanilla` value exists in `entities` +- every `ItemNames.Vanilla` module value exists in `modules` +- the `Direction` enum members match `directions` for north/east/south/west +- the pole and beacon raw values behind `OilFieldOptions` presets are unchanged + +Two carve-outs, both deliberate and both needing a comment saying why: + +- `EntityNames.AaiIndustry` is a mod entity. A vanilla capture will never contain it. Absence is expected, not drift. +- `ItemNames.Vanilla.Blueprint` is an item, not a module or entity, so it is checked against a different part of the fixture or excluded outright. + +This mirrors `PlannerDefaultsTest`, which is already the repo's "the test is the generator" pattern. When Factorio 2.2 lands, re-capture, and a changed fixture fails here with a diff. + +The failure message must name `tools/capture-factorio-oracle.sh`, so a red CI run says how to fix itself. + +Note: tests resolve the fixture through `BaseTest.GetRepositoryRoot()`, matching `BasePlannerTest.SmallListFilePath`, so no csproj entry is needed. + +### C3 - Direction conversion + +**The internal `Direction` enum does not change.** It stays `Up=0, Right=2, Down=4, Left=6`. It is the planner's logical four-way concept, it is transpiled to Lua, and renumbering it would churn the whole core, every snapshot, and the Lua output for no benefit. All version knowledge lives at the serialization boundary. + +Output keeps `ToOutputDirection` (multiply by 2). Output already stamps a real version: `GridToBlueprintString.cs:222` sets `Version = FormatVersion(2, 0, 32, 0)`, and `ParseVersion`/`FormatVersion` already exist at `:233` and `:243`, matching the confirmed layout below. + +Input gains the inverse, in `FactorioTools.Serialization`: + +``` +ToInternalDirection(raw, version): + 16-way (version >= 2.0, or version missing/0) -> raw / 2 + 8-way (version below 2.0) -> raw + result not in {0, 2, 4, 6} -> throw, naming the entity and the raw value +``` + +Missing or zero defaults to 16-way, because that is what every user pastes today. That choice is what forces D4: the existing corpus has `version: 0` and 1.1 values, so it must be re-normalized in the same change or it will be read wrong. + +**Value-sniffing was considered and rejected.** A blueprint whose directions are all in `{0, 4}` is valid under both encodings with different meanings, so inference cannot always be correct. The version field is the only sound signal. + +#### Version encoding (confirmed) + +`Blueprint.Version` is a `ulong` this repo has never read. The layout is confirmed against a real blueprint exported from Factorio 2.1.14: + +``` +raw 562954249306113 +hex 0x0002_0001_000e_0001 + major=2 minor=1 patch=14 dev=1 -> 2.1.14.1 +``` + +So the layout is `major<<48 | minor<<32 | patch<<16 | dev`, and the 2.0 threshold is `2<<48 = 562949953421312`. Only an ordered comparison is needed, and the major version dominates it. + +#### Direction encoding in blueprints (confirmed) + +Confirmed by round-tripping a blueprint through the game rather than reading docs. Asking for each cardinal and reading back what the exporter wrote: + +| Requested | Field written | +| --- | --- | +| north | *omitted entirely* | +| east | `4` | +| south | `8` | +| west | `12` | + +North being omitted rather than written as `0` matters: `Entity.Direction` is already nullable and `InitializeContext.cs:250` already defaults it to `Up`, so that path is correct today and must stay correct. + +### C4 - Rename, including persisted settings + +`ItemNames.Vanilla` moves to `efficiency-module-3` (and siblings if added). `ModuleSelect.vue` option values move to `efficiency-module{,-2,-3}`. Display labels already read "Efficiency module" and do not change. + +**A rename alone is not enough.** `src/vue/src/stores/OilFieldStore.ts` persists settings to `localStorage`. A user who ever picked an efficiency module has `effectivity-module-3` saved, and will keep sending that dead name after the fix ships. The store needs a one-time migration on load that rewrites any persisted `effectivity-*` value to `efficiency-*`. + +The fixture's `renames` table is the source for that mapping, so the migration should not hand-type the pairs where it can avoid it. + +**Deviation, as shipped:** `src/vue/src/stores/OilFieldStore.ts` hand-types all three pairs in a `RENAMED_MODULES` map instead of reading them from the fixture's `renames` table. This is a deliberate decision, not an oversight: it is three pairs, both directions are covered by `persistence.test.ts`, and reading the fixture from a Vite/TypeScript build would mean either bundling `factorio-oracle.json` (a maintainer-only artifact, not meant to ship) or adding a build step to generate a TypeScript module from it, for a mapping that only grows when Factorio renames another item. If a future rename makes the hand-typed list unwieldy, generating it from the fixture (the same way `plannerDefaults.verified.json` is generated from `OilFieldOptions`) is the natural next step. + +### C5 - Mirror + +Add `mirror` to `Entity` as `bool?`. Parse it so it survives deserialization; ignore it when planning, because the planner re-chooses every pumpjack orientation anyway; do not emit it. + +This is the same treatment input direction already gets, and it is the minimum that stops a 2.1 blueprint from losing information silently. Honoring a flip is explicitly out of scope - see below. + +### C6 - Corpus re-normalization + +Extend `NormalizeBlueprints` to convert 1.1 direction values to 2.x and stamp a real version, then re-run `oil-field normalize` over both `small-list.txt` (61) and `big-list.txt` (1147). + +Both lists, not just the scored one. `big-list.txt` is not scored so it carries no snapshot risk, but leaving it on 1.1 means the two corpora disagree and the next person to look hits exactly the confusion this document exists to clear up. + +The corpus carries `version: 0` because `CleanBlueprint.cs:34` builds a `new Blueprint` without copying `Version` from the input. + +## The invariant that makes this safe + +Re-normalizing the corpus to 2.x and fixing the parser are inverse operations. Applied together, they should cancel exactly. + +**`Score.HasExpectedScore.verified.txt` should come out unchanged.** + +That turns the 61-blueprint scoreboard from churn to be rubber-stamped into a free end-to-end check. If the scoreboard moves, the change is wrong - most likely the conversion is not symmetric, or the corpus rewrite altered something beyond direction. Investigate before accepting any diff there. + +The same is expected of the per-blueprint plan snapshots. A moved snapshot is a signal, not a chore. + +## Testing + +- `FactorioOracleTest`, per C2. +- Direction round-trip unit tests: a 2.x blueprint parses east as `Right`; a 1.1 blueprint with an explicit sub-2.0 version parses east as `Right`; the ambiguous `{0, 4}` case resolves by version; a non-cardinal value throws with a useful message. +- A regression test pinning the actual reported bug: parse a 2.1-exported blueprint with a non-north pumpjack and assert the orientation survives a round trip. This is the test that would have caught the original report, and the corpus cannot provide it. Generate the fixture with the probe-mod technique above, so it carries a genuine 2.1 version stamp and genuine `4`/`8`/`12` direction values rather than hand-written ones. +- A `mirror: true` blueprint parses without loss or error, per C5. The same probe generates it. +- `Score.HasExpectedScore.verified.txt` unchanged, per the invariant above. +- Vue: a persistence test that a stored `effectivity-module-3` loads as `efficiency-module-3`. + +## Also required, because the core changes + +- Regenerate `src/lua` via `Invoke-LuaBuild.ps1` and commit it, or `transpile-lua` fails. +- Rebuild the WASM bundle (`npm run build-wasm` in `src/vue`). +- Build and test under `UseLuaSettings=true` as well as the default, per the repo's CI matrix. + +## Decided: the drift check stays manual + +`tools/capture-factorio-oracle.sh --check` will not run in CI. CI has no Factorio install and never will, so it could not run there anyway. + +The axis that matters is covered: `FactorioOracleTest` (C2) compares the committed fixture against the C# constants on every build, with no game needed. `--check` covers a different axis - installed game versus committed fixture - and that is a human step after a Factorio update, documented in CLAUDE.md. + +The residual gap is that nothing automatically notices a new Factorio release. Accepted for now. + +## Out of scope + +- **Honoring a pumpjack flip on output.** The planner picks orientations itself, so it is not clear what honoring an input flip would even mean, and answering that needs real investigation into whether a mirrored pumpjack changes valid terminal positions. C5 only stops the flag being lost. +- **Beacon diminishing returns.** Factorio 2.0 models this with a `profile` array; the planner scores beacons as though each contributes equally. A real gap, deliberately not captured in the fixture and not addressed here. Worth its own issue. +- **Quality-scaled beacon effects.** `distribution_effectivity_bonus_per_quality_level` is captured in the fixture but nothing reads it. +- **AAI Industry and other mod entities.** The oracle is vanilla by design. + +## Risks + +- ~~The version threshold is unconfirmed.~~ **Resolved.** Confirmed against a real 2.1.14 export; see the version encoding section. +- **The corpus rewrite is large.** 1208 blueprints across two files. Mitigated by the score invariant: a correct rewrite moves no scores. +- **A blueprint with a genuinely missing version stamp that really is 1.1** will now be read as 2.x and mis-rotated. This is a deliberate trade: it favors the many users pasting current blueprints over the few pasting decade-old ones. + + The error on non-cardinal values catches most of it, but not all, and it is worth being exact about which. Reading 1.1 values as 2.x halves them: east `2` becomes `1` and west `6` becomes `3`, neither of which is a cardinal, so both throw. South `4` becomes `2`, which is a valid `Direction.Right`. **A 1.1 south-facing pumpjack in an unstamped blueprint is therefore read as east, silently.** North is unaffected either way. + + So the guard is loud for two of the three non-north directions and silent for the third. Accepted, because an unstamped 1.1 blueprint is already a corner case and the alternative (defaulting to 1.1) mis-rotates the common case instead. If this proves to matter in practice, the fallback could be tightened by rejecting an unstamped blueprint whose directions are all even and non-zero, which is the signature of 1.1 content. diff --git a/src/FactorioTools.Serialization/OilField/Steps/GridToBlueprintString.cs b/src/FactorioTools.Serialization/OilField/Steps/GridToBlueprintString.cs index 6cdabd4a..3a26d429 100644 --- a/src/FactorioTools.Serialization/OilField/Steps/GridToBlueprintString.cs +++ b/src/FactorioTools.Serialization/OilField/Steps/GridToBlueprintString.cs @@ -38,6 +38,14 @@ private static Direction ToOutputDirection(Direction direction) return (Direction)((int)direction * 2); } + /// + /// The version stamped on every blueprint this class emits. It must be 2.0 or later, + /// because always writes Factorio 2.0's 16-way direction + /// values, and reads the stamp to decide + /// how to read them back. + /// + private static readonly ulong OutputVersion = FormatVersion(2, 0, 32, 0); + // defines.inventory.mining_drill_modules and defines.inventory.beacon_modules in Factorio 2.0, used to target the // module inventory in the 2.0 "items" array form (confirmed against a real 2.0 export). private const int MiningDrillModuleInventory = 2; @@ -104,7 +112,7 @@ int GetEntityNumber(GridEntity entity) entities.Add(new Entity { EntityNumber = nextEntityNumber++, - Direction = ToOutputDirection(pumpjackCenter.Direction), + Direction = pumpjackCenter.Direction, Name = EntityNames.Vanilla.Pumpjack, Position = position, Items = ToOutputItems(context.Options.PumpjackModules, MiningDrillModuleInventory, context.Options.PumpjackModuleQuality), @@ -118,7 +126,7 @@ int GetEntityNumber(GridEntity entity) entities.Add(new Entity { EntityNumber = nextEntityNumber++, - Direction = ToOutputDirection(undergroundPipe.Direction), + Direction = undergroundPipe.Direction, Name = EntityNames.Vanilla.PipeToGround, Position = position, }); @@ -219,7 +227,6 @@ int GetEntityNumber(GridEntity entity) } } }, - Version = FormatVersion(2, 0, 32, 0), Item = ItemNames.Vanilla.Blueprint, Entities = entities.ToArray(), }; @@ -245,8 +252,29 @@ public static ulong FormatVersion(ushort major, ushort minor, ushort patch, usho return ((ulong)major << 48) | ((ulong)minor << 32) | ((ulong)patch << 16) | developer; } + /// + /// Turns a blueprint holding internal directions into a blueprint string. This rewrites the + /// blueprint it is given - version, directions, and with addFbeOffset the entity array - so + /// call it once per blueprint. Every caller today builds a fresh blueprint for it. + /// public static string SerializeBlueprint(Blueprint blueprint, bool addFbeOffset) { + // Every blueprint string this class produces goes through here, so this is the one + // place that converts internal directions to Factorio 2.0's 16-way values and the one + // place that stamps the version saying so. Keeping them together means they can never + // disagree. Before, only the planner path converted, so the normalize path (see + // NormalizeBlueprints and PlanOrchestrator, which serialize a CleanBlueprint result) + // emitted internal 1.1-style directions under version 0. + blueprint.Version = OutputVersion; + for (var i = 0; i < blueprint.Entities.Length; i++) + { + var entity = blueprint.Entities[i]; + if (entity.Direction.HasValue) + { + entity.Direction = ToOutputDirection(entity.Direction.Value); + } + } + // FBE applies some offset to the blueprint coordinates. This makes it hard to compare the grid used in memory // with the rendered blueprint in FBE. To account for this, we can add an entity to the corner of the // blueprint with a position that makes FBE keep the original entity positions used by the grid. diff --git a/src/FactorioTools.Serialization/OilField/Steps/ParseBlueprint.cs b/src/FactorioTools.Serialization/OilField/Steps/ParseBlueprint.cs index 3ff77552..7fabff83 100644 --- a/src/FactorioTools.Serialization/OilField/Steps/ParseBlueprint.cs +++ b/src/FactorioTools.Serialization/OilField/Steps/ParseBlueprint.cs @@ -10,6 +10,56 @@ namespace Knapcode.FactorioTools.OilField; public static class ParseBlueprint { + /// + /// The blueprint version at which Factorio widened directions from 8-way to 16-way. + /// GridToBlueprintString.FormatVersion(2, 0, 0, 0). Confirmed against a real 2.1.14 + /// export, whose version is 562954249306113 (2.1.14.1). + /// + private const ulong FirstSixteenWayVersion = 562949953421312UL; + + /// + /// Converts a blueprint's direction to the internal 1.1-style four-way + /// (N=0, E=2, S=4, W=6). + /// + /// Factorio 2.0 widened directions to 16-way (N=0, E=4, S=8, W=12). The old values are + /// still legal, so a 2.x east read as 1.1 is not an error, it is a silently rotated + /// pumpjack. Sniffing the values cannot resolve it either: a blueprint whose directions + /// are all in {0, 4} is valid under both readings with different meanings. The version + /// is the only sound signal. + /// + /// A missing or zero version is treated as modern, because that is what users paste + /// today. The trade-off is spelled out in the design doc. + /// + public static Direction ToInternalDirection(Direction direction, ulong version) + { + var raw = (int)direction; + var internalValue = raw; + + if (version == 0 || version >= FirstSixteenWayVersion) + { + // Halving turns a 16-way value into an internal one, but only for even values. + // Integer division would quietly round an odd value down to the cardinal below it + // - 1 to Up, 13 to Left - so leave odd values alone and let the check below reject + // them, the same way it rejects the even non-cardinals 2, 6, 10 and 14. + if (raw % 2 == 0) + { + internalValue = raw / 2; + } + } + + if (internalValue != (int)Direction.Up + && internalValue != (int)Direction.Right + && internalValue != (int)Direction.Down + && internalValue != (int)Direction.Left) + { + throw new FactorioToolsException( + $"Blueprint direction {raw} is not one of the four directions a pumpjack can face.", + badInput: true); + } + + return (Direction)internalValue; + } + public static List ReadBlueprintFile(string fileName) { return File @@ -82,6 +132,21 @@ public static Blueprint Execute(string blueprintString) throw new FactorioToolsException("No blueprint was found in the deserialized JSON.", badInput: true); } + for (var i = 0; i < root.Blueprint.Entities.Length; i++) + { + var entity = root.Blueprint.Entities[i]; + + // Only pumpjack directions are converted, because only pumpjack directions are read + // downstream - InitializeContext and CleanBlueprint both keep the pumpjacks and + // discard every other entity. Converting the rest would also reject any blueprint + // holding a diagonal entity, such as a rail at 2.x direction 2, 6, 10 or 14, which + // parsed fine before. + if (entity.Direction.HasValue && entity.Name == EntityNames.Vanilla.Pumpjack) + { + entity.Direction = ToInternalDirection(entity.Direction.Value, root.Blueprint.Version); + } + } + return root.Blueprint; } } diff --git a/src/FactorioTools/Data/Entity.cs b/src/FactorioTools/Data/Entity.cs index eb7b6e5c..af0e10c9 100644 --- a/src/FactorioTools/Data/Entity.cs +++ b/src/FactorioTools/Data/Entity.cs @@ -17,6 +17,13 @@ public class Entity [JsonPropertyName("direction")] public Direction? Direction { get; set; } + // Factorio 2.1.7 added pumpjack and burner mining drill flipping, and a flipped entity + // carries "mirror": true (confirmed by round-tripping a blueprint through 2.1.14). + // Parsed so the flag is not silently lost. The planner re-chooses every pumpjack + // orientation itself, so nothing reads this and it is never emitted. + [JsonPropertyName("mirror")] + public bool? Mirror { get; set; } + // Either a Dictionary (Factorio 1.1 "items" object) or a List (Factorio 2.0 "items" // array). The shape is handled by EntityItemsConverter in the serialization project so the core library stays free // of serialization logic. See GridToBlueprintString for how each version is produced. diff --git a/src/FactorioTools/Data/ItemNames.cs b/src/FactorioTools/Data/ItemNames.cs index 69f59a83..c1a98775 100644 --- a/src/FactorioTools/Data/ItemNames.cs +++ b/src/FactorioTools/Data/ItemNames.cs @@ -4,7 +4,9 @@ public static class ItemNames { public static class Vanilla { - public const string EfficiencyModule3 = "effectivity-module-3"; + // Renamed in Factorio 2.0. "effectivity-module-3" no longer exists, so the game + // silently rejects it. See base/migrations/2.0.0.json in the game data. + public const string EfficiencyModule3 = "efficiency-module-3"; public const string ProductivityModule3 = "productivity-module-3"; public const string SpeedModule3 = "speed-module-3"; diff --git a/src/vue/src/components/ModuleSelect.vue b/src/vue/src/components/ModuleSelect.vue index ecd05ea3..eb9756f8 100644 --- a/src/vue/src/components/ModuleSelect.vue +++ b/src/vue/src/components/ModuleSelect.vue @@ -9,9 +9,9 @@ @update:modelValue="(newVal: string) => $emit('update:modelValue', newVal)" > - - - + + +