Dynamic merging of dropped items

CHANGELOG.md

@@ -96,6 +96,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Weapons block are based on poise
 - Wooden Shield recipe
 - Overhauled the visuals of several cave biomes
+- Dropped items now merge dynamically (including non-stackables)
 
 ### Removed
 - Medium and large potions from all loot tables

common/net/src/synced_components.rs

@@ -29,7 +29,7 @@ macro_rules! synced_components {
             poise: Poise,
             light_emitter: LightEmitter,
             loot_owner: LootOwner,
-            item: Item,
+            item: PickupItem,
             scale: Scale,
             group: Group,
             is_mount: IsMount,
@@ -166,7 +166,7 @@ impl NetSync for LootOwner {
     const SYNC_FROM: SyncFrom = SyncFrom::AnyEntity;
 }
 
-impl NetSync for Item {
+impl NetSync for PickupItem {
     const SYNC_FROM: SyncFrom = SyncFrom::AnyEntity;
 }
 

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

@@ -12,6 +12,7 @@ use crate::{
     comp::inventory::InvSlot,
     effect::Effect,
     recipe::RecipeInput,
+    resources::ProgramTime,
     terrain::Block,
 };
 use common_i18n::Content;
@@ -474,6 +475,27 @@ pub struct Item {
     durability_lost: Option<u32>,
 }
 
+// An item that is dropped into the world an can be picked up. It can stack with
+// other items of the same type regardless of the stack limit, when picked up
+// the last item from the list is popped
+//
+// NOTE: Never call PickupItem::clone, it is only used for network
+// synchronization
+//
+// Invariants:
+//  - Any item that is not the last one must have an amount equal to its
+//    `max_amount()`
+//  - All items must be equal and have a zero amount of slots
+//  - The Item list must not be empty
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct PickupItem {
+    items: Vec<Item>,
+    /// This [`ProgramTime`] only makes sense on the server
+    created_at: ProgramTime,
+    /// This [`ProgramTime`] only makes sense on the server
+    next_merge_check: ProgramTime,
+}
+
 use std::hash::{Hash, Hasher};
 
 // Used to find inventory item corresponding to hotbar slot
@@ -824,14 +846,13 @@ impl ItemDef {
 /// please don't rely on this for anything!
 impl PartialEq for Item {
     fn eq(&self, other: &Self) -> bool {
-        if let (ItemBase::Simple(self_def), ItemBase::Simple(other_def)) =
+        (match (&self.item_base, &other.item_base) {
-            (&self.item_base, &other.item_base)
+            (ItemBase::Simple(our_def), ItemBase::Simple(other_def)) => {
-        {
+                our_def.item_definition_id == other_def.item_definition_id
-            self_def.item_definition_id == other_def.item_definition_id
+            },
-                && self.components == other.components
+            (ItemBase::Modular(our_base), ItemBase::Modular(other_base)) => our_base == other_base,
-        } else {
+            _ => false,
-            false
+        }) && self.components() == other.components()
-        }
     }
 }
 
@@ -1270,37 +1291,6 @@ 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.can_merge(&other) {
-            self.increase_amount(other.amount())
-                .expect("`can_merge` succeeded but `increase_amount` did not");
-            Ok(())
-        } else {
-            Err(other)
-        }
-    }
-
     pub fn num_slots(&self) -> u16 { self.item_base.num_slots() }
 
     /// NOTE: invariant that amount() ≤ max_amount(), 1 ≤ max_amount(),
@@ -1476,6 +1466,173 @@ impl Item {
             msm,
         )
     }
+
+    /// Checks if this item and another are suitable for grouping into the same
+    /// [`PickItem`].
+    ///
+    /// Also see [`Item::try_merge`].
+    pub fn can_merge(&self, other: &Self) -> bool {
+        if self.amount() > self.max_amount() || other.amount() > other.max_amount() {
+            error!("An item amount is over max_amount!");
+            return false;
+        }
+
+        (self == other)
+            && self.slots().iter().all(Option::is_none)
+            && other.slots().iter().all(Option::is_none)
+            && self.durability_lost() == other.durability_lost()
+    }
+
+    /// Checks if this item and another are suitable for grouping into the same
+    /// [`PickItem`] and combines stackable items if possible.
+    ///
+    /// If the sum of both amounts is larger than their max amount, a remainder
+    /// item is returned as `Ok(Some(remainder))`. A remainder item will
+    /// always be produced for non-stackable items.
+    ///
+    /// If the items are not suitable for grouping `Err(other)` will be
+    /// returned.
+    pub fn try_merge(&mut self, mut other: Self) -> Result<Option<Self>, Self> {
+        if self.can_merge(&other) {
+            let max_amount = self.max_amount();
+            debug_assert_eq!(
+                max_amount,
+                other.max_amount(),
+                "Mergeable items must have the same max_amount()"
+            );
+
+            // Additional amount `self` can hold
+            // For non-stackable items this is always zero
+            let to_fill_self = max_amount
+                .checked_sub(self.amount())
+                .expect("can_merge should ensure that amount() <= max_amount()");
+
+            if let Some(remainder) = other.amount().checked_sub(to_fill_self).filter(|r| *r > 0) {
+                self.set_amount(max_amount)
+                    .expect("max_amount() is always a valid amount.");
+                other.set_amount(remainder).expect(
+                    "We know remainder is more than 0 and less than or equal to max_amount()",
+                );
+                Ok(Some(other))
+            } else {
+                // If there would be no remainder, add the amounts!
+                self.increase_amount(other.amount())
+                    .expect("We know that we can at least add other.amount() to this item");
+                drop(other);
+                Ok(None)
+            }
+        } else {
+            Err(other)
+        }
+    }
+}
+
+impl PickupItem {
+    pub fn new(item: Item, time: ProgramTime) -> Self {
+        Self {
+            items: vec![item],
+            created_at: time,
+            next_merge_check: time,
+        }
+    }
+
+    /// Get a reference to the last item in this stack
+    ///
+    /// The amount of this item should *not* be used.
+    pub fn item(&self) -> &Item {
+        self.items
+            .last()
+            .expect("PickupItem without at least one item is an invariant")
+    }
+
+    pub fn created(&self) -> ProgramTime { self.created_at }
+
+    pub fn next_merge_check(&self) -> ProgramTime { self.next_merge_check }
+
+    pub fn next_merge_check_mut(&mut self) -> &mut ProgramTime { &mut self.next_merge_check }
+
+    // Get the total amount of items in here
+    pub fn amount(&self) -> u32 { self.items.iter().map(Item::amount).sum() }
+
+    /// Remove any debug items if this is a container, used before dropping an
+    /// item from an inventory
+    pub fn remove_debug_items(&mut self) {
+        for item in self.items.iter_mut() {
+            item.slots_mut().iter_mut().for_each(|container_slot| {
+                container_slot
+                    .take_if(|contained_item| matches!(contained_item.quality(), Quality::Debug));
+            });
+        }
+    }
+
+    pub fn can_merge(&self, other: &PickupItem) -> bool {
+        let self_item = self.item();
+        let other_item = other.item();
+
+        self_item.can_merge(other_item)
+    }
+
+    // Attempt to merge another PickupItem into this one, can only fail if
+    // `can_merge` returns false
+    pub fn try_merge(&mut self, mut other: PickupItem) -> Result<(), PickupItem> {
+        if self.can_merge(&other) {
+            // Pop the last item from `self` and `other` to merge them, as only the last
+            // items can have an amount != max_amount()
+            let mut self_last = self
+                .items
+                .pop()
+                .expect("PickupItem without at least one item is an invariant");
+            let other_last = other
+                .items
+                .pop()
+                .expect("PickupItem without at least one item is an invariant");
+
+            // Merge other_last into self_last
+            let merged = self_last
+                .try_merge(other_last)
+                .expect("We know these items can be merged");
+
+            debug_assert!(
+                other
+                    .items
+                    .iter()
+                    .chain(self.items.iter())
+                    .all(|item| item.amount() == item.max_amount()),
+                "All items before the last in `PickupItem` should have a full amount"
+            );
+
+            // We know all items except the last have a full amount, so we can safely append
+            // them here
+            self.items.append(&mut other.items);
+
+            debug_assert!(
+                merged.is_none() || self_last.amount() == self_last.max_amount(),
+                "Merged can only be `Some` if the origin was set to `max_amount()`"
+            );
+
+            // Push the potentially not fully-stacked item at the end
+            self.items.push(self_last);
+
+            // Push the remainder, merged is only `Some` if self_last was set to
+            // `max_amount()`
+            if let Some(remainder) = merged {
+                self.items.push(remainder);
+            }
+
+            Ok(())
+        } else {
+            Err(other)
+        }
+    }
+
+    pub fn pick_up(mut self) -> (Item, Option<Self>) {
+        (
+            self.items
+                .pop()
+                .expect("PickupItem without at least one item is an invariant"),
+            (!self.items.is_empty()).then_some(self),
+        )
+    }
 }
 
 pub fn flatten_counted_items<'a>(
