From 0364f9e7f931a8fdf751519dd9909ff2444a94e1 Mon Sep 17 00:00:00 2001 From: tsubasakong Date: Sun, 12 Jul 2026 16:09:10 -0700 Subject: [PATCH] fix: preserve agent inventories when serializing GameState 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 --- fle/commons/models/game_state.py | 26 +++++++- fle/env/tools/admin/reset/client.py | 14 +++- tests/unit/conftest.py | 13 ++++ tests/unit/test_game_state_serialization.py | 71 +++++++++++++++++++++ 4 files changed, 119 insertions(+), 5 deletions(-) create mode 100644 tests/unit/conftest.py create mode 100644 tests/unit/test_game_state_serialization.py diff --git a/fle/commons/models/game_state.py b/fle/commons/models/game_state.py index 027f7c9c1..fbdec8a05 100644 --- a/fle/commons/models/game_state.py +++ b/fle/commons/models/game_state.py @@ -167,13 +167,35 @@ def parse(cls, data) -> "GameState": agent_messages=cls.parse_agent_messages(data), ) + @staticmethod + def _inventory_to_dict(inventory) -> Any: + """Serialize an inventory to a plain dict of item counts. + + Inventory is a pydantic model with extra="allow": item counts are + stored in __pydantic_extra__, so `inventory.__dict__` is always {}. + Serializing via __dict__ silently wiped every agent inventory in + saved game states. (It worked historically because Inventory once + populated self.__dict__ directly in a custom __init__ — see the + commented-out constructor in fle/env/entities.py — and broke when + that constructor was removed.) + """ + if hasattr(inventory, "model_dump"): + return inventory.model_dump() + if hasattr(inventory, "items"): + try: + return dict(inventory.items()) + except Exception: + pass + if hasattr(inventory, "__dict__"): + return inventory.__dict__ + return inventory + def to_raw(self) -> str: """Convert state to JSON string""" data = { "entities": self.entities, "inventories": [ - inventory.__dict__ if hasattr(inventory, "__dict__") else inventory - for inventory in self.inventories + self._inventory_to_dict(inventory) for inventory in self.inventories ], "timestamp": self.timestamp, "namespaces": [ns.hex() if ns else "" for ns in self.namespaces], diff --git a/fle/env/tools/admin/reset/client.py b/fle/env/tools/admin/reset/client.py index 28ed3349e..11690fbc1 100644 --- a/fle/env/tools/admin/reset/client.py +++ b/fle/env/tools/admin/reset/client.py @@ -27,10 +27,18 @@ def __call__( dict_inventories = [] for inv in inventories: - if not isinstance(inv, dict): - dict_inventories.append(inv.__dict__) - else: + if isinstance(inv, dict): dict_inventories.append(inv) + elif hasattr(inv, "model_dump"): + # Inventory is a pydantic extra="allow" model: item counts + # live in __pydantic_extra__ and inv.__dict__ is {}, so the + # previous inv.__dict__ serialization reset agents to EMPTY + # inventories whenever a GameState was restored. + dict_inventories.append(inv.model_dump()) + elif hasattr(inv, "items"): + dict_inventories.append(dict(inv.items())) + else: + dict_inventories.append(inv.__dict__) # Encode to JSON string for Lua inventories_json = json.dumps(dict_inventories) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py new file mode 100644 index 000000000..bc2b09027 --- /dev/null +++ b/tests/unit/conftest.py @@ -0,0 +1,13 @@ +"""Unit tests in this directory run WITHOUT a live Factorio server. + +The repo-level tests/conftest.py installs an autouse fixture +(_reset_between_tests) that depends on a running game instance; shadow it +with a no-op here so pure serialization/unit tests stay server-free. +""" + +import pytest + + +@pytest.fixture(autouse=True) +def _reset_between_tests(): + yield diff --git a/tests/unit/test_game_state_serialization.py b/tests/unit/test_game_state_serialization.py new file mode 100644 index 000000000..1942107da --- /dev/null +++ b/tests/unit/test_game_state_serialization.py @@ -0,0 +1,71 @@ +"""Regression tests: GameState serialization must not wipe agent inventories. + +Inventory (fle/env/entities.py) is a pydantic model with extra="allow", so +item counts live in __pydantic_extra__ and `inventory.__dict__` is always {}. +GameState.to_raw() and the reset tool client previously serialized +inventories via __dict__, which silently emptied every agent inventory in +saved snapshots and on every GameState restore. +""" + +import json + +from fle.commons.models.game_state import GameState +from fle.env.entities import Inventory + + +class TestGameStateToRaw: + def test_preserves_pydantic_extra_inventories(self): + gs = GameState( + entities="abc", + inventories=[Inventory(**{"iron-plate": 10, "coal": 5})], + research=None, + ) + raw = json.loads(gs.to_raw()) + assert raw["inventories"] == [{"iron-plate": 10, "coal": 5}] + + def test_accepts_plain_dict_inventories(self): + gs = GameState(entities="abc", inventories=[{"stone": 3}], research=None) + raw = json.loads(gs.to_raw()) + assert raw["inventories"] == [{"stone": 3}] + + def test_multiagent_roundtrip_through_parse_raw(self): + gs = GameState( + entities="abc", + inventories=[ + Inventory(**{"iron-plate": 10}), + Inventory(**{"copper-cable": 7, "coal": 1}), + ], + research=None, + ) + restored = GameState.parse_raw(gs.to_raw()) + assert restored.inventories == [ + {"iron-plate": 10}, + {"copper-cable": 7, "coal": 1}, + ] + + +class TestResetClientInventorySerialization: + def test_inventory_objects_serialize_to_item_counts(self): + """The reset tool sends inventories to Lua as JSON; Inventory objects + must serialize to their item counts, not to an empty __dict__.""" + from fle.env.tools.admin.reset.client import Reset + + # Replicate the client's serialization logic on a mixed input list + # without needing a live connection: exercise __call__'s conversion + # by monkeypatching execute. + captured = {} + + class FakeReset(Reset): + def __init__(self): # bypass Tool/connection setup entirely + pass + + def execute(self, inventories_json, *args): + captured["json"] = inventories_json + return True, 0.0 + + FakeReset()( + inventories=[Inventory(**{"iron-plate": 4}), {"stone": 2}], + reset_position=False, + ) + sent = json.loads(captured["json"]) + assert sent == [{"iron-plate": 4}, {"stone": 2}]