Merge branch 'fix-hotbar-bug' into 'master'

Fix hotbar bug

See merge request veloren/veloren!3066

CHANGELOG.md

@@ -60,6 +60,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Poise damage dealt to a target that is in a stunned state is now converted to health damage at an efficiency dependent on the severity of the stunned state
 - You are now immune to poise damage for 1 second after leaving a stunned state
 - Removed or reduced poise damage from most abilities
+- Made the hotbar link to items by item definition id and component composition instead of specific inventory slots.
 
 ### Removed
 
@@ -75,6 +76,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Merchant cost percentages displayed as floored, whole numbers
 - Bodies of water no longer contain black chunks on the voxel minimap.
 - Agents can flee once again, and more appropriately
+- Items in hotbar no longer change when sorting inventory
 
 ## [0.11.0] - 2021-09-11
 

common/src/comp/agent.rs

@@ -76,11 +76,6 @@ impl Alignment {
             _ => false,
         }
     }
-
-    // TODO: Remove this hack
-    pub fn is_friendly_to_players(&self) -> bool {
-        matches!(self, Alignment::Npc | Alignment::Tame | Alignment::Owned(_))
-    }
 }
 
 impl Component for Alignment {

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

@@ -23,7 +23,7 @@ use crossbeam_utils::atomic::AtomicCell;
 use serde::{de, Deserialize, Serialize, Serializer};
 use specs::{Component, DerefFlaggedStorage};
 use specs_idvs::IdvStorage;
-use std::{fmt, sync::Arc};
+use std::{collections::hash_map::DefaultHasher, fmt, sync::Arc};
 use strum_macros::IntoStaticStr;
 use tracing::error;
 use vek::Rgb;
@@ -369,6 +369,17 @@ pub struct Item {
     /// The slots for items that this item has
     slots: Vec<InvSlot>,
     item_config: Option<Box<ItemConfig>>,
+    hash: u64,
+}
+
+use std::hash::{Hash, Hasher};
+
+// Used to find inventory item corresponding to hotbar slot
+impl Hash for Item {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.item_def.item_definition_id.hash(state);
+        self.components.hash(state);
+    }
 }
 
 // Custom serialization for ItemDef, we only want to send the item_definition_id
@@ -633,6 +644,12 @@ impl Item {
                     .map(|comp| comp.duplicate(ability_map, msm)),
             );
         }
+        let item_hash = {
+            let mut s = DefaultHasher::new();
+            inner_item.item_definition_id.hash(&mut s);
+            components.hash(&mut s);
+            s.finish()
+        };
 
         let mut item = Item {
             item_id: Arc::new(AtomicCell::new(None)),
@@ -641,6 +658,7 @@ impl Item {
             slots: vec![None; inner_item.slots as usize],
             item_def: inner_item,
             item_config: None,
+            hash: item_hash,
         };
         item.update_item_config(ability_map, msm);
         item
@@ -856,6 +874,8 @@ impl Item {
     }
 
     pub fn ability_spec(&self) -> Option<&AbilitySpec> { self.item_def.ability_spec.as_ref() }
+
+    pub fn item_hash(&self) -> u64 { self.hash }
 }
 
 /// Provides common methods providing details about an item definition

common/src/comp/inventory/mod.rs

@@ -347,6 +347,20 @@ impl Inventory {
         self.slot(inv_slot_id).and_then(Option::as_ref)
     }
 
