diff --git a/crates/benilla-app/src/go_templates.rs b/crates/benilla-app/src/go_templates.rs index d45c0fbe..4c4892f9 100644 --- a/crates/benilla-app/src/go_templates.rs +++ b/crates/benilla-app/src/go_templates.rs @@ -2,6 +2,10 @@ //! streams in. The template's lockId decides a right-click: a locked object is opened by an //! `OPEN_LOCK` cast, an unlocked one by `CMSG_GAMEOBJ_USE`. +use std::collections::HashMap; +use std::sync::Mutex; + +use benilla_assets::LockRecover; use benilla_formats::{LockCatalog, LockTypeCatalog}; use benilla_protocol::guid; use bevy::prelude::*; @@ -79,6 +83,15 @@ pub(crate) struct TextPage { #[derive(Resource, Default)] pub(crate) struct GameObjectTemplates { templates: QueryCache, + /// The guids whose descriptor entry is not the one their guid's bits carry, so a lookup that + /// has only the guid still finds the template. Only those: a 1.12 guid costs nothing here. + /// + /// A hardcore gravestone on this realm reads high `0xF112`, which names no 1.12 object family + /// at all (`guid::entry` answers `None` for it), so every lookup by guid alone missed and the + /// query was never even sent. `OBJECT_FIELD_ENTRY` is the reference's own cache key — the + /// guid's entry slot is the shortcut, not the fact. Behind a lock, as `QueryCache`'s in-flight + /// set is, so the read that asks stays `&self`. + fixups: Mutex>, } impl crate::query_cache::AskOnce for GameObjectTemplates { @@ -89,18 +102,47 @@ impl crate::query_cache::AskOnce for GameObjectTemplates { impl GameObjectTemplates { /// Asks for a template once per entry; every spawn of a template shares the one query. - pub(crate) fn request(&self, guid: u64, commands: &NetCommands) { - let Some(entry) = guid::entry(guid) else { + /// + /// `entry` is the object's `OBJECT_FIELD_ENTRY`, which a caller holding its descriptor passes + /// and a caller without one leaves `None` to fall back to the guid's entry slot. When the two + /// disagree the descriptor wins and the guid is remembered, so a later lookup by guid alone + /// resolves. + pub(crate) fn request(&self, guid: u64, entry: Option, commands: &NetCommands) { + let Some(entry) = entry.filter(|&e| e != 0).or_else(|| guid::entry(guid)) else { return; }; + let from_guid = guid::entry(guid); + if from_guid != Some(entry) { + self.fixups.lock_recover().insert(guid, entry); + } self.templates.get_or_ask(entry, || { - debug!("go: asking template (entry {entry}, guid {guid:#x})"); + if from_guid == Some(entry) { + debug!("go: asking template (entry {entry}, guid {guid:#x})"); + } else { + // Once per entry, because a guid that carries no entry of its own is the shape + // behind a nameless object, and the next one should not need a bisect. + info!( + "go: entry {entry} is the descriptor's, not guid {guid:#x}'s (high {:#06x}, \ + whose entry slot reads {from_guid:?}) — asking under the descriptor's", + guid::high(guid), + ); + } let _ = commands .0 .send(ClientCommand::GameObjectQuery { entry, guid }); }); } + /// A GameObject guid's template entry: the descriptor's where one disagreed with the guid, + /// else the guid's own entry slot. + fn entry_of(&self, guid: u64) -> Option { + self.fixups + .lock_recover() + .get(&guid) + .copied() + .or_else(|| guid::entry(guid)) + } + /// Records a `SMSG_GAMEOBJECT_QUERY_RESPONSE`; a miss arrives zeroed, so no lock. pub(crate) fn insert(&mut self, entry: u32, type_id: u32, name: String, data: &[i32; 24]) { let lock_id = go_lock_slot(type_id) @@ -152,7 +194,7 @@ impl GameObjectTemplates { /// The cached template for a GameObject guid. pub(crate) fn get(&self, guid: u64) -> Option<&GoTemplate> { - guid::entry(guid).and_then(|e| self.templates.get(e)) + self.entry_of(guid).and_then(|e| self.templates.get(e)) } } @@ -174,6 +216,65 @@ fn go_lock_slot(type_id: u32) -> Option { mod tests { use super::*; + fn commands() -> (NetCommands, crossbeam_channel::Receiver) { + let (tx, rx) = crossbeam_channel::unbounded(); + (NetCommands(tx), rx) + } + + /// A hardcore gravestone's guid: high `0xF112` names no 1.12 object family, so the guid's + /// entry slot is not its entry and the descriptor's is the only one there is. + const GRAVE_GUID: u64 = 0xf112_aea6_e34d_2828; + + #[test] + fn an_object_whose_guid_carries_no_entry_is_asked_for_and_found_by_its_descriptor() { + let (cmds, rx) = commands(); + let mut t = GameObjectTemplates::default(); + + // The guid alone answers nothing, so the guid alone asks nothing: the old shape, and the + // reason the plate drew nothing however well the pick landed. + assert_eq!(guid::entry(GRAVE_GUID), None); + t.request(GRAVE_GUID, None, &cmds); + assert!(rx.try_recv().is_err(), "no entry, no query"); + + // With the descriptor's entry the query goes out under that entry, carrying the guid. + t.request(GRAVE_GUID, Some(1_000_123), &cmds); + assert!(matches!( + rx.try_recv(), + Ok(ClientCommand::GameObjectQuery { + entry: 1_000_123, + guid: GRAVE_GUID + }) + )); + // Once per entry, however many gravestones share it. + t.request(GRAVE_GUID, Some(1_000_123), &cmds); + assert!(rx.try_recv().is_err(), "asked once per entry"); + + // ...and the answer is found again by guid alone, which is all a hover has. + t.insert(1_000_123, 5, "Grave of Gromgosh".into(), &[0i32; 24]); + assert_eq!( + t.get(GRAVE_GUID).map(|g| g.name.as_str()), + Some("Grave of Gromgosh") + ); + } + + #[test] + fn a_standard_guid_needs_no_fixup_and_a_zero_entry_falls_back_to_it() { + let (cmds, rx) = commands(); + let t = GameObjectTemplates::default(); + let g = u64::from(guid::HIGH_GAMEOBJECT) << 48 | 180_000 << 24 | 42; + assert_eq!(guid::entry(g), Some(180_000)); + + // A descriptor that agrees is not remembered: the guid's own slot answers. + t.request(g, Some(180_000), &cmds); + assert!(rx.try_recv().is_ok()); + assert!(t.fixups.lock_recover().is_empty()); + + // A descriptor not yet received reads 0, which is not an entry; the guid's slot stands. + t.request(g, Some(0), &cmds); + assert!(t.fixups.lock_recover().is_empty()); + assert_eq!(t.entry_of(g), Some(180_000)); + } + #[test] fn lock_slot_by_type_matches_getlockid() { assert_eq!(go_lock_slot(3), Some(0)); // CHEST (and gathering nodes) diff --git a/crates/benilla-app/src/net/objects.rs b/crates/benilla-app/src/net/objects.rs index e9f4bd06..e70af0c9 100644 --- a/crates/benilla-app/src/net/objects.rs +++ b/crates/benilla-app/src/net/objects.rs @@ -516,7 +516,7 @@ fn object_create( // Warm the template cache as a GameObject streams in: only the query carries its lockId, // which decides use-vs-cast on a right-click. if matches!(kind, EntityKind::GameObject) { - go_templates.request(guid, net_commands); + go_templates.request(guid, fields.object_entry(), net_commands); // Where and how big, under `RUST_LOG=benilla_app::net::objects=debug`. debug!( "gameobject spawn: entry {:?} display {display_id:?} type {go_type:?} \ diff --git a/crates/benilla-app/src/ui_item_text.rs b/crates/benilla-app/src/ui_item_text.rs index a69f1ecf..1b1dbb97 100644 --- a/crates/benilla-app/src/ui_item_text.rs +++ b/crates/benilla-app/src/ui_item_text.rs @@ -11,6 +11,7 @@ use bevy::prelude::*; +use benilla_protocol::ObjectFields; use benilla_ui::script::{ItemTextState, UiScript}; use crate::go_templates::GameObjectTemplates; @@ -199,16 +200,24 @@ pub(crate) fn object_material( commands: &NetCommands, ) -> Option { let name = |id: u32| materials.and_then(|m| m.0.name(id)).map(str::to_string); - if benilla_protocol::guid::is_gameobject(guid) { + // A GameObject by its own `OBJECT_FIELD_TYPE` bit, the reference's test, and only by the + // guid's family tag when no descriptor arrived: a realm's custom object can carry a guid + // family 1.12 never named, and reading one as an item asks the wrong query. + let fields = objects.object(guid); + let is_gameobject = fields.and_then(ObjectFields::object_type).map_or_else( + || benilla_protocol::guid::is_gameobject(guid), + |t| matches!(t, benilla_protocol::messages::ObjectType::GameObject), + ); + if is_gameobject { return match go_templates.get(guid) { Some(go) => go.quest_material.and_then(name), None => { - go_templates.request(guid, commands); + go_templates.request(guid, fields.and_then(ObjectFields::object_entry), commands); None } }; } - let entry = objects.object(guid)?.object_entry()?; + let entry = fields?.object_entry()?; let t = items.template(entry, guid, commands)?; name(t.page_material) } diff --git a/crates/benilla-app/src/ui_tooltip/mod.rs b/crates/benilla-app/src/ui_tooltip/mod.rs index c5c83636..caae0334 100644 --- a/crates/benilla-app/src/ui_tooltip/mod.rs +++ b/crates/benilla-app/src/ui_tooltip/mod.rs @@ -805,16 +805,20 @@ fn drive_mouseover_tooltip( ); } let Some(template) = go_inputs.templates.get(guid).cloned() else { - // Template in flight: ask once, and show it when it lands. Said out loud once per - // object, because this is the plate's only silent exit and the one place a server's - // own GameObject — a hardcore gravestone, say — can go nameless: no - // `SMSG_GAMEOBJECT_QUERY_RESPONSE`, no plate, however well the pick landed. A line - // that never gets a sequel means the query went unanswered. + // Template in flight: ask once, and show it when it lands. The ask carries the + // object's own `OBJECT_FIELD_ENTRY`, because a server's custom GameObject — a + // hardcore gravestone, whose guid reads high `0xF112` — has no entry in its guid for + // the cache to key on. Said out loud once per object, because this is the plate's + // only silent exit: a line that never gets a sequel means the query went unanswered. + let entry = stores.get(entity).ok().and_then(|s| s.0.object_entry()); if *memo.go_miss != Some(guid) { *memo.go_miss = Some(guid); - info!("go plate: {guid:#x} has no template yet — asked, drawing nothing"); + info!( + "go plate: {guid:#x} (entry {entry:?}) has no template yet — asking, drawing \ + nothing" + ); } - go_inputs.templates.request(guid, &commands); + go_inputs.templates.request(guid, entry, &commands); return; }; // The builder's lock lines (`0x52aa20`): "Locked" iff `GO_FLAG_LOCKED` (`0x52aae5`), then