Merge branch 'termac/inventory_collect_when_full_fix' into 'master'

Fix bug collecting items into full inventory

See merge request veloren/veloren!1333

common/src/state.rs

@@ -249,10 +249,14 @@ impl State {
         self.ecs.write_resource::<BlockChange>().set(pos, block);
     }
 
-    /// Set a block in this state's terrain. Will return `None` if the block has
+    /// Check if the block at given position `pos` has already been modified
-    /// already been modified this tick.
+    /// this tick.
-    pub fn try_set_block(&mut self, pos: Vec3<i32>, block: Block) -> Option<()> {
+    pub fn can_set_block(&mut self, pos: Vec3<i32>) -> bool {
-        self.ecs.write_resource::<BlockChange>().try_set(pos, block)
+        !self
+            .ecs
+            .write_resource::<BlockChange>()
+            .blocks
+            .contains_key(&pos)
     }
 
     /// Removes every chunk of the terrain.

server/src/events/inventory_manip.rs

@@ -100,23 +100,54 @@ pub fn handle_inventory(server: &mut Server, entity: EcsEntity, manip: comp::Inv
             let block = state.terrain().get(pos).ok().copied();
 
             if let Some(block) = block {
-                let has_inv_space = state
+                if block.is_collectible() && state.can_set_block(pos) {
-                    .ecs()
+                    if let Some(item) = comp::Item::try_reclaim_from_block(block) {
-                    .read_storage::<comp::Inventory>()
+                        let (event, item_was_added) = if let Some(inv) = state
-                    .get(entity)
+                            .ecs()
-                    .map(|inv| !inv.is_full())
+                            .write_storage::<comp::Inventory>()
-                    .unwrap_or(false);
+                            .get_mut(entity)
-
+                        {
-                if !has_inv_space {
+                            match inv.push(item.clone()) {
-                    state.write_component(
+                                None => (
-                        entity,
+                                    Some(comp::InventoryUpdate::new(
-                        comp::InventoryUpdate::new(comp::InventoryUpdateEvent::CollectFailed),
+                                        comp::InventoryUpdateEvent::Collected(item),
+                                    )),
+                                    true,
+                                ),
+                                Some(_) => (
+                                    Some(comp::InventoryUpdate::new(
+                                        comp::InventoryUpdateEvent::CollectFailed,
+                                    )),
+                                    false,
+                                ),
+                            }
+                        } else {
+                            debug!(
+                                "Can't add item to inventory: entity has no inventory ({:?})",
+                                entity
+                            );
+                            (None, false)
+                        };
+                        if let Some(event) = event {
+                            state.write_component(entity, event);
+                            if item_was_added {
+                                // we made sure earlier the block was not already modified this tick
+                                state.set_block(pos, Block::empty())
+                            };
+                        }
+                    } else {
+                        debug!(
+                            "Failed to reclaim item from block at pos={} or entity had no \
+                             inventory",
+                            pos
+                        )
+                    }
+                } else {
+                    debug!(
+                        "Can't reclaim item from block at pos={}: block is not collectable or was \
+                         already set this tick.",
+                        pos
                     );
-                } else if block.is_collectible()
-                    && state.try_set_block(pos, Block::empty()).is_some()
-                {
-                    comp::Item::try_reclaim_from_block(block)
-                        .map(|item| state.give_item(entity, item));
                 }
             }
         },

server/src/state_ext.rs

@@ -18,8 +18,6 @@ use tracing::warn;
 use vek::*;
 
 pub trait StateExt {
-    /// Push an item into the provided entity's inventory
-    fn give_item(&mut self, entity: EcsEntity, item: comp::Item) -> bool;
     /// Updates a component associated with the entity based on the `Effect`
     fn apply_effect(&mut self, entity: EcsEntity, effect: Effect);
     /// Build a non-player character
@@ -56,22 +54,6 @@ pub trait StateExt {
 }
 
 impl StateExt for State {
-    fn give_item(&mut self, entity: EcsEntity, item: comp::Item) -> bool {
-        let success = self
-            .ecs()
-            .write_storage::<comp::Inventory>()
-            .get_mut(entity)
-            .map(|inv| inv.push(item.clone()).is_none())
-            .unwrap_or(false);
-        if success {
-            self.write_component(
-                entity,
-                comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Collected(item)),
-            );
-        }
-        success
-    }
-
     fn apply_effect(&mut self, entity: EcsEntity, effect: Effect) {
         match effect {
             Effect::Health(change) => {