diff --git a/crates/benilla-app/src/entities/attach/preview.rs b/crates/benilla-app/src/entities/attach/preview.rs index dfa181f2..b5b88ba9 100644 --- a/crates/benilla-app/src/entities/attach/preview.rs +++ b/crates/benilla-app/src/entities/attach/preview.rs @@ -417,87 +417,106 @@ pub(in crate::entities) fn build_pane_previews( .by_pane .retain(|pane, _| preview.panes.contains_key(pane)); - let Some((pane, look)) = next_stale_pane(pool.pooled_panes(), &preview.panes, &bakes.by_pane) - else { - return; - }; let Some(creatures) = creatures.as_deref() else { return; }; - let spec = pane_spec(&look); - let Some(a) = assemble( - &spec, - &mut PreviewCtx { - creatures, - characters: characters.as_deref(), - displays: displays.as_deref_mut(), - glows: glows.as_deref_mut(), - sections: sections.as_deref(), - world_assets: world_assets.as_deref(), - images: &mut images, - skin_composites: &mut skin_composites, - asset_server: &asset_server, - mats: &mut mats, - }, - ) else { - return; // an item model is still loading: retry next frame + // Every stale pane this frame may try, in slot order, collected because the assembly below + // needs `bakes` mutably to write its result. + let stale: Vec<_> = stale_panes(pool.pooled_panes(), &preview.panes, &bakes.by_pane).collect(); + let mut ctx = PreviewCtx { + creatures, + characters: characters.as_deref(), + displays: displays.as_deref_mut(), + glows: glows.as_deref_mut(), + sections: sections.as_deref(), + world_assets: world_assets.as_deref(), + images: &mut images, + skin_composites: &mut skin_composites, + asset_server: &asset_server, + mats: &mut mats, }; - let revision = bakes - .by_pane - .get(&pane) - .map_or(0, |bake| bake.revision) - .wrapping_add(1); - // The equipment fingerprint, not just the counts: two panes wearing different boots assemble - // the same number of parts, so counts cannot tell "every pane composed its own look" from - // "every pane got one pane's look" — which is exactly the question a page of identical-looking - // previews asks. - let worn: Vec = look - .equipment - .iter() - .enumerate() - .filter(|(_, item)| item.display_id != 0) - .map(|(slot, item)| format!("{slot}:{}", item.display_id)) - .collect(); - debug!( - "pane preview: pane {pane:?} race {} display {} → {} parts, {} riders, {} effect model(s); \ - worn [{}]", - look.race, - look.display_id, - a.parts.len(), - a.riders.len(), - a.effects.len(), - worn.join(" "), - ); - bakes.by_pane.insert( - pane, - DressUpBake { - look: Some(look), - display_id: look.display_id, - parts: a.parts, - riders: a.riders, - effects: a.effects, - billboards: a.billboards, - grip: a.grip, - revision, - }, - ); + // One assembly a frame, but the *first that can be made*, not the first that is stale. + // + // A pane whose assembly cannot complete is chosen again every frame — it stays stale by + // definition — so giving the frame up on it starved every pane behind it in slot order for as + // long as its model refused to resolve. One item doing that (the shop's Smoky Wooden Torch) + // left the rest of the page on the mirrored live player, which looks exactly like the + // per-pane dress-up having failed. Walking past it costs nothing: every `assemble` bail is a + // lookup before the skin composite, the expensive half. + for (pane, look) in stale { + let spec = pane_spec(&look); + let Some(a) = assemble(&spec, &mut ctx) else { + continue; // an item model is still loading: the pane behind it can still have its turn + }; + let revision = bakes + .by_pane + .get(&pane) + .map_or(0, |bake| bake.revision) + .wrapping_add(1); + // The equipment fingerprint, not just the counts: two panes wearing different boots + // assemble the same number of parts, so counts cannot tell "every pane composed its own + // look" from "every pane got one pane's look" — which is exactly the question a page of + // identical-looking previews asks. + let worn: Vec = look + .equipment + .iter() + .enumerate() + .filter(|(_, item)| item.display_id != 0) + .map(|(slot, item)| format!("{slot}:{}", item.display_id)) + .collect(); + debug!( + "pane preview: pane {pane:?} race {} display {} → {} parts, {} riders, {} effect \ + model(s); worn [{}]", + look.race, + look.display_id, + a.parts.len(), + a.riders.len(), + a.effects.len(), + worn.join(" "), + ); + bakes.by_pane.insert( + pane, + DressUpBake { + look: Some(look), + display_id: look.display_id, + parts: a.parts, + riders: a.riders, + effects: a.effects, + billboards: a.billboards, + grip: a.grip, + revision, + }, + ); + return; + } } -/// The first pooled pane whose assembled bake is missing or no longer matches its look, in slot -/// order; `None` when every drawn pane is current. +/// Every pooled pane whose assembled bake is missing or no longer matches its look, in slot +/// order; empty when every drawn pane is current. /// /// Pulled out of the system so the choice is testable without a world: "one pane per frame" is /// only safe if the choice also *advances*, and the failure mode if it does not — one pane /// re-assembled forever while the rest of the page stays blank — looks identical to an item model -/// that will not load. +/// that will not load. The caller walks this in order and stops at the first pane it can actually +/// assemble, so a pane that is permanently unassemblable costs its own turn and nothing else. +fn stale_panes<'a>( + pooled: impl Iterator + 'a, + looks: &'a std::collections::HashMap, + bakes: &'a std::collections::HashMap, +) -> impl Iterator + 'a { + pooled + .filter_map(|pane| looks.get(&pane).map(|look| (pane, *look))) + .filter(|(pane, look)| bakes.get(pane).is_none_or(|bake| bake.look != Some(*look))) +} + +/// The first of [`stale_panes`]; the shape the choice tests ask about. +#[cfg(test)] fn next_stale_pane( pooled: impl Iterator, looks: &std::collections::HashMap, bakes: &std::collections::HashMap, ) -> Option<(benilla_ui::widget::FrameHandle, DressUpLook)> { - pooled - .filter_map(|pane| looks.get(&pane).map(|look| (pane, *look))) - .find(|(pane, look)| bakes.get(pane).is_none_or(|bake| bake.look != Some(*look))) + stale_panes(pooled, looks, bakes).next() } /// A pane's look as an assembly spec. The flag choices are [`build_dressup_preview`]'s, for the @@ -1401,7 +1420,7 @@ mod pane_preview_tests { use benilla_ui::widget::{FrameKind, WidgetArena}; - use super::{next_stale_pane, DressUpBake, DressUpLook}; + use super::{next_stale_pane, stale_panes, DressUpBake, DressUpLook}; /// Two real handles from a real arena: `FrameHandle` is generational and has no public /// constructor, which is the right call — a handle the arena never minted could not resolve. @@ -1469,6 +1488,35 @@ mod pane_preview_tests { ); } + /// Every stale pane is offered, in slot order — not just the first. + /// + /// The caller assembles one pane a frame and walks this list until one of them can actually + /// be made. A pane whose item model never resolves is stale forever and is therefore offered + /// first forever, so if the list stopped at it, every pane behind it would wait on it: the + /// shop's Smoky Wooden Torch kept a whole page of cards on the mirrored live player that way. + #[test] + fn a_pane_that_cannot_assemble_does_not_hide_the_ones_behind_it() { + let mut arena = WidgetArena::default(); + let stuck = arena.create(FrameKind::Frame, None, None); + let (b, c) = ( + arena.create(FrameKind::Frame, None, None), + arena.create(FrameKind::Frame, None, None), + ); + let looks: HashMap<_, _> = [(stuck, look(10)), (b, look(20)), (c, look(30))] + .into_iter() + .collect(); + let bakes = HashMap::new(); + + let offered: Vec<_> = stale_panes([stuck, b, c].into_iter(), &looks, &bakes) + .map(|(pane, _)| pane) + .collect(); + assert_eq!( + offered, + vec![stuck, b, c], + "the pane that cannot be assembled comes first and the rest follow it" + ); + } + /// Nothing to do once every drawn pane matches its look — the steady state a settled shop /// page sits in, where this system must cost nothing. #[test] diff --git a/crates/benilla-app/src/portrait/mod.rs b/crates/benilla-app/src/portrait/mod.rs index ad8f258c..c7ae8d39 100644 --- a/crates/benilla-app/src/portrait/mod.rs +++ b/crates/benilla-app/src/portrait/mod.rs @@ -2214,6 +2214,9 @@ fn sync_body_booths( mut cams: Query<(&BoothCam, &mut Transform, &mut Projection)>, framing_in: BoothFraming, anim_data: Option>, + // The last reason each pooled slot drew what it drew, so `WOW_BOOTH_LOG` prints one line per + // change instead of one a frame. + mut pane_source: Local>, ) { if test_mode(&mut env_cache) { return; // the test bake owns the booths @@ -2270,6 +2273,18 @@ fn sync_body_booths( // path below can only ever show what the player is actually wearing, which is why a page of // cards used to be a page of the same character and the Fashion tab could not show an item // on you. When `build_pane_previews` has assembled this pane's look, stage that instead. + // Why this slot draws what it draws, logged under `WOW_BOOTH_LOG` and only when it + // changes. A pooled pane that falls through to the mirror shows the live player, and on a + // page of cards that is indistinguishable by eye from the per-pane dress-up never having + // run at all — so the one thing the log has to say is *which* of the three it is. + let mut why = match claim { + None => "idle: no pane claims this slot", + Some(c) => match subjects.bakes.by_pane.get(&c.pane) { + None => "mirror: no bake (its room is closed, or it has not assembled yet)", + Some(bake) if bake.parts.is_empty() => "mirror: the bake assembled to no parts", + Some(_) => "staged: its own look", + }, + }; if let Some((pane, staged)) = claim .map(|c| c.pane) .and_then(|pane| subjects.bakes.by_pane.get(&pane).map(|bake| (pane, bake))) @@ -2293,11 +2308,14 @@ fn sync_body_booths( &framing_in.panes, last_pose.entry(slot).or_default(), ) { + log_pane_source(slot, claim.map(|c| c.pane), why, &mut pane_source); continue; } // Not ready (a material or the rig is still loading): fall through to the mirror for // this frame rather than leaving the pane blank, and restage when it lands. + why = "mirror: the bake's rig or a material is still loading"; } + log_pane_source(slot, claim.map(|c| c.pane), why, &mut pane_source); sync_body_booth( &mut palettes, slot, @@ -2322,6 +2340,19 @@ fn sync_body_booths( } } +/// `WOW_BOOTH_LOG=1`: one line whenever a pooled slot changes where its character comes from. +fn log_pane_source( + slot: &'static str, + pane: Option, + why: &'static str, + seen: &mut HashMap<&'static str, &'static str>, +) { + if !booth_log() || seen.insert(slot, why) == Some(why) { + return; + } + eprintln!("[booth] pane source slot={slot} pane={pane:?} {why}"); +} + /// Does the booth have to be rebuilt for this pane's bake? /// /// Keyed on the **pane and** the revision, never the revision alone: every pane numbers its own