fix(inventory add): no longer crashes when read only items are added

Using ItemEntryData to pass in items to InventoryInstance.AddEntry would crash due to an edge case where it thought the read only item could be runtime manipulated. fix #17
ashblue · Jan 11, 2025 · afb6b44 · afb6b44
1 parent 489ec64
commit afb6b44
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 9 deletions.
diff --git a/Assets/com.fluid.elastic-inventory/Runtime/Inventory/InventoryInstance.cs b/Assets/com.fluid.elastic-inventory/Runtime/Inventory/InventoryInstance.cs
@@ -35,13 +35,13 @@ public IItemEntryReadOnly Add(IItemDefinition item, int quantity = 1) {
             IItemEntryReadOnly entry;
             if (item.Unique) {
                 entry = item.CreateItemEntry(_database);
-                AddEntry((IItemEntry)entry);
+                AddEntryInternal((IItemEntry)entry);
             } else if (_entries.TryGetValue(item, out var existingEntry)) {
                 existingEntry.SetQuantity(existingEntry.Quantity + quantity);
                 entry = existingEntry;
             } else {
                 entry = item.CreateItemEntry(_database, quantity);
-                AddEntry((IItemEntry)entry);
+                AddEntryInternal((IItemEntry)entry);
             }
 
             Events.ItemAdded.Invoke(entry);
@@ -58,17 +58,17 @@ public IItemEntryReadOnly Add (string definitionId, int quantity = 1) {
         public IItemEntryReadOnly AddEntry(IItemEntryReadOnly entry) {
             if (entry == null) return null;
 
-            // Unique items and items without existing quantities can be added directly
-            if (entry.Definition.Unique || !Has(entry.Definition)) {
-                AddEntry(entry as IItemEntry);
-                entry.UpdateTimeLogs();
-                return entry;
+            // Unique items and items without existing quantities can be added directly (unless they are read only)
+            if (entry is IItemEntry existingEntry && (existingEntry.Definition.Unique || !Has(existingEntry.Definition))) {
+                AddEntryInternal(existingEntry);
+                existingEntry.UpdateTimeLogs();
+                return existingEntry;
             }
 
             return Add(entry.Definition, entry.Quantity);
         }
 
-        private void AddEntry (IItemEntry entry) {
+        private void AddEntryInternal (IItemEntry entry) {
             if (entry.Definition.Unique) {
                 _uniqueEntries.Add(entry);
             } else {
@@ -160,7 +160,7 @@ public void Load (string save) {
                 var tmpEntry = resolver.Load(json, _database);
                 var entry = tmpEntry.Definition.DataResolver.Load(json, _database);
 
-                AddEntry(entry);
+                AddEntryInternal(entry);
             });
         }
 

diff --git a/Assets/com.fluid.elastic-inventory/Tests/Editor/Scripts/InventoryInstanceTest.cs b/Assets/com.fluid.elastic-inventory/Tests/Editor/Scripts/InventoryInstanceTest.cs
@@ -152,6 +152,18 @@ public void It_should_add_an_item_entry_object () {
                 }
             }
 
+            public class ReadOnly_Items : InventoryInstanceTest {
+                [Test]
+                public void It_should_not_error_when_a_read_only_item_is_passed () {
+                    var inventory = Setup();
+                    var item = A.ItemEntryReadOnly().Build();
+
+                    Assert.DoesNotThrow(() => {
+                        inventory.AddEntry(item);
+                    });
+                }
+            }
+
             public class Unique_Items : InventoryInstanceTest {
                 [Test]
                 public void It_should_add_two_unique_items_as_individual_entries () {