@@ -1603,8 +1760,42 @@ impl ItemDesc for ItemDef {
     fn stats_durability_multiplier(&self) -> DurabilityMultiplier { DurabilityMultiplier(1.0) }
 }
 
-impl Component for Item {
+impl ItemDesc for PickupItem {
-    type Storage = DerefFlaggedStorage<Self, DenseVecStorage<Self>>;
+    fn description(&self) -> &str {
+        #[allow(deprecated)]
+        self.item().description()
+    }
+
+    fn name(&self) -> Cow<str> {
+        #[allow(deprecated)]
+        self.item().name()
+    }
+
+    fn kind(&self) -> Cow<ItemKind> { self.item().kind() }
+
+    fn amount(&self) -> NonZeroU32 {
+        NonZeroU32::new(self.amount()).expect("Item having amount of 0 is invariant")
+    }
+
+    fn quality(&self) -> Quality { self.item().quality() }
+
+    fn num_slots(&self) -> u16 { self.item().num_slots() }
+
+    fn item_definition_id(&self) -> ItemDefinitionId<'_> { self.item().item_definition_id() }
+
+    fn tags(&self) -> Vec<ItemTag> { self.item().tags() }
+
+    fn is_modular(&self) -> bool { self.item().is_modular() }
+
+    fn components(&self) -> &[Item] { self.item().components() }
+
+    fn has_durability(&self) -> bool { self.item().has_durability() }
+
+    fn durability_lost(&self) -> Option<u32> { self.item().durability_lost() }
+
+    fn stats_durability_multiplier(&self) -> DurabilityMultiplier {
+        self.item().stats_durability_multiplier()
+    }
 }
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
@@ -1614,6 +1805,10 @@ impl Component for ItemDrops {
     type Storage = DenseVecStorage<Self>;
 }
 
+impl Component for PickupItem {
+    type Storage = DerefFlaggedStorage<Self, DenseVecStorage<Self>>;
+}
+
 #[derive(Copy, Clone, Debug)]
 pub struct DurabilityMultiplier(pub f32);
 

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

@@ -56,7 +56,7 @@ impl Asset for MaterialStatManifest {
     const EXTENSION: &'static str = "ron";
 }
 
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
 pub enum ModularBase {
     Tool,
 }

common/src/comp/mod.rs

@@ -78,7 +78,7 @@ pub use self::{
             self,
             item_key::ItemKey,
             tool::{self, AbilityItem},
-            Item, ItemConfig, ItemDrops,
+            Item, ItemConfig, ItemDrops, PickupItem,
         },
         slot, CollectFailedReason, Inventory, InventoryUpdate, InventoryUpdateEvent,
     },

