A quest item he was carrying had no name, because one query went unanswered and nothing asked again
grom, on the Deeprun Rat Roundup quest: "I'm not recieving the flute item needed for the quest". He had it the whole time. Entry 17117, Rat Catcher's Flute, bag 0 slot 2 — the only trace of it in a three-hour log is `GetNumJunkItems` reporting one carried slot "with no item template yet", twice. The ask-once rule is the reference's `DBCache` and it is right for everything it covers: one `CMSG_ITEM_QUERY_SINGLE` per entry per connection, a negative cached as a negative. What it has no arm for is a query that goes out and is never answered, and the whole cost of that falls on the player — `GetItemInfo` keeps returning nil, so the item draws with no name, no icon and no tooltip, in the bag, in the quest frame's required-items box and in every addon that reads it, for the rest of the session. `QueryCache` now ages its in-flight asks and `items.rs` re-sends one that has waited five seconds, three sends in all, then says so once. Bounded on purpose: an entry the server genuinely does not know is answered with the high-bit miss and never reaches the sweep, so a key that survives three sends is one nothing is going to answer. Ordered after the world-entry release, which forgets every in-flight ask — a sweep that ran first would re-send queries the next statement drops. The same gap exists for creature names and gameobject templates. Left alone until one of them is seen to drop an answer: a nameless mob is at least self-correcting on the next `SMSG_UPDATE_OBJECT`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
@@ -480,6 +480,58 @@ impl crate::query_cache::AskOnce for Items {
|
||||
}
|
||||
}
|
||||
|
||||
/// How long an unanswered `CMSG_ITEM_QUERY_SINGLE` waits before it is sent again.
|
||||
///
|
||||
/// Long enough that a slow answer is never raced (the realm's own answers land in tens of
|
||||
/// milliseconds), short enough that a lost one is invisible: an item that appears in a bag with
|
||||
/// no name and gets one a few seconds later reads as the cache filling, which is what the player
|
||||
/// already sees on a cold login.
|
||||
const TEMPLATE_RETRY_AFTER: f64 = 5.0;
|
||||
|
||||
/// Sends in all, so two re-asks past the original. Bounded on purpose: an entry the server
|
||||
/// genuinely does not know is answered with the high-bit miss and never reaches the sweep, so a
|
||||
/// key that survives three sends is one nothing is going to answer, and asking forever would
|
||||
/// turn a lost packet into a standing stream.
|
||||
const TEMPLATE_MAX_TRIES: u8 = 3;
|
||||
|
||||
/// Re-ask item templates whose answer never arrived, and say so once when giving up.
|
||||
///
|
||||
/// The ask-once rule is the reference's ([`crate::query_cache`]), and it is right for everything
|
||||
/// it covers: one query per entry per connection, a negative cached as a negative. What it has no
|
||||
/// arm for is an ask that is simply never answered, and the cost of that falls entirely on the
|
||||
/// player — `GetItemInfo` keeps answering nil, so the item draws with no name, no icon and no
|
||||
/// tooltip, in the bag, in the quest frame's required-items box and in every addon that reads it,
|
||||
/// for the rest of the session. That is grom's *"I'm not recieving the flute item needed for the
|
||||
/// quest"*: entry 17117 sat in bag 0 slot 2 the whole time, and the only trace of it was
|
||||
/// `GetNumJunkItems` reporting a carried slot *"with no item template yet"*.
|
||||
///
|
||||
/// The same gap exists for creature names and gameobject templates ([`crate::names`],
|
||||
/// [`crate::go_templates`]); they are left alone until one of them is actually seen to drop an
|
||||
/// answer, since a nameless mob is at least self-correcting on the next `SMSG_UPDATE_OBJECT`.
|
||||
pub(crate) fn retry_unanswered_templates(
|
||||
items: Res<Items>,
|
||||
commands: Res<NetCommands>,
|
||||
time: Res<Time>,
|
||||
) {
|
||||
let now = f64::from(time.elapsed_secs());
|
||||
let sweep = items
|
||||
.templates
|
||||
.sweep_pending(now, TEMPLATE_RETRY_AFTER, TEMPLATE_MAX_TRIES);
|
||||
for entry in sweep.retry {
|
||||
warn!(
|
||||
"items: no SMSG_ITEM_QUERY_SINGLE_RESPONSE for entry {entry} after \
|
||||
{TEMPLATE_RETRY_AFTER:.0}s — asking again"
|
||||
);
|
||||
let _ = commands.0.send(ClientCommand::ItemQuery { entry, guid: 0 });
|
||||
}
|
||||
for entry in sweep.abandoned {
|
||||
warn!(
|
||||
"items: entry {entry} went unanswered {TEMPLATE_MAX_TRIES} times — it will have no \
|
||||
name, icon or tooltip for the rest of this session"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The ask-once test fixture; it holds the live receiver, without which every send fails.
|
||||
#[cfg(test)]
|
||||
pub(crate) struct TestDeps {
|
||||
|
||||
@@ -66,6 +66,17 @@ impl Plugin for NetPlugin {
|
||||
crate::query_cache::register::<crate::names::NameCache>(app);
|
||||
crate::query_cache::register::<crate::go_templates::GameObjectTemplates>(app);
|
||||
crate::query_cache::register::<crate::items::Items>(app);
|
||||
// And the one cache that re-asks: an item template nobody answers leaves the item blank
|
||||
// in the player's bag for the whole session.
|
||||
// After the release, not merely beside it: entering the world forgets every in-flight
|
||||
// ask, and a sweep that ran first would re-send queries the very next statement drops.
|
||||
app.add_systems(
|
||||
Update,
|
||||
crate::items::retry_unanswered_templates
|
||||
.in_set(benilla_world::schedule::WorldStage::Net)
|
||||
.after(apply_net_updates)
|
||||
.after(crate::query_cache::release_on_enter::<crate::items::Items>),
|
||||
);
|
||||
let handles = io::spawn_net(self.connect);
|
||||
if !self.connect {
|
||||
app.insert_resource(NetOffline);
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
//! A negative answer is cached as `None` and never re-asked: an integer-keyed store's high-bit miss
|
||||
//! (`0x556e67`), or a player's empty name.
|
||||
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::collections::HashMap;
|
||||
use std::hash::Hash;
|
||||
use std::sync::Mutex;
|
||||
|
||||
@@ -21,16 +21,36 @@ pub(crate) struct QueryCache<K, V> {
|
||||
answered: HashMap<K, Option<V>>,
|
||||
/// The keys in flight, locked so the read that asks is `&self`; uncontended on the main
|
||||
/// thread.
|
||||
pending: Mutex<HashSet<K>>,
|
||||
pending: Mutex<HashMap<K, Ask>>,
|
||||
/// Bumped by every landing or eviction, the reference's `DBCACHECALLBACK` redisplay.
|
||||
generation: u64,
|
||||
}
|
||||
|
||||
/// One in-flight ask, for [`QueryCache::sweep_pending`].
|
||||
#[derive(Default)]
|
||||
struct Ask {
|
||||
/// When the ask was last sent, as the sweep's own clock reads it; `None` until the first
|
||||
/// sweep sees it, since `get_or_ask` has no clock to stamp it with.
|
||||
last: Option<f64>,
|
||||
/// How many times it has been sent, the first included.
|
||||
tries: u8,
|
||||
/// Whether giving up on it has been reported, so the warning is said once.
|
||||
abandoned: bool,
|
||||
}
|
||||
|
||||
/// What a [`QueryCache::sweep_pending`] pass found.
|
||||
pub(crate) struct AskSweep<K> {
|
||||
/// Asks past their deadline with tries left: re-send these.
|
||||
pub(crate) retry: Vec<K>,
|
||||
/// Asks past their deadline with no tries left, reported once each.
|
||||
pub(crate) abandoned: Vec<K>,
|
||||
}
|
||||
|
||||
impl<K: Copy + Eq + Hash, V> Default for QueryCache<K, V> {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
answered: HashMap::new(),
|
||||
pending: Mutex::new(HashSet::new()),
|
||||
pending: Mutex::new(HashMap::new()),
|
||||
generation: 0,
|
||||
}
|
||||
}
|
||||
@@ -43,7 +63,14 @@ impl<K: Copy + Eq + Hash, V> QueryCache<K, V> {
|
||||
match self.answered.get(&key) {
|
||||
Some(answer) => answer.as_ref(),
|
||||
None => {
|
||||
if self.pending.lock_recover().insert(key) {
|
||||
let mut pending = self.pending.lock_recover();
|
||||
if let std::collections::hash_map::Entry::Vacant(slot) = pending.entry(key) {
|
||||
slot.insert(Ask {
|
||||
last: None,
|
||||
tries: 1,
|
||||
abandoned: false,
|
||||
});
|
||||
drop(pending);
|
||||
ask();
|
||||
}
|
||||
None
|
||||
@@ -74,7 +101,46 @@ impl<K: Copy + Eq + Hash, V> QueryCache<K, V> {
|
||||
#[cfg(test)]
|
||||
/// Whether an ask for `key` is in flight.
|
||||
pub(crate) fn is_pending(&self, key: K) -> bool {
|
||||
self.pending.lock_recover().contains(&key)
|
||||
self.pending.lock_recover().contains_key(&key)
|
||||
}
|
||||
|
||||
/// Age the in-flight asks against `now` and report the ones that have waited longer than
|
||||
/// `after` seconds: those with tries left (re-send them), and those without, once each.
|
||||
///
|
||||
/// **Why an ask-once cache needs this at all.** The reference re-asks nothing either, and on
|
||||
/// a client that never loses a response it never has to. Here a query that goes out and is
|
||||
/// never answered leaves its key answered-never: `GetItemInfo` keeps returning nil, so the
|
||||
/// item has no name, no icon and no tooltip for the rest of the session, and nothing in the
|
||||
/// UI ever asks again. grom, 2026-11-08, on the quest item in bag 0 slot 2: *"on the quest
|
||||
/// Deeprun Rat Roundup, I'm not recieving the flute item needed for the quest"* — he had it,
|
||||
/// entry 17117, and it was blank.
|
||||
///
|
||||
/// The first sweep to see an ask only stamps it, because `get_or_ask` takes `&self` and has
|
||||
/// no clock; the deadline therefore runs from that stamp, which is at most one sweep period
|
||||
/// after the send.
|
||||
pub(crate) fn sweep_pending(&self, now: f64, after: f64, max_tries: u8) -> AskSweep<K> {
|
||||
let mut sweep = AskSweep {
|
||||
retry: Vec::new(),
|
||||
abandoned: Vec::new(),
|
||||
};
|
||||
for (key, ask) in self.pending.lock_recover().iter_mut() {
|
||||
let Some(last) = ask.last else {
|
||||
ask.last = Some(now);
|
||||
continue;
|
||||
};
|
||||
if now - last < after {
|
||||
continue;
|
||||
}
|
||||
if ask.tries < max_tries {
|
||||
ask.tries += 1;
|
||||
ask.last = Some(now);
|
||||
sweep.retry.push(*key);
|
||||
} else if !ask.abandoned {
|
||||
ask.abandoned = true;
|
||||
sweep.abandoned.push(*key);
|
||||
}
|
||||
}
|
||||
sweep
|
||||
}
|
||||
|
||||
/// Lands an answer; `None` is the server's negative.
|
||||
@@ -151,7 +217,7 @@ pub(crate) fn register<T: AskOnce>(app: &mut App) {
|
||||
);
|
||||
}
|
||||
|
||||
fn release_on_enter<T: AskOnce>(
|
||||
pub(crate) fn release_on_enter<T: AskOnce>(
|
||||
mut entered: MessageReader<EnteredWorldMessage>,
|
||||
mut cache: ResMut<T>,
|
||||
) {
|
||||
@@ -165,6 +231,49 @@ fn release_on_enter<T: AskOnce>(
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// The sweep's whole job: a key asked once and never answered gets asked again, a bounded
|
||||
/// number of times, and is then reported once rather than re-asked forever.
|
||||
#[test]
|
||||
fn an_unanswered_ask_is_re_sent_until_the_tries_run_out() {
|
||||
let cache: QueryCache<u32, &str> = QueryCache::default();
|
||||
let mut asks = 0;
|
||||
cache.get_or_ask(17117, || asks += 1);
|
||||
|
||||
// The first sweep only stamps it: `get_or_ask` has no clock of its own.
|
||||
assert!(cache.sweep_pending(0.0, 5.0, 3).retry.is_empty());
|
||||
assert!(
|
||||
cache.sweep_pending(4.9, 5.0, 3).retry.is_empty(),
|
||||
"and nothing is re-asked before the deadline"
|
||||
);
|
||||
|
||||
assert_eq!(cache.sweep_pending(5.0, 5.0, 3).retry, vec![17117]);
|
||||
assert_eq!(cache.sweep_pending(10.0, 5.0, 3).retry, vec![17117]);
|
||||
let third = cache.sweep_pending(15.0, 5.0, 3);
|
||||
assert!(
|
||||
third.retry.is_empty(),
|
||||
"three sends in all, the original included"
|
||||
);
|
||||
assert_eq!(third.abandoned, vec![17117]);
|
||||
assert!(
|
||||
cache.sweep_pending(20.0, 5.0, 3).abandoned.is_empty(),
|
||||
"and giving up is said once, not once a sweep"
|
||||
);
|
||||
}
|
||||
|
||||
/// An answer takes the key out of the sweep, whichever answer it is: a server that knows
|
||||
/// nothing about an entry says so with the high-bit miss, and that is an answer.
|
||||
#[test]
|
||||
fn an_answered_key_is_never_swept_again() {
|
||||
for answer in [Some("flute"), None] {
|
||||
let mut cache: QueryCache<u32, &str> = QueryCache::default();
|
||||
cache.get_or_ask(17117, || ());
|
||||
cache.sweep_pending(0.0, 5.0, 3);
|
||||
cache.insert(17117, answer);
|
||||
let sweep = cache.sweep_pending(60.0, 5.0, 3);
|
||||
assert!(sweep.retry.is_empty() && sweep.abandoned.is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_miss_asks_once_and_a_landing_answers_it() {
|
||||
let mut cache: QueryCache<u32, &str> = QueryCache::default();
|
||||
|
||||
Reference in New Issue
Block a user