+    /// Get item from inventory
+    pub fn get_by_hash(&self, item_hash: u64) -> Option<&Item> {
+        self.slots().flatten().find(|i| i.item_hash() == item_hash)
+    }
+
+    /// Get slot from hash
+    pub fn get_slot_from_hash(&self, item_hash: u64) -> Option<InvSlotId> {
+        let slot_with_id = self.slots_with_id().find(|slot| match slot.1 {
+            None => false,
+            Some(item) => item.item_hash() == item_hash,
+        });
+        slot_with_id.map(|s| s.0)
+    }
+
     /// Mutably get content of a slot
     fn get_mut(&mut self, inv_slot_id: InvSlotId) -> Option<&mut Item> {
         self.slot_mut(inv_slot_id).and_then(Option::as_mut)

voxygen/src/hud/hotbar.rs

@@ -1,4 +1,5 @@
-use common::comp::slot::InvSlotId;
+use crate::hud::item_imgs::ItemKey;
+use common::comp::inventory::item::Item;
 use serde::{Deserialize, Serialize};
 
 #[derive(Clone, Copy, Debug, PartialEq)]
@@ -15,13 +16,13 @@ pub enum Slot {
     Ten = 9,
 }
 
-#[derive(Clone, Copy, PartialEq, Debug, Serialize, Deserialize)]
+#[derive(Clone, PartialEq, Debug, Serialize, Deserialize)]
 pub enum SlotContents {
-    Inventory(InvSlotId),
+    Inventory(u64, ItemKey),
     Ability(usize),
 }
 
-#[derive(Clone, Copy, Default)]
+#[derive(Clone, Default)]
 pub struct State {
     pub slots: [Option<SlotContents>; 10],
     inputs: [bool; 10],
@@ -43,14 +44,17 @@ impl State {
         just_pressed
     }
 
-    pub fn get(&self, slot: Slot) -> Option<SlotContents> { self.slots[slot as usize] }
+    pub fn get(&self, slot: Slot) -> Option<SlotContents> { self.slots[slot as usize].clone() }
 
     pub fn swap(&mut self, a: Slot, b: Slot) { self.slots.swap(a as usize, b as usize); }
 
     pub fn clear_slot(&mut self, slot: Slot) { self.slots[slot as usize] = None; }
 
-    pub fn add_inventory_link(&mut self, slot: Slot, inventory_pos: InvSlotId) {
+    pub fn add_inventory_link(&mut self, slot: Slot, item: &Item) {
-        self.slots[slot as usize] = Some(SlotContents::Inventory(inventory_pos));
+        self.slots[slot as usize] = Some(SlotContents::Inventory(
+            item.item_hash(),
+            ItemKey::from(item),
+        ));
     }
 
     // TODO: remove pending UI

voxygen/src/hud/mod.rs

@@ -3304,8 +3304,13 @@ impl Hud {
                         Hotbar(h),
                     ) = (a, b)
                     {
-                        self.hotbar.add_inventory_link(h, slot);
+                        if let Some(item) = inventories
-                        events.push(Event::ChangeHotbarState(Box::new(self.hotbar.to_owned())));
+                            .get(client.entity())
+                            .and_then(|inv| inv.get(slot))
+                        {
+                            self.hotbar.add_inventory_link(h, item);
+                            events.push(Event::ChangeHotbarState(Box::new(self.hotbar.to_owned())));
+                        }
                     } else if let (Hotbar(a), Hotbar(b)) = (a, b) {
                         self.hotbar.swap(a, b);
                         events.push(Event::ChangeHotbarState(Box::new(self.hotbar.to_owned())));
@@ -3370,8 +3375,13 @@ impl Hud {
                             bypass_dialog: false,
                         });
                     } else if let (Inventory(i), Hotbar(h)) = (a, b) {
-                        self.hotbar.add_inventory_link(h, i.slot);
+                        if let Some(item) = inventories
-                        events.push(Event::ChangeHotbarState(Box::new(self.hotbar.to_owned())));
+                            .get(client.entity())
+                            .and_then(|inv| inv.get(i.slot))
+                        {
+                            self.hotbar.add_inventory_link(h, item);
+                            events.push(Event::ChangeHotbarState(Box::new(self.hotbar.to_owned())));
+                        }
                     } else if let (Hotbar(a), Hotbar(b)) = (a, b) {
                         self.hotbar.swap(a, b);
                         events.push(Event::ChangeHotbarState(Box::new(self.hotbar.to_owned())));
@@ -3419,11 +3429,16 @@ impl Hud {
                     } else if let Hotbar(h) = from {
                         // Used from hotbar
                         self.hotbar.get(h).map(|s| match s {
-                            hotbar::SlotContents::Inventory(i) => {
+                            hotbar::SlotContents::Inventory(i, _) => {
-                                events.push(Event::UseSlot {
+                                if let Some(slot) = inventories
-                                    slot: comp::slot::Slot::Inventory(i),
+                                    .get(client.entity())
-                                    bypass_dialog: false,
+                                    .and_then(|inv| inv.get_slot_from_hash(i))
-                                });
+                                {
+                                    events.push(Event::UseSlot {
+                                        slot: comp::slot::Slot::Inventory(slot),
+                                        bypass_dialog: false,
+                                    });
+                                }
                             },
                             hotbar::SlotContents::Ability(_) => {},
                         });
@@ -3602,7 +3617,12 @@ impl Hud {
         }
     }
 
-    pub fn handle_event(&mut self, event: WinEvent, global_state: &mut GlobalState) -> bool {
+    pub fn handle_event(
+        &mut self,
+        event: WinEvent,
+        global_state: &mut GlobalState,
+        client_inventory: Option<&comp::Inventory>,
+    ) -> bool {
         // Helper
         fn handle_slot(
             slot: hotbar::Slot,
@@ -3610,6 +3630,7 @@ impl Hud {
             events: &mut Vec<Event>,
             slot_manager: &mut slots::SlotManager,
             hotbar: &mut hotbar::State,
+            client_inventory: Option<&comp::Inventory>,
         ) {
             use slots::InventorySlot;
             if let Some(slots::SlotKind::Inventory(InventorySlot {
@@ -3618,18 +3639,24 @@ impl Hud {
                 ..
             })) = slot_manager.selected()
             {
-                hotbar.add_inventory_link(slot, i);
+                if let Some(item) = client_inventory.and_then(|inv| inv.get(i)) {
-                events.push(Event::ChangeHotbarState(Box::new(hotbar.to_owned())));
+                    hotbar.add_inventory_link(slot, item);
-                slot_manager.idle();
+                    events.push(Event::ChangeHotbarState(Box::new(hotbar.to_owned())));
+                    slot_manager.idle();
+                }
             } else {
                 let just_pressed = hotbar.process_input(slot, state);
                 hotbar.get(slot).map(|s| match s {
-                    hotbar::SlotContents::Inventory(i) => {
+                    hotbar::SlotContents::Inventory(i, _) => {
                         if just_pressed {
-                            events.push(Event::UseSlot {
+                            if let Some(slot) =
-                                slot: comp::slot::Slot::Inventory(i),
+                                client_inventory.and_then(|inv| inv.get_slot_from_hash(i))
-                                bypass_dialog: false,
+                            {
-                            });
+                                events.push(Event::UseSlot {
+                                    slot: comp::slot::Slot::Inventory(slot),
+                                    bypass_dialog: false,
+                                });
+                            }
                         }
                     },
                     hotbar::SlotContents::Ability(i) => events.push(Event::Ability(i, state)),
@@ -3714,6 +3741,7 @@ impl Hud {
                         &mut self.events,
                         &mut self.slot_manager,
                         &mut self.hotbar,
+                        client_inventory,
                     );
                     true
                 } else {
@@ -3817,6 +3845,7 @@ impl Hud {
                                 &mut self.events,
                                 &mut self.slot_manager,
                                 &mut self.hotbar,
+                                client_inventory,
                             );
                             true
                         } else {

voxygen/src/hud/skillbar.rs

@@ -594,7 +594,7 @@ impl<'a> Skillbar<'a> {
         let slot_content = |slot| {
             let (hotbar, inventory, ..) = content_source;
             hotbar.get(slot).and_then(|content| match content {
-                hotbar::SlotContents::Inventory(i) => inventory.get(i),
+                hotbar::SlotContents::Inventory(i, _) => inventory.get_by_hash(i),
                 _ => None,
             })
         };
@@ -603,8 +603,8 @@ impl<'a> Skillbar<'a> {
         let tooltip_text = |slot| {
             let (hotbar, inventory, _, _, active_abilities, _) = content_source;
             hotbar.get(slot).and_then(|content| match content {
-                hotbar::SlotContents::Inventory(i) => inventory
+                hotbar::SlotContents::Inventory(i, _) => inventory
-                    .get(i)
+                    .get_by_hash(i)
                     .map(|item| (item.name(), item.description())),
                 hotbar::SlotContents::Ability(i) => active_abilities
                     .abilities

voxygen/src/hud/slots.rs

@@ -130,11 +130,15 @@ impl<'a> SlotKey<HotbarSource<'a>, HotbarImageSource<'a>> for HotbarSlot {
         &self,
         (hotbar, inventory, energy, skillset, active_abilities, body): &HotbarSource<'a>,
     ) -> Option<(Self::ImageKey, Option<Color>)> {
+        const GREYED_OUT: Color = Color::Rgba(0.3, 0.3, 0.3, 0.8);
         hotbar.get(*self).and_then(|contents| match contents {
-            hotbar::SlotContents::Inventory(idx) => inventory
+            hotbar::SlotContents::Inventory(item_hash, item_key) => {
-                .get(idx)
+                let item = inventory.get_by_hash(item_hash);
-                .map(|item| HotbarImage::Item(item.into()))
+                match item {
-                .map(|i| (i, None)),
+                    Some(item) => Some((HotbarImage::Item(item.into()), None)),
+                    None => Some((HotbarImage::Item(item_key), Some(GREYED_OUT))),
+                }
+            },
             hotbar::SlotContents::Ability(i) => {
                 let ability_id = active_abilities
                     .abilities
@@ -157,7 +161,7 @@ impl<'a> SlotKey<HotbarSource<'a>, HotbarImageSource<'a>> for HotbarSlot {
                                     if energy.current() > ability.get_energy_cost() {
                                         Some(Color::Rgba(1.0, 1.0, 1.0, 1.0))
                                     } else {
-                                        Some(Color::Rgba(0.3, 0.3, 0.3, 0.8))
+                                        Some(GREYED_OUT)
                                     },
                                 )
                             })
@@ -170,7 +174,7 @@ impl<'a> SlotKey<HotbarSource<'a>, HotbarImageSource<'a>> for HotbarSlot {
         hotbar
             .get(*self)
             .and_then(|content| match content {
-                hotbar::SlotContents::Inventory(idx) => inventory.get(idx),
+                hotbar::SlotContents::Inventory(item_hash, _) => inventory.get_by_hash(item_hash),
                 hotbar::SlotContents::Ability(_) => None,
             })
             .map(|item| item.amount())

voxygen/src/profile.rs

@@ -1,5 +1,5 @@
 use crate::hud;
-use common::{character::CharacterId, comp::slot::InvSlotId};
+use common::character::CharacterId;
 use hashbrown::HashMap;
 use serde::{Deserialize, Serialize};
 use std::{
@@ -18,18 +18,7 @@ pub struct CharacterProfile {
 }
 
 const fn default_slots() -> [Option<hud::HotbarSlotContents>; 10] {
-    [
+    [None, None, None, None, None, None, None, None, None, None]
-        None,
-        None,
-        None,
-        None,
-        None,
-        Some(hud::HotbarSlotContents::Inventory(InvSlotId::new(0, 0))),
-        Some(hud::HotbarSlotContents::Inventory(InvSlotId::new(0, 1))),
-        None,
-        None,
-        None,
-    ]
 }
 
 impl Default for CharacterProfile {
@@ -132,7 +121,7 @@ impl Profile {
         self.servers
             .get(server)
             .and_then(|s| s.characters.get(&character_id))
-            .map(|c| c.hotbar_slots)
+            .map(|c| c.hotbar_slots.clone())
             .unwrap_or_else(default_slots)
     }
 
@@ -216,41 +205,18 @@ impl Profile {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use common::comp::inventory::slot::InvSlotId;
 
     #[test]
     fn test_get_slots_with_empty_profile() {
         let profile = Profile::default();
         let slots = profile.get_hotbar_slots("TestServer", 12345);
-        assert_eq!(slots, [
+        assert_eq!(slots, [(); 10].map(|()| None))
-            None,
-            None,
-            None,
-            None,
-            None,
-            Some(hud::HotbarSlotContents::Inventory(InvSlotId::new(0, 0))),
-            Some(hud::HotbarSlotContents::Inventory(InvSlotId::new(0, 1))),
-            None,
-            None,
-            None,
-        ])
     }
 
     #[test]
     fn test_set_slots_with_empty_profile() {
         let mut profile = Profile::default();
-        let slots = [
+        let slots = [(); 10].map(|()| None);
-            None,
-            None,
-            None,
-            None,
-            None,
-            Some(hud::HotbarSlotContents::Inventory(InvSlotId::new(0, 0))),
-            Some(hud::HotbarSlotContents::Inventory(InvSlotId::new(0, 1))),
-            None,
-            None,
-            None,
-        ];
         profile.set_hotbar_slots("TestServer", 12345, slots);
     }
 }

voxygen/src/session/mod.rs

@@ -462,8 +462,16 @@ impl PlayState for SessionState {
             // Handle window events.
             for event in events {
                 // Pass all events to the ui first.
-                if self.hud.handle_event(event.clone(), global_state) {
+                {
-                    continue;
+                    let client = self.client.borrow();
+                    let inventories = client.inventories();
+                    let inventory = inventories.get(client.entity());
+                    if self
+                        .hud
+                        .handle_event(event.clone(), global_state, inventory)
+                    {
+                        continue;
+                    }
                 }
 
                 match event {