common/src/event.rs

@@ -165,7 +165,7 @@ pub struct CreateItemDropEvent {
     pub pos: Pos,
     pub vel: Vel,
     pub ori: Ori,
-    pub item: comp::Item,
+    pub item: comp::PickupItem,
     pub loot_owner: Option<LootOwner>,
 }
 pub struct CreateObjectEvent {
@@ -173,7 +173,7 @@ pub struct CreateObjectEvent {
     pub vel: Vel,
     pub body: comp::object::Body,
     pub object: Option<comp::Object>,
-    pub item: Option<comp::Item>,
+    pub item: Option<comp::PickupItem>,
     pub light_emitter: Option<comp::LightEmitter>,
     pub stats: Option<comp::Stats>,
 }

common/src/lib.rs

@@ -13,7 +13,8 @@
     extend_one,
     arbitrary_self_types,
     int_roundings,
-    hash_extract_if
+    hash_extract_if,
+    option_take_if
 )]
 
 pub use common_assets as assets;

common/state/src/state.rs

@@ -234,7 +234,7 @@ impl State {
         ecs.register::<comp::Poise>();
         ecs.register::<comp::CanBuild>();
         ecs.register::<comp::LightEmitter>();
-        ecs.register::<comp::Item>();
+        ecs.register::<comp::PickupItem>();
         ecs.register::<comp::Scale>();
         ecs.register::<Is<Mount>>();
         ecs.register::<Is<Rider>>();

server/src/cmd.rs

@@ -49,7 +49,7 @@ use common::{
     npc::{self, get_npc_name},
     outcome::Outcome,
     parse_cmd_args,
-    resources::{BattleMode, PlayerPhysicsSettings, Secs, Time, TimeOfDay, TimeScale},
+    resources::{BattleMode, PlayerPhysicsSettings, ProgramTime, Secs, Time, TimeOfDay, TimeScale},
     rtsim::{Actor, Role},
     terrain::{Block, BlockKind, CoordinateConversions, SpriteKind, TerrainChunkSize},
     tether::Tethered,
@@ -494,7 +494,7 @@ fn handle_drop_all(
             )),
             comp::Ori::default(),
             comp::Vel(vel),
-            item,
+            comp::PickupItem::new(item, ProgramTime(server.state.get_program_time())),
             None,
         );
     }

server/src/events/entity_manipulation.rs

