From 3afa16bf0333b94e070ceec1f60d26f0f2fe4721 Mon Sep 17 00:00:00 2001 From: "Tormod G. Hellen" Date: Thu, 16 Dec 2021 19:37:15 +0100 Subject: [PATCH] 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. --- CHANGELOG.md | 2 + common/src/comp/inventory/item/mod.rs | 22 ++++++++- common/src/comp/inventory/mod.rs | 14 ++++++ voxygen/src/hud/hotbar.rs | 18 +++++--- voxygen/src/hud/mod.rs | 65 +++++++++++++++++++-------- voxygen/src/hud/skillbar.rs | 6 +-- voxygen/src/hud/slots.rs | 16 ++++--- voxygen/src/profile.rs | 44 +++--------------- voxygen/src/session/mod.rs | 12 ++++- 9 files changed, 123 insertions(+), 76 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 777e49dd91..6fb7ab61d6 100644 --- 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 diff --git a/common/src/comp/inventory/item/mod.rs b/common/src/comp/inventory/item/mod.rs index bca2288256..71eba26cd7 100644 --- 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, item_config: Option>, + hash: u64, +} + +use std::hash::{Hash, Hasher}; + +// Used to find inventory item corresponding to hotbar slot +impl Hash for Item { + fn hash(&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 diff --git a/common/src/comp/inventory/mod.rs b/common/src/comp/inventory/mod.rs index edc953e337..50d6db001c 100644 --- 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 { + 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) diff --git a/voxygen/src/hud/hotbar.rs b/voxygen/src/hud/hotbar.rs index c22e1b5b96..6007aa812a 100644 --- 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; 10], inputs: [bool; 10], @@ -43,14 +44,17 @@ impl State { just_pressed } - pub fn get(&self, slot: Slot) -> Option { self.slots[slot as usize] } + pub fn get(&self, slot: Slot) -> Option { 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 diff --git a/voxygen/src/hud/mod.rs b/voxygen/src/hud/mod.rs index be51c4e424..3bea1bdc01 100644 --- 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, 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 { diff --git a/voxygen/src/hud/skillbar.rs b/voxygen/src/hud/skillbar.rs index 599f0da3e9..76e4510799 100644 --- 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 diff --git a/voxygen/src/hud/slots.rs b/voxygen/src/hud/slots.rs index 5e83a48371..7256c698a7 100644 --- a/voxygen/src/hud/slots.rs +++ b/voxygen/src/hud/slots.rs @@ -130,11 +130,15 @@ impl<'a> SlotKey, HotbarImageSource<'a>> for HotbarSlot { &self, (hotbar, inventory, energy, skillset, active_abilities, body): &HotbarSource<'a>, ) -> Option<(Self::ImageKey, Option)> { + 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, 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, 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()) diff --git a/voxygen/src/profile.rs b/voxygen/src/profile.rs index 6c94695616..1006e04c99 100644 --- 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; 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); } } diff --git a/voxygen/src/session/mod.rs b/voxygen/src/session/mod.rs index fba8c75c5b..359a90e01c 100644 --- 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 {