From 0c2da217f3a21042d51131021772e24b45718cda Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Thu, 17 Sep 2026 18:22:22 +0200 Subject: [PATCH] Cut 0: give Entity its own EntitySettings copy, delete the duplicate copies Entity's constructor now takes a MessagePack round-trip copy of the settings it is given, so no entity can alias GameplaySettings.DefaultEntitySettings. That makes ActionGameManager.NewEntitySettings and EntitySerializer.Unpack's null-coalescing copy redundant; both are deleted, and TradeMenu buys a ship straight off the template (Entity's ctor copies it). EntitiesDoNotShareDefaultEntitySettings pins the invariant: writing ShutdownPerformance on one ship must not move the default for a sibling or the template (settings-globals-cut.md, Cut 0). Co-Authored-By: Claude Opus 5 --- Assets/Scripts/Gameplay/ActionGameManager.cs | 5 ---- Assets/Scripts/ServerShared/Entity.cs | 3 ++- .../Scripts/ServerShared/EntitySerializer.cs | 2 -- Assets/Scripts/UI/Menu/TradeMenu.cs | 2 +- tests/Aetheria.Shared.Tests/LoadoutTests.cs | 27 +++++++++++++++++++ 5 files changed, 30 insertions(+), 9 deletions(-) diff --git a/Assets/Scripts/Gameplay/ActionGameManager.cs b/Assets/Scripts/Gameplay/ActionGameManager.cs index 3573dfba3..491fb93c5 100644 --- a/Assets/Scripts/Gameplay/ActionGameManager.cs +++ b/Assets/Scripts/Gameplay/ActionGameManager.cs @@ -225,11 +225,6 @@ public class ActionGameManager : MonoBehaviour private List _stories = new List(); - public EntitySettings NewEntitySettings - { - get => MessagePackSerializer.Deserialize(MessagePackSerializer.Serialize(Settings.GameplaySettings.DefaultEntitySettings)); - } - private void OnApplicationQuit() { SaveRun(); diff --git a/Assets/Scripts/ServerShared/Entity.cs b/Assets/Scripts/ServerShared/Entity.cs index 5870c11a5..0383c96e1 100644 --- a/Assets/Scripts/ServerShared/Entity.cs +++ b/Assets/Scripts/ServerShared/Entity.cs @@ -6,6 +6,7 @@ using GameCult.Caching; using System; using System.Collections.Generic; using System.Linq; +using MessagePack; using UniRx; using CultMath; using static CultMath.math; @@ -337,7 +338,7 @@ public abstract class Entity public Entity(ItemManager itemManager, Zone zone, EquippableItem hull, EntitySettings settings) { - Settings = settings; + Settings = MessagePackSerializer.Deserialize(MessagePackSerializer.Serialize(settings)); ItemManager = itemManager; Zone = zone; Hull = hull; diff --git a/Assets/Scripts/ServerShared/EntitySerializer.cs b/Assets/Scripts/ServerShared/EntitySerializer.cs index fc375dd8d..4b727eee4 100644 --- a/Assets/Scripts/ServerShared/EntitySerializer.cs +++ b/Assets/Scripts/ServerShared/EntitySerializer.cs @@ -48,8 +48,6 @@ public static class EntitySerializer public static Entity Unpack(ItemManager itemManager, Zone zone, EntityPack pack) { - pack.Settings ??= MessagePackSerializer.Deserialize( - MessagePackSerializer.Serialize(itemManager.GameplaySettings.DefaultEntitySettings)); return pack switch { ShipPack shipPack => Unpack(itemManager, zone, shipPack), diff --git a/Assets/Scripts/UI/Menu/TradeMenu.cs b/Assets/Scripts/UI/Menu/TradeMenu.cs index 1642dc24c..9bc285b75 100644 --- a/Assets/Scripts/UI/Menu/TradeMenu.cs +++ b/Assets/Scripts/UI/Menu/TradeMenu.cs @@ -374,7 +374,7 @@ public class TradeMenu : MonoBehaviour { if (hullData.HullType != HullType.Ship) throw new ArgumentException("Attempted to buy non-ship hull from station, WTF are you doing?!"); - var ship = new Ship(GameManager.ItemManager, GameManager.Zone, item as EquippableItem, GameManager.NewEntitySettings) { IsPlayerShip = true }; + var ship = new Ship(GameManager.ItemManager, GameManager.Zone, item as EquippableItem, GameManager.ItemManager.GameplaySettings.DefaultEntitySettings) { IsPlayerShip = true }; ship.SetParent(GameManager.DockedEntity); GameManager.Credits -= data.Price; diff --git a/tests/Aetheria.Shared.Tests/LoadoutTests.cs b/tests/Aetheria.Shared.Tests/LoadoutTests.cs index 911434dcf..6403120a5 100644 --- a/tests/Aetheria.Shared.Tests/LoadoutTests.cs +++ b/tests/Aetheria.Shared.Tests/LoadoutTests.cs @@ -715,6 +715,33 @@ public sealed class LoadoutTests : IDisposable } } + // Cut 0 (settings-globals-cut.md): Entity's ctor must copy the template it is given. No entity may alias + // GameplaySettings.DefaultEntitySettings, or a write through one ship (InventoryMenu's Shutdown Threshold + // edit) would silently move the default for every other ship, and once settings are a catalog global, the + // cached record itself. + [Fact] + public void EntitiesDoNotShareDefaultEntitySettings() + { + using var cache = Open(); + var items = new ItemManager(cache, new ProvenanceLedger(), RunSaveTests.TestSettings(), _ => { }); + var maker = cache.RefOf(cache.GetByName("Maker")); + var hullData = cache.GetByName("Skiff"); + var template = items.GameplaySettings.DefaultEntitySettings; + var originalThreshold = template.ShutdownPerformance; + + Ship BuildShip() => new Ship(items, null, (EquippableItem) items.CreateInstance(items.CreateLot(hullData, maker, .5f)), template); + + var shipA = BuildShip(); + var shipB = BuildShip(); + + shipA.Settings.ShutdownPerformance = originalThreshold + 1; + + Assert.NotSame(template, shipA.Settings); + Assert.NotSame(shipA.Settings, shipB.Settings); + Assert.Equal(originalThreshold, shipB.Settings.ShutdownPerformance); + Assert.Equal(originalThreshold, template.ShutdownPerformance); + } + // Everything available: for tests about placement and failure lists. private static Ship Build(ItemManager items, Loadout loadout, List failures) => Loadouts.Materialize(items, null, loadout, _ => true, failures);