v1.2 (experimental) save support + comprehensive test suite#15
Open
ChrisonSimtian wants to merge 12 commits into
Open
v1.2 (experimental) save support + comprehensive test suite#15ChrisonSimtian wants to merge 12 commits into
ChrisonSimtian wants to merge 12 commits into
Conversation
Progresses save-version-60 parsing far enough to reach the property layer but stops short of full support. v1.0 (SaveVersion 46) is unaffected. What this commit lands: - FSaveObjectVersionData model + serializer + nested types (FPackageFileVersion, FEngineVersion, FCustomVersion[Container]). - ComponentObject.ObjectVersionData property. - SaveFileSerializer: read save-level FSaveObjectVersionData immediately after the body envelope at header.SaveVersion >= 53. This was the missing first read that previously caused the body parse to mis-align at the grid block. - BodySerializer: pass per-level saveCustomVersion through to ObjectSerializer, and read an optional level-wide FSaveObjectVersionData after the collectables list for non-persistent levels at >= 53. - ObjectSerializer: accept a saveVersion parameter on Deserialize and read an optional post-body FSaveObjectVersionData per object, keyed on the per-object saveCustomVersion captured at >= 41. - PropertySerializer: improve the unknown-property-type exception message to include the offending name + stream position. What's still missing (the real wall): - FPropertyTag in v1.2 changes shape: when saveVersion.object >= 53 AND packageFileVersionUE5 >= PROPERTY_TAG_COMPLETE_TYPE_NAME, every property tag is now a nested FPropertyTagNode with a flags byte plus optional index/GUID, and SaveObject.ParseData reads a one-byte 'serializationControl' before the property list. - Reaching that branch requires porting FPropertyTagNode, the IsCompletePropertyTagType gate, and per-property-type readers that consume the new tag shape. This is a much larger refactor than the original diagnostic on issue #31 estimated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Builds on the FSaveObjectVersionData groundwork and now parses Chris's current v1.2 save (header SaveVersion 60, BuildVersion 489969): Levels: 302, Total objects: 7,218 19 Build_MinerMk1_C, 20 Build_SmelterMk1_C, 43 Constructors, 4 Assemblers, 459 BP_ResourceNode_C, 242 Mk1 + 173 Mk2 belts. v1.0 (SaveVersion 46) unchanged: 241 levels, 2,278 objects, 4 miners. Strategy: minimal viable port — read enough of the new layout to keep the stream aligned through every object, defer deep per-type parsing of properties and class-specific extra data to follow-ups. Lands: - FPropertyTagNode model (recursive type tree used by the v1.2 complete-property-tag format). - RawProperty (placeholder Property covering the new tag's metadata plus an opaque value blob; PropertyConstraint.Raw enum added). - IPropertySerializer.DeserializeProperties/Property accept a saveVersion parameter; PropertySerializer threads it through. - PropertySerializer reads the v1.2 tag (FPropertyTagNode + binarySize + flags + optional index/GUID) and skips binarySize bytes for the value, returning a RawProperty so callers stay aligned. - PropertySerializer.DeserializeProperties consumes the leading serializationControl byte that the v1.2 SaveObject.ParseData writes before the property list. - ObjectSerializer threads saveVersion into the property reader and reads the post-properties hasGuid+GUID pair at v1.2 before extra data. - ObjectSerializer skips the class-specific ExtraData branch at v1.2 — Conveyor/PowerLine/Circuit/etc. layouts diverged; the missing- bytes handler at the end of the body parse absorbs the remainder and keeps the stream lined up to the next object. Per-class v1.2 ports come later. - ExtraDataSerializer.DeserializeCircuitData: v1.2 dropped the leading Int32 (it now matches etothepii's CircuitSpecialProperties.Parse — single count + records). Version-gated on header.SaveVersion < 53. Known limitations (intentional for this commit): - Properties on v1.2 objects are RawProperty (no value). Per-type readers in the new format are TODO. - ExtraData (Conveyor, PowerLine, Circuit, Drone, Vehicle, …) on v1.2 is unread. Per-class v1.2 layouts are TODO. - Component-header decode at v1.2 may be slightly off: etothepii reports ~1.8k FGFactoryConnectionComponent on this save where the fork shows zero. Total-object delta is ~1–2k. Doesn't affect actor counts or positions (which are what miner/smelter/etc. inventories actually need). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…, StrProperty
Lifts the C# fork from "counts only" to full feature parity with
@etothepii/satisfactory-file-parser on the metrics that matter to the planner:
per-miner mined resource, per-producer recipe, per-generator fuel.
Parity confirmed against the Node reference on the same v1.2 save —
every count matches exactly:
Miners by resource: 1 Coal, 3 OreCopper, 12 OreIron, 3 Stone
Smelters by recipe: 16 IngotIron, 4 IngotCopper
Constructors: 14 IronRod, 9 Screw, 8 IronPlate, 4 Concrete,
2 Wire, 2 Biofuel, 1 Cable, 1 Biomass_Wood,
1 Biomass_Leaves, 1 CopperSheet
Coal generators: 6 on Coal
Biomass burners: 12 on Biofuel
Water extractors: 3
Approach:
- Targeted, not exhaustive. RawProperty (the v1.2 placeholder Property we
already produce) gains three optional value slots:
* ObjectValue (ObjectReferenceValue, for ObjectProperty)
* ArrayObjectValues (IReadOnlyList<ObjectReferenceValue>, for
ArrayProperty whose element is ObjectProperty)
* StringValue (string, for StrProperty / NameProperty)
Every other property type continues to round-trip as an opaque
RawProperty — the planner doesn't need its value yet.
- New ObjectReferenceValue readonly record struct (LevelName, PathName) in
the Properties namespace alongside RawProperty.
- PropertySerializer.TryParseKnownValue is the dispatcher. It runs after
the FPropertyTag is consumed and before the binary-size fence. Each
branch reads exactly the bytes its property type defines (per
etothepii's ObjectProperty.Parse / ArrayProperty.Parse for the Object
subtype / StrProperty.Parse).
- Hardening: the v1.2 reader now bounds every property value with
`posBeforeValue + binarySize`. If the new value parser reads too few
or too many bytes — or hits an unhandled type — the fence seeks the
stream back to the expected position rather than throwing. Keeps
parsing resilient to future property-type additions.
- Array elementCount sanity-capped at 1M; refuses to allocate hostile
arrays and falls back to "skip and align".
Old code paths (header SaveVersion < 53) are untouched — v1.0 saves still
parse identically to before (241 levels, 2,278 objects, 4 miners on the
Finally 1.0 save).
Known gaps (intentional, follow-ups):
- StructProperty, MapProperty, SetProperty, IntProperty, FloatProperty,
BoolProperty etc. still come back as RawProperty without value data.
The planner needs none of these for stocktakes today.
- Class-specific ExtraData at v1.2 (ConveyorBelt segment paths,
PowerLine endpoints, etc.) is still skipped.
- Component-header decode at v1.2 still under-counts component classes
vs etothepii (~1k difference). Doesn't affect Actor counts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-enables Closes the remaining property-value gaps in the v1.2 path that the planner can benefit from today, and re-enables the class-specific ExtraData branches whose v1.2 format is a one-or-two-line port. Property values now decoded in the v1.2 RawProperty path: - All scalar Int*, UInt*, Float, Double properties (read exactly their binarySize bytes). - BoolProperty (value is encoded in tag flag bit 0x10 — no value bytes). - StructProperty for the fixed-size raw struct types: Vector, Rotator, Quat, Vector4(D), Vector2D, Box, LinearColor, Color, IntPoint, DateTime, Guid, TimerHandle, SlateBrush, FluidBox, RailroadTrackPosition. Unknown / property-list-shaped structs leave StructValue.Value null and the binary-size fence in the outer reader advances past the bytes. - MapProperty when key AND value are both fixed-size primitives or framed strings (Int/Long/UInt/ULong/Byte/Bool/Float/Double/Str/Name/Enum/Object). Composite-typed maps (Struct/Array/Set/Map-of-Maps) leave MapEntries null. Why conservative on Struct + Map: the first attempt at this work tried DynamicStructPropertyValue (recursive property-list reads from inside a StructProperty value) and similar handling for Map struct keys/values, plus a try-catch-rewind fence to absorb missteps. The recursion was unbounded for struct shapes we couldn't recognise and ended up either runaway-reading or catching+rewinding millions of exceptions — the parse degraded from 1 s to >2 min on the same save. The new implementation intentionally skips any property type whose binary extent depends on data we can't see in the tag alone, and trusts the binary-size fence to advance past it. Class-specific ExtraData at v1.2: - ConveyorBelt / ConveyorLift: v1.2 moved per-belt item storage onto the FGConveyorChainActor — per-belt ExtraData is now a single Int32 zero (matches etothepii's ConveyorSpecialProperties.Parse). - PowerLine: v1.2 dropped the leading count Int32; ExtraData is now just source + target ObjectReferences (matches PowerLineSpecialProperties.Parse). - CircuitSubsystem: already v1.2-correct from an earlier commit; the ObjectSerializer skip-list excluded it incorrectly — re-included now. - ConveyorChainActor, Vehicle, Drone, Player, LightweightBuildableSubsystem: still skipped at v1.2. Their v1.2 layouts are complex enough to warrant a follow-up; the missing-bytes handler at the end of the body parse absorbs them today. Verified on the v1.2 Beta Game save (SaveVersion 60, BuildVersion 489969): 302 levels · 3690 actors · 7326 total objects · parsed in ~200 ms. v1.0 save (SaveVersion 46) unchanged: 241 levels · 1659 actors · 2278 total objects · 4 miners · 120 ms. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Introduced a comprehensive test suite for the SatisfactorySaveNet serialization logic, focusing on version 1.2. - Created a README.md file detailing the structure and purpose of the tests. - Implemented BinaryReaderHelpers for synthesizing byte sequences in tests. - Developed tests for FSaveObjectVersionDataSerializer, ObjectSerializer, PropertySerializer, and SaveFileSerializer. - Ensured coverage for both unit tests with synthesized data and integration tests using real .sav file fixtures. - Added assertions to validate deserialization behavior and stream consumption.
… architectural improvements
…Object Captures the rationale, proposed API surface, and impact for adding convenience accessors (TryGetEnumValue/TryGetFloatValue/etc.) on ComponentObject so downstream adapters don't repeat property-walk boilerplate for every actor type. Tracked here rather than as a GitHub issue because issues are disabled on this fork. Referenced from consuming-repo issue erp-for-factory-games/ErpForFactoryGames#35. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Proposes shipping a known-nodes dataset (coordinate-keyed) alongside the parser so downstream consumers don't each have to maintain their own. Resource type + purity are blueprint-class defaults — not in the .sav — so this is the only sane path short of parsing .pak assets. Referenced from the ERP planner's resource node identification work (consumer-side seed lives in src/Satisfactory/Save/Data/ until this lands upstream). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
|
I checked this with all the save games that I have gathered. This is around 35 games from some different players. The 1.2 seems to parse now. |
Owner
|
I am currently short on time and this PR is a little bigger than expected. A careful review might take some hours and I am not sure when I will be able to finish it. @denxorz if you have a little time left and would like to - it would be very kind if you could lend me hand reviewing it based on my step by step guide. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FSaveObjectVersionDatablock, new property tag format withserializationControlbyte, optional per-object GUID, v1.2 ExtraData branches for Conveyor / PowerLine / CircuitSubsystem.SatisfactorySaveNet.dllsequence coverage from effectively 0% to ~63%..savfixtures committed for end-to-end regression testing, plus aFixtures/.gitignorethat keeps personal autosaves out of git while still smoke-testing them locally.Update-Fixtures.ps1helper to pull matching saves from%LOCALAPPDATA%\FactoryGame\Saved\SaveGames\…into the test project.TODO.mdat the repo root tracking follow-ups — most importantly an architectural sketch for separating vanilla and mod-specific types (FicsItNetworks, FicsItFarming, etc.) into their own packages.Happy to split into smaller PRs if that's easier — the test suite + fixtures are independent of the v1.2 parser work and can land separately.
v1.1 ↔ v1.2 compatibility
The v1.2 work is gated behind
SaveCustomVersion >= 53everywhere it diverges. Three layers prove v1.1 still parses cleanly:Serializers/PropertySerializerLegacyTests— every typedDeserialize<Type>Propertybranch atsaveVersion = 50Serializers/ExtraDataSerializerLegacyTests— every pre-v1.2 ExtraData branch (Conveyor / PowerLine / Circuit / Vehicle / Locomotive / Blueprint / PlayerData / UnknownExtraData fallback / etc.)Compat/VersionCompatibilityTests— cross-version dispatch tests pinning the legacy-vs-RawProperty switch, the ObjectHeader Flags gate at v51, the TypedData double-vs-float gate at v41, the CircuitData leading-count gate at v53Deliberate-break sanity check: flipping
>= 53→> 53inPropertySerializer.cs:60fails at least one test in the compatibility suite.Production-code changes
SatisfactorySaveNet/SatisfactorySaveNet.csproj—InternalsVisibleTo("SatisfactorySaveNet.Tests")SatisfactorySaveNet/ObjectSerializer.cs—ReadOptionalObjectGuidandReadOptionalPostBodyVersionDatapromoted fromprivatetointernalso the helpers can be unit-tested directlySatisfactorySaveNet/FSaveObjectVersionDataSerializer.cs(entirely new for v1.2)PropertySerializer.cs,ObjectSerializer.cs,BodySerializer.cs,SaveFileSerializer.cs,ExtraDataSerializer.csKnown issue pinned by regression test
PropertySerializer.cs:117gatesTryParseKnownValuebehindif (binarySize > 0), but theBoolPropertycase inside that method documents itself as having no value bytes — the boolean lives in flag bit0x10. If a save emitsBoolPropertywithbinarySize == 0,RawProperty.BoolValuestays null. Pinned byDeserializeProperty_AtV12_BoolProperty_WithZeroBinarySize_LeavesBoolValueNullas a regression marker — happy to fix if you want the value populated from the flag bit even whenbinarySize == 0. See item 3 inTODO.md.Test plan
dotnet build SatisfactorySaveNet.sln— clean, zero warningsdotnet test SatisfactorySaveNet.sln— 134/134 pass (10s wall time)dotnet test … --collect:"XPlat Code Coverage" --settings coverlet.runsettings.xml— module aggregate ~63% sequence🤖 Generated with Claude Code