Addressed comments

common/src/comp/inventory/item/mod.rs

@@ -1158,15 +1158,31 @@ impl Item {
         }
     }
 
+    /// Return `true` if `other` can be merged into this item. This is generally
+    /// only possible if the item has a compatible item ID and is stackable,
+    /// along with any other similarity checks.
+    pub fn can_merge(&self, other: &Item) -> bool {
+        if self.is_stackable()
+            && let ItemBase::Simple(other_item_def) = &other.item_base
+            && self.is_same_item_def(other_item_def)
+            && u32::from(self.amount)
+                .checked_add(other.amount())
+                .filter(|&amount| amount <= self.max_amount())
+                .is_some()
+        {
+            true
+        } else {
+            false
+        }
+    }
+
     /// Try to merge `other` into this item. This is generally only possible if
     /// the item has a compatible item ID and is stackable, along with any
     /// other similarity checks.
     pub fn try_merge(&mut self, other: Item) -> Result<(), Item> {
-        if self.is_stackable()
+        if self.can_merge(&other) {
-            && let ItemBase::Simple(other_item_def) = &other.item_base
+            self.increase_amount(other.amount())
-            && self.is_same_item_def(other_item_def)
+                .expect("`can_merge` succeeded but `increase_amount` did not");
-        {
-            self.increase_amount(other.amount()).map_err(|_| other)?;
             Ok(())
         } else {
             Err(other)

server/src/state_ext.rs

@@ -323,7 +323,7 @@ impl StateExt for State {
         &mut self,
         pos: comp::Pos,
         vel: comp::Vel,
-        mut item: Item,
+        item: Item,
         loot_owner: Option<LootOwner>,
     ) -> Option<EcsEntity> {
         {
@@ -340,7 +340,7 @@ impl StateExt for State {
                 .read_resource::<common::CachedSpatialGrid>()
                 .0
                 .in_circle_aabr(pos.0.xy(), MAX_MERGE_DIST)
-                .filter(|entity| items.get(*entity).is_some())
+                .filter(|entity| items.contains(*entity))
                 .filter_map(|entity| {
                     Some((entity, positions.get(entity)?.0.distance_squared(pos.0)))
                 })
@@ -350,11 +350,19 @@ impl StateExt for State {
             for (nearby, _) in nearby_items {
                 // Only merge if the loot owner is the same
                 if loot_owners.get(nearby).map(|lo| lo.owner()) == loot_owner.map(|lo| lo.owner()) {
-                    if let Some(mut nearby_item) = items.get_mut(nearby) {
+                    if items
-                        match nearby_item.try_merge(item) {
+                        .get(nearby)
-                            Ok(()) => return None, // Merging was successful!
+                        .map_or(false, |nearby_item| nearby_item.can_merge(&item))
-                            Err(rejected_item) => item = rejected_item,
+                    {
-                        }
+                        // Merging can occur! Perform the merge:
+                        items
+                            .get_mut(nearby)
+                            .expect("we know that the item exists")
+                            .try_merge(item)
+                            .expect(
+                                "`try_merge` should succeed because `can_merge` returned `true`",
+                            );
+                        return None;
                     }
                 }
             }