Fix hotbar changing when sorting inventory.

Previously the hotbar slots would refer to inventory slots. An unfortunate consequence of this was that when the contents of an inventory slot changed, so would the corresponding hotbar slot change. This commit fixes that.
2024-08-30 18:12:32 +00:00 · 2021-12-16 19:37:15 +01:00 · 2021-12-16 19:37:15 +01:00 · 3afa16bf03
commit 3afa16bf03
parent a33e6df303
9 changed files with 123 additions and 76 deletions
--- a/CHANGELOG.md
+++ b/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

--- a/common/src/comp/inventory/item/mod.rs
+++ b/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
--- a/common/src/comp/inventory/mod.rs
+++ b/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)
--- a/voxygen/src/hud/hotbar.rs
+++ b/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) {
-        self.slots[slot as usize] = Some(SlotContents::Inventory(inventory_pos));
+    pub fn add_inventory_link(&mut self, slot: Slot, item: &Item) {
+        self.slots[slot as usize] = Some(SlotContents::Inventory(
+            item.item_hash(),
+            ItemKey::from(item),
+        ));
    }

    // TODO: remove pending UI
--- a/voxygen/src/hud/mod.rs
+++ b/voxygen/src/hud/mod.rs
@ -3304,8 +3304,13 @@ impl Hud {
                        Hotbar(h),
                    ) = (a, b)
                    {
-                        self.hotbar.add_inventory_link(h, slot);
-                        events.push(Event::ChangeHotbarState(Box::new(self.hotbar.to_owned())));
+                        if let Some(item) = inventories
+                            .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);
-                        events.push(Event::ChangeHotbarState(Box::new(self.hotbar.to_owned())));
+                        if let Some(item) = inventories
+                            .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) => {
-                                events.push(Event::UseSlot {
-                                    slot: comp::slot::Slot::Inventory(i),
-                                    bypass_dialog: false,
-                                });
+                            hotbar::SlotContents::Inventory(i, _) => {
+                                if let Some(slot) = inventories
+                                    .get(client.entity())
+                                    .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);
-                events.push(Event::ChangeHotbarState(Box::new(hotbar.to_owned())));
-                slot_manager.idle();
+                if let Some(item) = client_inventory.and_then(|inv| inv.get(i)) {
+                    hotbar.add_inventory_link(slot, item);
+                    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 {
-                                slot: comp::slot::Slot::Inventory(i),
-                                bypass_dialog: false,
-                            });
+                            if let Some(slot) =
+                                client_inventory.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(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 {
--- a/voxygen/src/hud/skillbar.rs
+++ b/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
-                    .get(i)
+                hotbar::SlotContents::Inventory(i, _) => inventory
+                    .get_by_hash(i)
                    .map(|item| (item.name(), item.description())),
                hotbar::SlotContents::Ability(i) => active_abilities
                    .abilities
--- a/voxygen/src/hud/slots.rs
+++ b/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
-                .get(idx)
-                .map(|item| HotbarImage::Item(item.into()))
-                .map(|i| (i, None)),
+            hotbar::SlotContents::Inventory(item_hash, item_key) => {
+                let item = inventory.get_by_hash(item_hash);
+                match item {
+                    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())
--- a/voxygen/src/profile.rs
+++ b/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,
-        Some(hud::HotbarSlotContents::Inventory(InvSlotId::new(0, 0))),
-        Some(hud::HotbarSlotContents::Inventory(InvSlotId::new(0, 1))),
-        None,
-        None,
-        None,
-    ]
+    [None, None, None, None, None, None, None, 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, [
-            None,
-            None,
-            None,
-            None,
-            None,
-            Some(hud::HotbarSlotContents::Inventory(InvSlotId::new(0, 0))),
-            Some(hud::HotbarSlotContents::Inventory(InvSlotId::new(0, 1))),
-            None,
-            None,
-            None,
-        ])
+        assert_eq!(slots, [(); 10].map(|()| None))
    }

    #[test]
    fn test_set_slots_with_empty_profile() {
        let mut profile = Profile::default();
-        let slots = [
-            None,
-            None,
-            None,
-            None,
-            None,
-            Some(hud::HotbarSlotContents::Inventory(InvSlotId::new(0, 0))),
-            Some(hud::HotbarSlotContents::Inventory(InvSlotId::new(0, 1))),
-            None,
-            None,
-            None,
-        ];
+        let slots = [(); 10].map(|()| None);
        profile.set_hotbar_slots("TestServer", 12345, slots);
    }
 }
--- a/voxygen/src/session/mod.rs
+++ b/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 {