@@ -21,8 +21,9 @@ use common::{
         inventory::item::{AbilityMap, MaterialStatManifest},
         item::flatten_counted_items,
         loot_owner::LootOwnerKind,
-        Alignment, Auras, Body, CharacterState, Energy, Group, Health, Inventory, Object, Player,
+        Alignment, Auras, Body, CharacterState, Energy, Group, Health, Inventory, Object,
-        Poise, Pos, Presence, PresenceKind, SkillSet, Stats, BASE_ABILITY_LIMIT,
+        PickupItem, Player, Poise, Pos, Presence, PresenceKind, SkillSet, Stats,
+        BASE_ABILITY_LIMIT,
     },
     consts::TELEPORTER_RADIUS,
     event::{
@@ -40,7 +41,7 @@ use common::{
     lottery::distribute_many,
     mounting::{Rider, VolumeRider},
     outcome::{HealthChangeInfo, Outcome},
-    resources::{Secs, Time},
+    resources::{ProgramTime, Secs, Time},
     rtsim::{Actor, RtSimEntity},
     spiral::Spiral2d,
     states::utils::StageSection,
@@ -286,6 +287,7 @@ pub struct DestroyEventData<'a> {
     msm: ReadExpect<'a, MaterialStatManifest>,
     ability_map: ReadExpect<'a, AbilityMap>,
     time: Read<'a, Time>,
+    program_time: ReadExpect<'a, ProgramTime>,
     world: ReadExpect<'a, Arc<World>>,
     index: ReadExpect<'a, world::IndexOwned>,
     areas_container: Read<'a, AreasContainer<NoDurabilityArea>>,
@@ -641,7 +643,7 @@ impl ServerEvent for DestroyEvent {
                                 vel: vel.copied().unwrap_or(comp::Vel(Vec3::zero())),
                                 // TODO: Random
                                 ori: comp::Ori::from(Dir::random_2d(&mut rng)),
-                                item,
+                                item: PickupItem::new(item, *data.program_time),
                                 loot_owner: if let Some(loot_owner) = loot_owner {
                                     debug!(
                                         "Assigned UID {loot_owner:?} as the winner for the loot \
@@ -1432,12 +1434,13 @@ impl ServerEvent for BonkEvent {
     type SystemData<'a> = (
         Write<'a, BlockChange>,
         ReadExpect<'a, TerrainGrid>,
+        ReadExpect<'a, ProgramTime>,
         Read<'a, EventBus<CreateObjectEvent>>,
     );
 
     fn handle(
         events: impl ExactSizeIterator<Item = Self>,
-        (mut block_change, terrain, create_object_events): Self::SystemData<'_>,
+        (mut block_change, terrain, program_time, create_object_events): Self::SystemData<'_>,
     ) {
         let mut create_object_emitter = create_object_events.emitter();
         for ev in events {
@@ -1474,7 +1477,7 @@ impl ServerEvent for BonkEvent {
                                         },
                                         _ => None,
                                     },
-                                    item: Some(item),
+                                    item: Some(comp::PickupItem::new(item, *program_time)),
                                     light_emitter: None,
                                     stats: None,
                                 });

server/src/events/interaction.rs

@@ -20,6 +20,7 @@ use common::{
     link::Is,
     mounting::Mount,
     outcome::Outcome,
+    resources::ProgramTime,
     terrain::{Block, SpriteKind, TerrainGrid},
     uid::Uid,
     util::Dir,
@@ -194,6 +195,7 @@ impl ServerEvent for MineBlockEvent {
         ReadExpect<'a, AbilityMap>,
         ReadExpect<'a, EventBus<CreateItemDropEvent>>,
         ReadExpect<'a, EventBus<Outcome>>,
+        ReadExpect<'a, ProgramTime>,
         WriteStorage<'a, comp::SkillSet>,
         ReadStorage<'a, Uid>,
     );
@@ -207,6 +209,7 @@ impl ServerEvent for MineBlockEvent {
             ability_map,
             create_item_drop_events,
             outcomes,
+            program_time,
             mut skill_sets,
             uids,
         ): Self::SystemData<'_>,
@@ -296,7 +299,7 @@ impl ServerEvent for MineBlockEvent {
                                 pos: comp::Pos(ev.pos.map(|e| e as f32) + Vec3::new(0.5, 0.5, 0.0)),
                                 vel: comp::Vel(Vec3::zero()),
                                 ori: comp::Ori::from(Dir::random_2d(&mut rng)),
-                                item,
+                                item: comp::PickupItem::new(item, *program_time),
                                 loot_owner,
                             });
                         }

server/src/events/inventory_manip.rs

@@ -14,7 +14,7 @@ use common::{
         item::{self, flatten_counted_items, tool::AbilityMap, MaterialStatManifest},
         loot_owner::LootOwnerKind,
         slot::{self, Slot},
-        InventoryUpdate, LootOwner,
+        InventoryUpdate, LootOwner, PickupItem,
     },
     consts::MAX_PICKUP_RANGE,
     event::{
@@ -26,7 +26,7 @@ use common::{
     recipe::{
         self, default_component_recipe_book, default_recipe_book, default_repair_recipe_book,
     },
-    resources::Time,
+    resources::{ProgramTime, Time},
     terrain::{Block, SpriteKind},
     trade::Trades,
     uid::{IdMaps, Uid},
@@ -82,10 +82,11 @@ pub struct InventoryManipData<'a> {
     terrain: ReadExpect<'a, common::terrain::TerrainGrid>,
     id_maps: Read<'a, IdMaps>,
     time: Read<'a, Time>,
+    program_time: ReadExpect<'a, ProgramTime>,
     ability_map: ReadExpect<'a, AbilityMap>,
     msm: ReadExpect<'a, MaterialStatManifest>,
     inventories: WriteStorage<'a, comp::Inventory>,
-    items: WriteStorage<'a, comp::Item>,
+    items: WriteStorage<'a, comp::PickupItem>,
     inventory_updates: WriteStorage<'a, comp::InventoryUpdate>,
     light_emitters: WriteStorage<'a, comp::LightEmitter>,
     positions: ReadStorage<'a, comp::Pos>,
@@ -234,6 +235,12 @@ impl ServerEvent for InventoryManipEvent {
                         continue;
                     };
 
+                    const ITEM_ENTITY_EXPECT_MESSAGE: &str = "We know item_entity still exist \
+                                                              since we just successfully removed \
+                                                              its PickupItem component.";
+
+                    let (item, reinsert_item) = item.pick_up();
+
                     // NOTE: We dup the item for message purposes.
                     let item_msg = item.duplicate(&data.ability_map, &data.msm);
 
@@ -244,13 +251,25 @@ impl ServerEvent for InventoryManipEvent {
                         inventory.pickup_item(returned_item)
                     }) {
                         Err(returned_item) => {
+                            // If we had a `reinsert_item`, merge returned_item into it
+                            let returned_item = if let Some(mut reinsert_item) = reinsert_item {
+                                reinsert_item
+                                    .try_merge(PickupItem::new(returned_item, *data.program_time))
+                                    .expect(
+                                        "We know this item must be mergeable since it is a \
+                                         duplicate",
+                                    );
+                                reinsert_item
+                            } else {
+                                PickupItem::new(returned_item, *data.program_time)
+                            };
+
                             // Inventory was full, so we need to put back the item (note that we
                             // know there was no old item component for
                             // this entity).
-                            data.items.insert(item_entity, returned_item).expect(
+                            data.items
-                                "We know item_entity exists since we just successfully removed \
+                                .insert(item_entity, returned_item)
-                                 its Item component.",
+                                .expect(ITEM_ENTITY_EXPECT_MESSAGE);
-                            );
                             comp::InventoryUpdate::new(InventoryUpdateEvent::EntityCollectFailed {
                                 entity: pickup_uid,
                                 reason: CollectFailedReason::InventoryFull,
@@ -259,7 +278,14 @@ impl ServerEvent for InventoryManipEvent {
                         Ok(_) => {
                             // We succeeded in picking up the item, so we may now delete its old
                             // entity entirely.
-                            emitters.emit(DeleteEvent(item_entity));
+                            if let Some(reinsert_item) = reinsert_item {
+                                data.items
+                                    .insert(item_entity, reinsert_item)
+                                    .expect(ITEM_ENTITY_EXPECT_MESSAGE);
+                            } else {
+                                emitters.emit(DeleteEvent(item_entity));
+                            }
+
                             if let Some(group_id) = data.groups.get(entity) {
                                 announce_loot_to_group(
                                     group_id,
@@ -415,7 +441,7 @@ impl ServerEvent for InventoryManipEvent {
                             ),
                             vel: comp::Vel(Vec3::zero()),
                             ori: data.orientations.get(entity).copied().unwrap_or_default(),
-                            item,
+                            item: PickupItem::new(item, *data.program_time),
                             loot_owner: Some(LootOwner::new(LootOwnerKind::Player(*uid), false)),
                         });
                     }
@@ -445,14 +471,14 @@ impl ServerEvent for InventoryManipEvent {
                                 }
                                 if let Some(pos) = data.positions.get(entity) {
                                     dropped_items.extend(
-                                        inventory.equip(slot, *data.time).into_iter().map(|x| {
+                                        inventory.equip(slot, *data.time).into_iter().map(|item| {
                                             (
                                                 *pos,
                                                 data.orientations
                                                     .get(entity)
                                                     .copied()
                                                     .unwrap_or_default(),
-                                                x,
+                                                PickupItem::new(item, *data.program_time),
                                                 *uid,
                                             )
                                         }),
@@ -567,14 +593,14 @@ impl ServerEvent for InventoryManipEvent {
                                 if let Ok(Some(leftover_items)) =
                                     inventory.unequip(slot, *data.time)
                                 {
-                                    dropped_items.extend(leftover_items.into_iter().map(|x| {
+                                    dropped_items.extend(leftover_items.into_iter().map(|item| {
                                         (
                                             *pos,
                                             data.orientations
                                                 .get(entity)
                                                 .copied()
                                                 .unwrap_or_default(),
-                                            x,
+                                            PickupItem::new(item, *data.program_time),
                                             *uid,
                                         )
                                     }));
@@ -671,11 +697,11 @@ impl ServerEvent for InventoryManipEvent {
                         // If the stacks weren't mergable carry out a swap.
                         if !merged_stacks {
                             dropped_items.extend(inventory.swap(a, b, *data.time).into_iter().map(
-                                |x| {
+                                |item| {
                                     (
                                         *pos,
                                         data.orientations.get(entity).copied().unwrap_or_default(),
-                                        x,
+                                        PickupItem::new(item, *data.program_time),
                                         *uid,
                                     )
                                 },
@@ -743,7 +769,7 @@ impl ServerEvent for InventoryManipEvent {
                         dropped_items.push((
                             *pos,
                             data.orientations.get(entity).copied().unwrap_or_default(),
-                            item,
+                            PickupItem::new(item, *data.program_time),
                             *uid,
                         ));
                     }
@@ -771,7 +797,7 @@ impl ServerEvent for InventoryManipEvent {
                         dropped_items.push((
                             *pos,
                             data.orientations.get(entity).copied().unwrap_or_default(),
-                            item,
+                            PickupItem::new(item, *data.program_time),
                             *uid,
                         ));
                     }
@@ -949,18 +975,35 @@ impl ServerEvent for InventoryManipEvent {
                     // Attempt to insert items into inventory, dropping them if there is not enough
                     // space
                     let items_were_crafted = if let Some(crafted_items) = crafted_items {
+                        let mut dropped: Vec<PickupItem> = Vec::new();
                         for item in crafted_items {
                             if let Err(item) = inventory.push(item) {
-                                if let Some(pos) = data.positions.get(entity) {
+                                let item = PickupItem::new(item, *data.program_time);
-                                    dropped_items.push((
+                                if let Some(can_merge) =
-                                        *pos,
+                                    dropped.iter_mut().find(|other| other.can_merge(&item))
-                                        data.orientations.get(entity).copied().unwrap_or_default(),
+                                {
-                                        item.duplicate(&data.ability_map, &data.msm),
+                                    can_merge
-                                        *uid,
+                                        .try_merge(item)
-                                    ));
+                                        .expect("We know these items can be merged");
+                                } else {
+                                    dropped.push(item);
                                 }
                             }
                         }
+
+                        if !dropped.is_empty()
+                            && let Some(pos) = data.positions.get(entity)
+                        {
+                            for item in dropped {
+                                dropped_items.push((
+                                    *pos,
+                                    data.orientations.get(entity).copied().unwrap_or_default(),
+                                    item,
+                                    *uid,
+                                ));
+                            }
+                        }
+
                         true
                     } else {
                         false
@@ -990,16 +1033,9 @@ impl ServerEvent for InventoryManipEvent {
         // Drop items, Debug items should simply disappear when dropped
         for (pos, ori, mut item, owner) in dropped_items
             .into_iter()
-            .filter(|(_, _, i, _)| !matches!(i.quality(), item::Quality::Debug))
+            .filter(|(_, _, i, _)| !matches!(i.item().quality(), item::Quality::Debug))
         {
-            // If item is a container check inside of it for Debug items and remove them
+            item.remove_debug_items();
-            item.slots_mut().iter_mut().for_each(|x| {
-                if let Some(contained_item) = &x {
-                    if matches!(contained_item.quality(), item::Quality::Debug) {
-                        std::mem::take(x);
-                    }
-                }
-            });
 
             emitters.emit(CreateItemDropEvent {
                 pos,

server/src/state_ext.rs

@@ -22,7 +22,7 @@ use common::{
         misc::PortalData,
         object,
         skills::{GeneralSkill, Skill},
-        ChatType, Content, Group, Inventory, Item, LootOwner, Object, Player, Poise, Presence,
+        ChatType, Content, Group, Inventory, LootOwner, Object, Player, Poise, Presence,
         PresenceKind, BASE_ABILITY_LIMIT,
     },
     effect::Effect,
@@ -74,7 +74,7 @@ pub trait StateExt {
         pos: comp::Pos,
         ori: comp::Ori,
         vel: comp::Vel,
-        item: Item,
+        item: comp::PickupItem,
         loot_owner: Option<LootOwner>,
     ) -> Option<EcsEntity>;
     fn create_ship<F: FnOnce(comp::ship::Body) -> comp::Collider>(
@@ -342,54 +342,44 @@ impl StateExt for State {
         pos: comp::Pos,
         ori: comp::Ori,
         vel: comp::Vel,
-        item: Item,
+        world_item: comp::PickupItem,
         loot_owner: Option<LootOwner>,
     ) -> Option<EcsEntity> {
+        // Attempt merging with any nearby entities if possible
         {
-            const MAX_MERGE_DIST: f32 = 1.5;
+            use crate::sys::item::get_nearby_mergeable_items;
 
-            // First, try to identify possible candidates for item merging
-            // We limit our search to just a few blocks and we prioritise merging with the
-            // closest
             let positions = self.ecs().read_storage::<comp::Pos>();
             let loot_owners = self.ecs().read_storage::<LootOwner>();
-            let mut items = self.ecs().write_storage::<Item>();
+            let mut items = self.ecs().write_storage::<comp::PickupItem>();
-            let mut nearby_items = self
+            let entities = self.ecs().entities();
-                .ecs()
+            let spatial_grid = self.ecs().read_resource();
-                .read_resource::<common::CachedSpatialGrid>()
+
-                .0
+            let nearby_items = get_nearby_mergeable_items(
-                .in_circle_aabr(pos.0.xy(), MAX_MERGE_DIST)
+                &world_item,
-                .filter(|entity| items.contains(*entity))
+                &pos,
-                .filter_map(|entity| {
+                loot_owner.as_ref(),
-                    Some((entity, positions.get(entity)?.0.distance_squared(pos.0)))
+                (&entities, &items, &positions, &loot_owners, &spatial_grid),
-                })
+            );
-                .filter(|(_, dist_sqrd)| *dist_sqrd < MAX_MERGE_DIST.powi(2))
+
-                .collect::<Vec<_>>();
+            // Merge the nearest item if possible, skip to creating a drop otherwise
-            nearby_items.sort_by_key(|(_, dist_sqrd)| (dist_sqrd * 1000.0) as i32);
+            if let Some((mergeable_item, _)) =
-            for (nearby, _) in nearby_items {
+                nearby_items.min_by_key(|(_, dist)| (dist * 1000.0) as i32)
-                // Only merge if the loot owner is the same
+            {
-                if loot_owners.get(nearby).map(|lo| lo.owner()) == loot_owner.map(|lo| lo.owner())
+                items
-                    && items
+                    .get_mut(mergeable_item)
-                        .get(nearby)
+                    .expect("we know that the item exists")
-                        .map_or(false, |nearby_item| nearby_item.can_merge(&item))
+                    .try_merge(world_item)
-                {
+                    .expect("`try_merge` should succeed because `can_merge` returned `true`");
-                    // Merging can occur! Perform the merge:
+                return None;
-                    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;
-                }
             }
-            // Only if merging items fails do we give up and create a new item
         }
 
         let spawned_at = *self.ecs().read_resource::<Time>();
 
-        let item_drop = comp::item_drop::Body::from(&item);
+        let item_drop = comp::item_drop::Body::from(world_item.item());
         let body = comp::Body::ItemDrop(item_drop);
-        let light_emitter = match &*item.kind() {
+        let light_emitter = match &*world_item.item().kind() {
             ItemKind::Lantern(lantern) => Some(comp::LightEmitter {
                 col: lantern.color(),
                 strength: lantern.strength(),
@@ -401,7 +391,7 @@ impl StateExt for State {
         Some(
             self.ecs_mut()
                 .create_entity_synced()
-                .with(item)
+                .with(world_item)
                 .with(pos)
                 .with(ori)
                 .with(vel)

server/src/sys/item.rs

@@ -0,0 +1,158 @@
+use std::collections::HashMap;
+
+use common::{
+    comp,
+    event::{DeleteEvent, EventBus},
+    resources::ProgramTime,
+    CachedSpatialGrid,
+};
+use common_ecs::{Origin, Phase, System};
+use specs::{Entities, Entity, Join, LendJoin, Read, ReadStorage, WriteStorage};
+
+const MAX_ITEM_MERGE_DIST: f32 = 2.0;
+const CHECKS_PER_SECOND: f64 = 10.0; // Start by checking an item 10 times every second
+
+#[derive(Default)]
+pub struct Sys;
+
+impl<'a> System<'a> for Sys {
+    type SystemData = (
+        Entities<'a>,
+        WriteStorage<'a, comp::PickupItem>,
+        ReadStorage<'a, comp::Pos>,
+        ReadStorage<'a, comp::LootOwner>,
+        Read<'a, CachedSpatialGrid>,
+        Read<'a, ProgramTime>,
+        Read<'a, EventBus<DeleteEvent>>,
+    );
+
+    const NAME: &'static str = "item";
+    const ORIGIN: Origin = Origin::Server;
+    const PHASE: Phase = Phase::Create;
+
+    fn run(
+        _job: &mut common_ecs::Job<Self>,
+        (
+            entities,
+            mut items,
+            positions,
+            loot_owners,
+            spatial_grid,
+            program_time,
+            delete_bus,
+        ): Self::SystemData,
+    ) {
+        // Contains items that have been checked for merge, or that were merged into
+        // another one
+        let mut merged = HashMap::new();
+        // Contains merges that will be performed (from, into)
+        let mut merges = Vec::new();
+        // Delete events are emitted when this is dropped
+        let mut delete_emitter = delete_bus.emitter();
+
+        for (entity, item, pos, loot_owner) in
+            (&entities, &items, &positions, loot_owners.maybe()).join()
+        {
+            // Do not process items that are already being merged
+            if merged.contains_key(&entity) {
+                continue;
+            }
+
+            // Exponentially back of the frequency at which items are checked for merge
+            if program_time.0 < item.next_merge_check().0 {
+                continue;
+            }
+
+            // We do not want to allow merging this item if it isn't already being
+            // merged into another
+            merged.insert(entity, true);
+
+            for (source_entity, _) in get_nearby_mergeable_items(
+                item,
+                pos,
+                loot_owner,
+                (&entities, &items, &positions, &loot_owners, &spatial_grid),
+            ) {
+                // Prevent merging an item multiple times, we cannot
+                // do this in the above filter since we mutate `merged` below
+                if merged.contains_key(&source_entity) {
+                    continue;
+                }
+
+                // Do not merge items multiple times
+                merged.insert(source_entity, false);
+                // Defer the merge
+                merges.push((source_entity, entity));
+            }
+        }
+
+        for (source, target) in merges {
+            let source_item = items
+                .remove(source)
+                .expect("We know this entity must have an item.");
+            let mut target_item = items
+                .get_mut(target)
+                .expect("We know this entity must have an item.");
+
+            if let Err(item) = target_item.try_merge(source_item) {
+                // We re-insert the item, should be unreachable since we already checked whether
+                // the items were mergeable in the above loop
+                items
+                    .insert(source, item)
+                    .expect("PickupItem was removed from this entity earlier");
+            } else {
+                // If the merging was successfull, we remove the old item entity from the ECS
+                delete_emitter.emit(DeleteEvent(source));
+            }
+        }
+
+        for updated in merged
+            .into_iter()
+            .filter_map(|(entity, is_merge_parent)| is_merge_parent.then_some(entity))
+        {
+            if let Some(mut item) = items.get_mut(updated) {
+                item.next_merge_check_mut().0 +=
+                    (program_time.0 - item.created().0).max(1.0 / CHECKS_PER_SECOND);
+            }
+        }
+    }
+}
+
+pub fn get_nearby_mergeable_items<'a>(
+    item: &'a comp::PickupItem,
+    pos: &'a comp::Pos,
+    loot_owner: Option<&'a comp::LootOwner>,
+    (entities, items, positions, loot_owners, spatial_grid): (
+        &'a Entities<'a>,
+        // We do not actually need write access here, but currently all callers of this function
+        // have a WriteStorage<Item> in scope which we cannot *downcast* into a ReadStorage
+        &'a WriteStorage<'a, comp::PickupItem>,
+        &'a ReadStorage<'a, comp::Pos>,
+        &'a ReadStorage<'a, comp::LootOwner>,
+        &'a CachedSpatialGrid,
+    ),
+) -> impl Iterator<Item = (Entity, f32)> + 'a {
+    // Get nearby items
+    spatial_grid
+        .0
+        .in_circle_aabr(pos.0.xy(), MAX_ITEM_MERGE_DIST)
+        // Filter out any unrelated entities
+        .flat_map(move |entity| {
+            (entities, items, positions, loot_owners.maybe())
+                .lend_join()
+                .get(entity, entities)
+                .and_then(|(entity, item, other_position, loot_owner)| {
+                    let distance_sqrd = other_position.0.distance_squared(pos.0);
+                    if distance_sqrd < MAX_ITEM_MERGE_DIST.powi(2) {
+                        Some((entity, item, distance_sqrd, loot_owner))
+                    } else {
+                        None
+                    }
+                })
+        })
+        // Filter by "mergeability"
+        .filter_map(move |(entity, other_item, distance, other_loot_owner)| {
+            (other_loot_owner.map(|owner| owner.owner()) == loot_owner.map(|owner| owner.owner())
+                && item.can_merge(other_item)).then_some((entity, distance))
+        })
+}

server/src/sys/mod.rs

@@ -3,6 +3,7 @@ pub mod chunk_send;
 pub mod chunk_serialize;
 pub mod entity_sync;
 pub mod invite_timeout;
+pub mod item;
 pub mod loot;
 pub mod metrics;
 pub mod msg;
@@ -42,6 +43,7 @@ pub fn add_server_systems(dispatch_builder: &mut DispatcherBuilder) {
     dispatch::<chunk_serialize::Sys>(dispatch_builder, &[]);
     // don't depend on chunk_serialize, as we assume everything is done in a SlowJow
     dispatch::<chunk_send::Sys>(dispatch_builder, &[]);
+    dispatch::<item::Sys>(dispatch_builder, &[]);
 }
 
 pub fn run_sync_systems(ecs: &mut specs::World) {

voxygen/src/hud/mod.rs

@@ -102,13 +102,13 @@ use common::{
         loot_owner::LootOwnerKind,
         pet::is_mountable,
         skillset::{skills::Skill, SkillGroupKind, SkillsPersistenceError},
-        BuffData, BuffKind, Health, Item, MapMarkerChange, PresenceKind,
+        BuffData, BuffKind, Health, Item, MapMarkerChange, PickupItem, PresenceKind,
     },
     consts::MAX_PICKUP_RANGE,
     link::Is,
     mounting::{Mount, Rider, VolumePos},
     outcome::Outcome,
-    resources::{Secs, Time},
+    resources::{ProgramTime, Secs, Time},
     slowjob::SlowJobPool,
     terrain::{SpriteKind, TerrainChunk, UnlockKind},
     trade::{ReducedInventory, TradeAction},
@@ -1495,7 +1495,7 @@ impl Hud {
             let interpolated = ecs.read_storage::<vcomp::Interpolated>();
             let scales = ecs.read_storage::<comp::Scale>();
             let bodies = ecs.read_storage::<comp::Body>();
-            let items = ecs.read_storage::<Item>();
+            let items = ecs.read_storage::<PickupItem>();
             let inventories = ecs.read_storage::<comp::Inventory>();
             let msm = ecs.read_resource::<MaterialStatManifest>();
             let entities = ecs.entities();
@@ -1992,8 +1992,8 @@ impl Hud {
             let pulse = self.pulse;
 
             let make_overitem =
-                |item: &Item, pos, distance, properties, fonts, interaction_options| {
+                |item: &PickupItem, pos, distance, properties, fonts, interaction_options| {
-                    let quality = get_quality_col(item);
+                    let quality = get_quality_col(item.item());
 
                     // Item
                     overitem::Overitem::new(
@@ -2201,7 +2201,7 @@ impl Hud {
                     item.set_amount(amount.clamp(1, item.max_amount()))
                         .expect("amount >= 1 and <= max_amount is always a valid amount");
                     make_overitem(
-                        &item,
+                        &PickupItem::new(item, ProgramTime(0.0)),
                         over_pos,
                         pos.distance_squared(player_pos),
                         overitem_properties,

voxygen/src/scene/figure/mod.rs

@@ -40,8 +40,8 @@ use common::{
         item::{armor::ArmorKind, Hands, ItemKind, ToolKind},
         ship::{self, figuredata::VOXEL_COLLIDER_MANIFEST},
         slot::ArmorSlot,
-        Body, CharacterActivity, CharacterState, Collider, Controller, Health, Inventory, Item,
+        Body, CharacterActivity, CharacterState, Collider, Controller, Health, Inventory, ItemKey,
-        ItemKey, Last, LightAnimation, LightEmitter, Object, Ori, PhysicsState, PoiseState, Pos,
+        Last, LightAnimation, LightEmitter, Object, Ori, PhysicsState, PickupItem, PoiseState, Pos,
         Scale, Vel,
     },
     link::Is,
@@ -889,7 +889,7 @@ impl FigureMgr {
             &ecs.read_storage::<PhysicsState>(),
             ecs.read_storage::<Health>().maybe(),
             ecs.read_storage::<Inventory>().maybe(),
-            ecs.read_storage::<Item>().maybe(),
+            ecs.read_storage::<PickupItem>().maybe(),
             ecs.read_storage::<LightEmitter>().maybe(),
             (
                 ecs.read_storage::<Is<Rider>>().maybe(),
@@ -6361,7 +6361,7 @@ impl FigureMgr {
                     );
                 },
                 Body::ItemDrop(body) => {
-                    let item_key = item.map(ItemKey::from);
+                    let item_key = item.map(|item| ItemKey::from(item.item()));
                     let (model, skeleton_attr) = self.item_drop_model_cache.get_or_create_model(
                         renderer,
                         &mut self.atlas,
@@ -6558,7 +6558,7 @@ impl FigureMgr {
     ) {
         let ecs = state.ecs();
         let time = ecs.read_resource::<Time>();
-        let items = ecs.read_storage::<Item>();
+        let items = ecs.read_storage::<PickupItem>();
         (
                 &ecs.entities(),
                 &ecs.read_storage::<Pos>(),
@@ -6591,7 +6591,7 @@ impl FigureMgr {
                         _ => 0,
                     },
                     &filter_state,
-                    if matches!(body, Body::ItemDrop(_)) { items.get(entity).map(ItemKey::from) } else { None },
+                    if matches!(body, Body::ItemDrop(_)) { items.get(entity).map(|item| ItemKey::from(item.item())) } else { None },
                 ) {
                     drawer.draw(model, bound);
                 }
@@ -6734,7 +6734,7 @@ impl FigureMgr {
         let time = ecs.read_resource::<Time>();
         let character_state_storage = state.read_storage::<CharacterState>();
         let character_state = character_state_storage.get(viewpoint_entity);
-        let items = ecs.read_storage::<Item>();
+        let items = ecs.read_storage::<PickupItem>();
         for (entity, pos, body, _, inventory, scale, collider, _) in (
             &ecs.entities(),
             &ecs.read_storage::<Pos>(),
@@ -6769,7 +6769,7 @@ impl FigureMgr {
                 },
                 |state| state.visible(),
                 if matches!(body, Body::ItemDrop(_)) {
-                    items.get(entity).map(ItemKey::from)
+                    items.get(entity).map(|item| ItemKey::from(item.item()))
                 } else {
                     None
                 },
@@ -6792,7 +6792,7 @@ impl FigureMgr {
 
         let character_state_storage = state.read_storage::<CharacterState>();
         let character_state = character_state_storage.get(viewpoint_entity);
-        let items = ecs.read_storage::<Item>();
+        let items = ecs.read_storage::<PickupItem>();
 
         if let (Some(pos), Some(body), scale) = (
             ecs.read_storage::<Pos>().get(viewpoint_entity),
@@ -6822,7 +6822,9 @@ impl FigureMgr {
                 0,
                 |state| state.visible(),
                 if matches!(body, Body::ItemDrop(_)) {
-                    items.get(viewpoint_entity).map(ItemKey::from)
+                    items
+                        .get(viewpoint_entity)
+                        .map(|item| ItemKey::from(item.item()))
                 } else {
                     None
                 },

voxygen/src/session/interactable.rs

@@ -207,7 +207,7 @@ pub(super) fn select_interactable(
         let is_mounts = ecs.read_storage::<Is<Mount>>();
         let is_riders = ecs.read_storage::<Is<Rider>>();
         let bodies = ecs.read_storage::<comp::Body>();
-        let items = ecs.read_storage::<comp::Item>();
+        let items = ecs.read_storage::<comp::PickupItem>();
         let stats = ecs.read_storage::<comp::Stats>();
 
         let player_char_state = char_states.get(player_entity);

voxygen/src/session/mod.rs

@@ -1074,7 +1074,7 @@ impl PlayState for SessionState {
                                                 if client
                                                     .state()
                                                     .ecs()
-                                                    .read_storage::<comp::Item>()
+                                                    .read_storage::<comp::PickupItem>()
                                                     .get(*entity)
                                                     .is_some()
                                                 {

voxygen/src/session/target.rs

@@ -129,7 +129,7 @@ pub(super) fn targets_under_cursor(
         &positions,
         scales.maybe(),
         &ecs.read_storage::<comp::Body>(),
-        ecs.read_storage::<comp::Item>().maybe(),
+        ecs.read_storage::<comp::PickupItem>().maybe(),
         !&ecs.read_storage::<Is<Mount>>(),
         ecs.read_storage::<Is<Rider>>().maybe(),
     )