fix: preserve agent inventories when serializing GameState - #374
Open
tsubasakong wants to merge 1 commit into
Open
fix: preserve agent inventories when serializing GameState#374tsubasakong wants to merge 1 commit into
tsubasakong wants to merge 1 commit into
Conversation
Inventory (fle/env/entities.py) is a pydantic model with extra="allow",
so item counts are stored in __pydantic_extra__ and inventory.__dict__
is always {}. GameState.to_raw() and the reset tool client serialized
inventories via __dict__, which:
- wrote "inventories": [{}] into every saved game state, and
- reset every agent to an EMPTY inventory whenever a GameState was
restored (FactorioInstance.reset(game_state) passes inventories
through the reset tool).
This worked historically because Inventory populated self.__dict__ in a
custom __init__ (still visible, commented out, in entities.py) and broke
silently when that constructor was removed.
Serialize via model_dump() instead, with items()/__dict__ fallbacks for
non-pydantic inputs. Adds server-free regression tests under tests/unit/
including a to_raw -> parse_raw round-trip; on the previous code these
fail 3/4, with the fix 4/4 pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Problem
Inventory(fle/env/entities.py) is a pydantic model withextra="allow", so item counts are stored in__pydantic_extra__andinventory.__dict__is always{}. Two call sites serialize inventories via__dict__:GameState.to_raw()(fle/commons/models/game_state.py) — every saved game state gets"inventories": [{}]resettool client (fle/env/tools/admin/reset/client.py) —FactorioInstance.reset(game_state)passes inventories through this path, so every GameState restore silently resets all agents to empty inventoriesThis appears to be a regression:
Inventorypreviously populatedself.__dict__directly in a custom__init__(still visible, commented out, infle/env/entities.py), which is why__dict__-based serialization used to work — and why it broke silently when that constructor was removed.Fix
Serialize via
model_dump()(which includes pydantic extras), withitems()/__dict__fallbacks for non-pydantic inputs. Plain-dict inventories are unaffected.Tests
Adds server-free regression tests under
tests/unit/(with a local conftest that shadows the repo-level autouse fixture requiring a live Factorio server):to_rawpreserves pydantic-extra inventoriesto_raw→parse_rawround-tripNegative control: on the unfixed code these tests fail 3/4 (the only pass is the plain-dict case, which was never broken); with the fix, 4/4 pass.
🤖 Generated with Claude Code