diff --git a/crates/benilla-app/src/ui_merchant/mod.rs b/crates/benilla-app/src/ui_merchant/mod.rs index e5d1950c..02381257 100644 --- a/crates/benilla-app/src/ui_merchant/mod.rs +++ b/crates/benilla-app/src/ui_merchant/mod.rs @@ -686,7 +686,15 @@ fn drain_merchant( count: 0, }); } - None => debug!("ui_merchant: sell on an empty slot ({bag}, {slot}) — ignored"), + // Warned, not debugged: the two queues that reach here both name a slot the VM has + // just looked at and found occupied, so "no item guid there" is an anomaly, not + // housekeeping. It is also the last silent place a sell-junk click could die — the + // count and the sweep both say what they saw now (`C_MerchantFrame`'s `junk_survey`), + // and this is where that trail would otherwise end. + None => warn!( + "ui_merchant: sell on bag {bag} slot {slot} found no item guid — nothing sent. \ + The slot read occupied when it was queued." + ), } } // A held vendor row dropped into a slot: `CMSG_BUY_ITEM_IN_SLOT` with count 1, the diff --git a/crates/benilla-ui/src/script/compat/classic_api/merchant_frame.rs b/crates/benilla-ui/src/script/compat/classic_api/merchant_frame.rs index 6fd8fc59..37aa3be9 100644 --- a/crates/benilla-ui/src/script/compat/classic_api/merchant_frame.rs +++ b/crates/benilla-ui/src/script/compat/classic_api/merchant_frame.rs @@ -32,6 +32,66 @@ const QUALITY_POOR: u32 = 0; /// `C_Container`'s own free-slot total covers. const CARRIED_BAGS: [i64; 5] = [0, 1, 2, 3, 4]; +/// What a junk sweep can see in the carried bags: the grey stacks, and the occupied slots it +/// **cannot judge** because their quality has not resolved yet (`ContainerSlot::quality` is `None` +/// until the item's template is in hand — `ui_items::feed`'s no-template arm). +/// +/// Shared by the count and the sweep so the two can never disagree about what they are looking at, +/// and so the count — which is the gate every caller actually stands behind — can say why it +/// answered zero. +fn junk_survey(model: &Model) -> JunkSurvey { + let mut grey = Vec::new(); + let mut unjudgeable = Vec::new(); + let mut occupied = 0usize; + for bag in CARRIED_BAGS { + let Some(c) = model.containers.get(&bag) else { + continue; + }; + for (slot, s) in &c.slots { + if s.item_id == 0 { + continue; + } + occupied += 1; + match s.quality { + Some(QUALITY_POOR) => grey.push((bag, *slot)), + Some(_) => {} + None => unjudgeable.push((bag, *slot, s.item_id)), + } + } + } + JunkSurvey { + grey, + unjudgeable, + occupied, + } +} + +/// [`junk_survey`]'s answer. +struct JunkSurvey { + /// `(bag, slot)` per grey stack, the set both the count and the sweep act on. + grey: Vec<(i64, u32)>, + /// `(bag, slot, item id)` per occupied slot whose quality has not resolved. + unjudgeable: Vec<(i64, u32, u32)>, + /// Every occupied carried slot, the denominator the other two are read against. + occupied: usize, +} + +/// `junk_survey`'s unjudgeable list as one sentence, up to twelve slots named with their item ids. +fn unjudgeable_line(unjudgeable: &[(i64, u32, u32)]) -> String { + let shown: Vec = unjudgeable + .iter() + .take(12) + .map(|(bag, slot, id)| format!("({bag},{slot}) item {id}")) + .collect(); + format!( + "{} carried slot(s) have no item template yet, so their quality is unknown and a grey \ + among them is neither counted nor sold: {}{}", + unjudgeable.len(), + shown.join(", "), + if unjudgeable.len() > 12 { ", …" } else { "" } + ) +} + pub(super) fn install(lua: &Lua) -> mlua::Result { let t = lua.create_table()?; @@ -97,14 +157,28 @@ pub(super) fn install(lua: &Lua) -> mlua::Result
{ t.set( "GetNumJunkItems", lua.create_function(|lua, ()| { - let model = lua.app_data_ref::().expect("model app_data"); - let n = CARRIED_BAGS - .iter() - .filter_map(|bag| model.containers.get(bag)) - .flat_map(|c| c.slots.values()) - .filter(|s| s.item_id != 0 && s.quality == Some(QUALITY_POOR)) - .count(); - Ok(n as i64) + let mut model = lua.app_data_mut::().expect("model app_data"); + let JunkSurvey { + grey, + unjudgeable, + occupied, + } = junk_survey(&model); + // **This is the gate, not the sweep.** pfUI's Sell Grey Items button reads this and + // returns without ever calling `SellAllJunkItems` when it answers zero + // (`modules/autovendor.lua`'s `SellJunkAndReport`), so a diagnostic that only lived in + // the sweep could not be reached by the one path grom actually clicks — which is why + // his report of greys not selling came back with nothing in the log. + // + // Said once per distinct answer (`record_warning_once` keys on the text), and only + // when there is something to explain: a truthful zero with nothing unresolved is + // silent. + if grey.is_empty() && !unjudgeable.is_empty() { + let line = unjudgeable_line(&unjudgeable); + model.record_warning_once(format!( + "GetNumJunkItems answered 0 of {occupied} occupied carried slot(s), and {line}" + )); + } + Ok(grey.len() as i64) })?, )?; @@ -125,53 +199,32 @@ pub(super) fn install(lua: &Lua) -> mlua::Result
{ "SellAllJunkItems", lua.create_function(|lua, ()| { let mut model = lua.app_data_mut::().expect("model app_data"); - let junk: Vec<(i64, u32)> = CARRIED_BAGS - .iter() - .filter_map(|bag| model.containers.get(bag).map(|c| (*bag, c))) - .flat_map(|(bag, c)| { - c.slots - .iter() - .filter(|(_, s)| s.item_id != 0 && s.quality == Some(QUALITY_POOR)) - .map(move |(slot, _)| (bag, *slot)) - }) - .collect(); - - // Every occupied slot this sweep could not judge, because the gate above is a - // **known** quality and `ContainerSlot::quality` is `None` until the item's template - // is in hand (`ui_items::feed`'s no-template arm: "show the slot occupied while the - // answer is in flight"). Such a slot is invisible to `GetNumJunkItems` as well, so a - // grey sitting in one is neither counted nor sold — and "not all grey items sell on - // the sell-junk button" is a defect grom reported from play. - // - // This says so instead of guessing which items they were. One line per distinct set, - // on the **click** rather than per frame, naming the item ids so the next session's - // log answers it: if the list is empty the cause is elsewhere (the slot → guid - // resolution in `ui_merchant`'s drain, or a server that refuses the sale), and if it - // is not, the fix is to wait for those templates before judging the sweep. - let unknown: Vec<(i64, u32, u32)> = CARRIED_BAGS - .iter() - .filter_map(|bag| model.containers.get(bag).map(|c| (*bag, c))) - .flat_map(|(bag, c)| { - c.slots - .iter() - .filter(|(_, s)| s.item_id != 0 && s.quality.is_none()) - .map(move |(slot, s)| (bag, *slot, s.item_id)) - }) - .collect(); - if !unknown.is_empty() { - let shown: Vec = unknown - .iter() - .take(12) - .map(|(bag, slot, id)| format!("({bag},{slot}) item {id}")) - .collect(); - model.record_warning_once(format!( - "SellAllJunkItems: {} carried slot(s) have no item template yet, so their quality is unknown and this sweep could neither count nor sell them — a grey among them does not sell: {}{}", - unknown.len(), - shown.join(", "), - if unknown.len() > 12 { ", …" } else { "" } - )); + let JunkSurvey { + grey: junk, + unjudgeable, + occupied, + } = junk_survey(&model); + // Every occupied slot this sweep could not judge, by the same survey the count uses — + // the two must not be able to disagree, or a button gated on the count sells a + // different set than it promised. + if !unjudgeable.is_empty() { + let line = unjudgeable_line(&unjudgeable); + model.record_warning_once(format!("SellAllJunkItems: {line}")); } - + // And what it did queue, which is the other half of the answer: if the sweep names + // slots and nothing sells, the fault is past here — in the slot → item-guid + // resolution (`ui_merchant`'s drain, which warns when it finds none) or in a server + // that refuses the sale. + model.record_warning_once(format!( + "SellAllJunkItems queued {} grey stack(s) of {occupied} occupied carried slot(s): \ + {}", + junk.len(), + junk.iter() + .take(12) + .map(|(b, sl)| format!("({b},{sl})")) + .collect::>() + .join(", "), + )); model.merchant_junk_sells.extend(junk); Ok(()) })?, @@ -363,6 +416,74 @@ mod tests { ); } + /// **The count is the gate, and it has to speak for itself.** + /// + /// pfUI's Sell Grey Items button is `if C_MerchantFrame.GetNumJunkItems() <= 0 then return + /// end` and only then `SellAllJunkItems` (`modules/autovendor.lua`). So on the one path grom + /// actually clicks, a zero count means the sweep never runs — and a diagnostic that lived only + /// in the sweep could not be reached. His report came back with an empty log for exactly that + /// reason. + /// + /// A truthful zero stays silent. A zero with occupied slots whose quality has not resolved + /// says so, and names them. + #[test] + fn a_zero_junk_count_says_whether_it_is_truthful() { + let slot = |item_id: u32, quality: Option| ContainerSlot { + item_id, + quality, + ..Default::default() + }; + let seat = |s: &mut crate::script::UiScript, rows: Vec<(u32, ContainerSlot)>| { + let mut slots = HashMap::new(); + for (k, v) in rows { + slots.insert(k, v); + } + s.set_container( + 0, + Some(ContainerState { + name: None, + num_slots: 16, + slots, + }), + ); + }; + + // Nothing grey, nothing in doubt: silence. + let mut s = merchant(); + seat( + &mut s, + vec![(1, slot(858, Some(1))), (2, slot(859, Some(2)))], + ); + assert_eq!( + s.eval::("return C_MerchantFrame.GetNumJunkItems()") + .unwrap(), + 0 + ); + assert!( + s.take_warnings().is_empty(), + "a bag of knowns with no greys is a truthful zero and must not be narrated" + ); + + // Nothing grey *that it can see*, and two slots it cannot judge: it says so. + let mut s = merchant(); + seat(&mut s, vec![(1, slot(1234, None)), (2, slot(5678, None))]); + assert_eq!( + s.eval::("return C_MerchantFrame.GetNumJunkItems()") + .unwrap(), + 0, + "an unresolved slot cannot be counted as junk" + ); + let warnings = s.take_warnings().join("\n"); + assert!( + warnings.contains("GetNumJunkItems answered 0 of 2 occupied carried slot(s)"), + "the gate must say the zero was not necessarily truthful: {warnings}" + ); + assert!( + warnings.contains("item 1234") && warnings.contains("item 5678"), + "…and name the slots, so the next log answers which items they were: {warnings}" + ); + } + #[test] fn merchant_item_info_is_a_named_table_over_the_stock_row() { let s = merchant();