grom/benilla-octo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

The sell-junk diagnostic sat behind the gate that fails

Browse Source 
He hit sell junk again and the log said nothing — not because the sweep had
nothing to report, but because the sweep never ran.

pfUI's Sell Grey Items button is `if GetNumJunkItems() <= 0 then return end`
and only then SellAllJunkItems (modules/autovendor.lua). That is the one path
he clicks, so a zero count ends the click there. Last session's instrument
lived inside SellAllJunkItems, downstream of the thing that was answering
zero, and it could not be reached.

Both now share one junk_survey of the carried bags, so the count and the
sweep can never disagree about what they are looking at, and each says its
half:

  GetNumJunkItems, when it answers zero while occupied slots have no resolved
  quality, says so and names them — a truthful zero stays silent;
  SellAllJunkItems says what it queued as well as what it could not judge;
  and ui_merchant's drain now warns, rather than debugs, when a queued slot
  resolves to no item guid. That was the last silent place the click could
  die, and debug does not reach his log.

So whichever of the three branches is at fault, his next click answers it.

Also checked and cleared on the way: the drain's slot.saturating_sub(1) is
right — the feed keys container slots 1-based.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
grom
2026-10-29 09:36:04 -06:00
co-authored by Claude Opus 5
parent 7ebae975d9
commit fff48cd75c
2 changed files with 184 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/benilla-app/src/ui_merchant/mod.rs
+9 -1
View File
@@ -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
crates/benilla-ui/src/script/compat/classic_api/merchant_frame.rs
+175 -54
View File
@@ -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<String> = 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<Table> {
let t = lua.create_table()?;
@@ -97,14 +157,28 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<Table> {
t.set(
"GetNumJunkItems",
lua.create_function(|lua, ()| {
let model = lua.app_data_ref::<Model>().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::<Model>().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<Table> {
"SellAllJunkItems",
lua.create_function(|lua, ()| {
let mut model = lua.app_data_mut::<Model>().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<String> = 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::<Vec<_>>()
.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<u32>| 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::<i64>("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::<i64>("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();