diff --git a/crates/benilla-app/src/cinematic.rs b/crates/benilla-app/src/cinematic.rs index 4dadd52c..687a09fe 100644 --- a/crates/benilla-app/src/cinematic.rs +++ b/crates/benilla-app/src/cinematic.rs @@ -238,7 +238,7 @@ impl Plugin for CinematicPlugin { feed_ui .in_set(WorldStage::Input) .after(drive) - .run_if(not(crate::ui_script::ingame_ui_pending)), + .run_if(crate::ui_script::ingame_ui_up), ) // A cinematic cannot outlive the world it was flying over: leaving drops it silently, // with no ack, exactly as the reference's own leave-world teardown does (`0x490a80` diff --git a/crates/benilla-app/src/game_tip.rs b/crates/benilla-app/src/game_tip.rs index 824cf5f9..810935f7 100644 --- a/crates/benilla-app/src/game_tip.rs +++ b/crates/benilla-app/src/game_tip.rs @@ -74,6 +74,7 @@ //! drawn as one string. use benilla_ui::markup::{self, TokenKind}; +use bevy::ecs::system::EntityCommands; use bevy::prelude::*; use benilla_formats::GameTipsCatalog; @@ -287,6 +288,26 @@ impl Plugin for GameTipPlugin { } } +/// **Putting the tip line away — one path, because there were two and they had drifted.** +/// +/// Two callers empty this node and both want the same end state: hidden, childless, and holding no +/// text. Only the second of them used to clear the ROOT's own run, and that difference crashed the +/// client (B383, decision 2212). Despawning a `Text` root's last `TextSpan` child does not *change* +/// `Children` — it REMOVES the component — and bevy 0.18's `detect_text_needs_rerender` watches +/// `Changed`, which a removal cannot satisfy. So the dismiss left a two-run shaped buffer +/// behind a one-run span list, and the first window resize after world entry re-laid-out that +/// buffer and indexed a run that no longer existed: an out-of-bounds panic inside `bevy_text`, on +/// a node nobody could see. Re-inserting `Text` is what tells the pipeline the block moved. +/// +/// [`crate::text_reshape`] nets the same hole app-wide; this is the site being coherent on its own +/// rather than leaning on the net, and it is the half that also keeps a hidden tip from holding a +/// stale sentence. +fn empty_tip(e: &mut EntityCommands, vis: &mut Visibility) { + *vis = Visibility::Hidden; + e.despawn_related::(); + e.insert(Text::new(String::new())); +} + /// Take the raise's tip edge, then paint whatever the screen is showing. /// /// Two jobs in one system because they are one mechanism seen at two moments: the reference picks @@ -351,8 +372,7 @@ fn drive_game_tip( return; }; let Some(tip) = tips.shown() else { - *vis = Visibility::Hidden; - commands.entity(entity).despawn_related::(); + empty_tip(&mut commands.entity(entity), &mut vis); return; }; // The 4:3 content box is `100vh` tall and `100vh · 4/3` wide, so the window's height is the @@ -373,9 +393,9 @@ fn drive_game_tip( let runs = spans(tip, base_color()); let painted = !runs.is_empty(); let mut e = commands.entity(entity); - e.despawn_related::(); match runs.split_first() { Some(((head, head_color), rest)) => { + e.despawn_related::(); e.insert((Text::new(head.clone()), TextColor(*head_color))); let (rest, tf) = (rest.to_vec(), font.clone()); e.with_children(|c| { @@ -384,10 +404,7 @@ fn drive_game_tip( } }); } - None => { - e.insert(Text::new(String::new())); - *vis = Visibility::Hidden; - } + None => empty_tip(&mut e, &mut vis), } // The run's own evidence — and it sits HERE, past the query, the layout and the runs, because @@ -549,11 +566,14 @@ mod tests { ); } - /// An app holding the real node and the real system, one tip in the table. + /// An app holding the real node and the real system, one tip in the table — plus the text + /// stack and the real `detect_text_needs_rerender`, so what the paint and the dismiss do to + /// the node's `ComputedTextBlock` is observable (B383, decision 2212). fn tip_app() -> (App, Entity) { let mut app = App::new(); app.add_plugins((MinimalPlugins, bevy::asset::AssetPlugin::default())); - app.init_asset::(); + crate::text_reshape::harness::add_text_plugins(&mut app); + app.add_systems(PostUpdate, bevy::text::detect_text_needs_rerender::); app.init_resource::(); app.init_resource::(); app.insert_resource(GameTips { @@ -668,4 +688,45 @@ mod tests { .get::(tip) .is_none_or(|c| c.is_empty())); } + + /// **B383 — the dismiss must leave the tip node re-shapeable.** The loading screen's root is + /// never despawned, only hidden, so the tip node outlives every load and every later window + /// resize re-lays-out its text block. Emptying it by despawning the spans alone left the + /// block's shaped buffer pointing at a run that no longer existed, and the first maximize + /// after world entry panicked inside `bevy_text`. The observable: after the dismiss the root + /// holds no text and the block is marked for a re-shape. + /// + /// The mechanism's own end — that the re-shape is what stops the panic — is + /// [`crate::text_reshape`]'s, which drives the real pipeline over the same shape. + #[test] + fn the_dismiss_leaves_the_node_reshapeable() { + let (mut app, tip) = tip_app(); + set_edge(&mut app, crate::loading_screen::TipEdge::Pick); + app.update(); + // The state a painted frame leaves behind: a two-run block, shaped, nothing pending. + crate::text_reshape::harness::shape(&mut app, tip); + assert_eq!( + app.world() + .get::(tip) + .map(|b| b.entities().len()), + Some(2), + "the gold prefix and the sentence are two runs" + ); + + set_edge(&mut app, crate::loading_screen::TipEdge::Clear); + app.update(); + + assert_eq!( + app.world().get::(tip).map(|t| t.0.as_str()), + Some(""), + "the dismissed tip holds no sentence — a hidden node with stale text is the bug" + ); + assert!( + app.world() + .get::(tip) + .expect("the block") + .needs_rerender(), + "and the block is marked for the re-shape a later resize would otherwise skip" + ); + } } diff --git a/crates/benilla-app/src/lib.rs b/crates/benilla-app/src/lib.rs index e003bf72..2fa1624f 100644 --- a/crates/benilla-app/src/lib.rs +++ b/crates/benilla-app/src/lib.rs @@ -118,6 +118,7 @@ mod sound; mod swing_refusal; mod target; mod text_filter; +mod text_reshape; mod textinput; mod transport; mod tutorial; @@ -672,6 +673,10 @@ pub fn run(build: BuildId) -> AppExit { .add_plugins(BattlefieldPositionsPlugin) .add_plugins(crate::game_tip::GameTipPlugin) .add_plugins(crate::text_filter::TextFilterPlugin) + // The re-shape a `bevy_ui` text root loses when its last span is despawned + // (decision 2212, B383): an upstream change-detection hole whose only symptom is a + // panic inside `bevy_text` on the next window resize. + .add_plugins(crate::text_reshape::TextReshapePlugin) .add_plugins(TutorialPlugin) // The melee swing refusals (`SMSG_ATTACKSWING_NOTINRANGE`/`_BADFACING`/`_DEADTARGET`/ // `_CANT_ATTACK`): the latch the packets set, and the 4 s repeat that shows it while an diff --git a/crates/benilla-app/src/perf/gpu.rs b/crates/benilla-app/src/perf/gpu.rs index d597f16f..50d1b8fa 100644 --- a/crates/benilla-app/src/perf/gpu.rs +++ b/crates/benilla-app/src/perf/gpu.rs @@ -176,6 +176,15 @@ struct GpuStamp { sentinel_view: wgpu::TextureView, resolve: Buffer, ring: Vec<(Buffer, Arc)>, + /// Frames this resource has seen a `readback` for. **Zero means the stamps have never been + /// written**: `init_stamp` inserts the resource in `Cleanup`, the scheduler's sync point + /// applies it before `readback` runs in the same set, and the sentinel nodes that write — + /// and, in wgpu, reset — the two queries only run in the NEXT frame's graph. A resolve on + /// that first pass reads a query pool no command has ever reset, which Vulkan forbids + /// (`VUID-vkCmdCopyQueryPoolResults-None-09402`) and answers, on NVIDIA's driver, by + /// waiting on it forever — a GPU hang, the TDR, and the lost device 2205 met at the + /// swapchain acquire. Metal hands back zeros for the same read, which is why the meter's + /// first live runs (1389) saw "zeros" and never a hang. Decision 2211. frame: usize, period: f32, shared: Arc, @@ -304,6 +313,12 @@ fn init_stamp( /// command buffer — the 1389 trap), kick the map, and publish the oldest mapped delta. fn readback(stamp: Option>, device: Res, queue: Res) { let Some(mut stamp) = stamp else { return }; + // The first cleanup after creation precedes the first stamped frame: nothing to resolve + // yet, and resolving anyway is the Vulkan device loss above (see `frame`). + if stamp.frame == 0 { + stamp.frame = 1; + return; + } // Copy into a FREE slot only (none free → skip this frame's sample; the meter is a meter). if let Some(i) = (0..RING).find(|&i| stamp.ring[i].1.load(Ordering::Relaxed) == FREE) { let mut encoder = device.create_command_encoder(&CommandEncoderDescriptor { diff --git a/crates/benilla-app/src/text_reshape.rs b/crates/benilla-app/src/text_reshape.rs new file mode 100644 index 00000000..266e2b69 --- /dev/null +++ b/crates/benilla-app/src/text_reshape.rs @@ -0,0 +1,370 @@ +//! **The re-shape a `bevy_ui` text root loses when its LAST span is despawned** — an upstream +//! change-detection hole, netted here because falling into it is a hard crash whose panic names +//! none of our code (decision 2212; bug B383, Goudy: *"if you enter world in window mode and then +//! maximize window on linux, the client will crash"*). +//! +//! ## The hole +//! +//! A `bevy_ui` text block is a `Text` root plus its `TextSpan` children. Two pieces of state hang +//! off the root and must agree: +//! +//! - `ComputedTextBlock::buffer` — the shaped cosmic-text buffer, whose every laid-out glyph +//! carries the **index of the run it came from** in its `metadata`; +//! - `ComputedTextBlock::entities` — the span list that index is an index *into*, rebuilt only by +//! `TextPipeline::update_buffer`. +//! +//! `bevy_text::detect_text_needs_rerender` is what asks for a re-shape, and on the root it watches +//! `Changed`, `Changed`, `Changed`, `Changed` and +//! `Changed`. **Despawning the last child does not *change* `Children` — it REMOVES +//! it** (`bevy_ecs`'s relationship `on_replace` hook queues a `remove::()` once the +//! collection empties), and `Changed` cannot fire for a component the entity no longer has. So +//! a root emptied of spans keeps `needs_rerender == false`, keeps its multi-run buffer, and keeps +//! an `entities` list naming dead entities. +//! +//! Nothing goes wrong until the node is re-laid-out **without** being re-shaped, which is exactly +//! what `widget::text_system` does on any frame where the node's `ComputedNode` changed — a window +//! resize, in other words. `TextPipeline::update_text_layout_info` rebuilds its per-span +//! `glyph_info` by `iter_many`-ing the stored `entities`, and `iter_many` *skips* the dead ones, so +//! the list comes back short while the buffer's glyphs still carry `metadata == 1`. The next line +//! is `self.glyph_info[span_index]` (`bevy_text-0.18.1/src/pipeline.rs:398`) and the client is +//! gone. Nothing about this is platform-specific: Linux is simply where somebody maximized a +//! window after the loading screen had put its tip away. +//! +//! ## The net +//! +//! [`reshape_emptied_text_roots`] restores the invariant the upstream detector cannot see: a +//! `Text` root that just lost its `Children` component is marked changed, one system before the +//! detector runs, so the block is re-shaped in the same frame and can never be re-laid-out stale. +//! +//! It is a **net, not the mechanism** — the site that emptied a node is still expected to leave it +//! coherent (`game_tip::empty_tip` is the one that did not, and it does now). The net exists +//! because the failure mode is a crash rather than a wrong pixel, and because the panic points at +//! third-party code: B383 cost a pastebin core dump and a session to attribute. +//! +//! **Scope: `bevy_ui` text only.** `bevy_sprite`'s `Text2d` has the identical hole and no entry +//! here, because this tree spawns none — and the in-game interface's own `FontString`s do not go +//! through `bevy_ui` at all (they shape through `cosmic-text` under the UI pass, 0068 §2). What is +//! left on `bevy_ui::Text` is the glue screens, the debug panel, and the loading screen's tip. + +use bevy::prelude::*; +use bevy::text::detect_text_needs_rerender; + +pub(crate) struct TextReshapePlugin; + +impl Plugin for TextReshapePlugin { + fn build(&self, app: &mut App) { + app.add_systems( + PostUpdate, + // Ordered against the detector itself rather than against `UiSystems::Content`: the + // one thing that has to be true is that the mark lands before the detector reads it, + // and naming the system says so where a set name would only imply it. + reshape_emptied_text_roots.before(detect_text_needs_rerender::), + ); + } +} + +/// Mark a `Text` root changed when it loses its `Children` — see the module header. +/// +/// `RemovedComponents` fires for every entity in the world that shed a hierarchy this +/// frame (models, portraits, the streamed scene), so the body is a single archetype lookup that +/// misses for all of them; only a live text root reaches the write. +fn reshape_emptied_text_roots( + mut emptied: RemovedComponents, + mut roots: Query<&mut Text>, +) { + for entity in emptied.read() { + if let Ok(mut text) = roots.get_mut(entity) { + text.set_changed(); + } + } +} + +/// **The two `bevy_ui` calls that bracket a text node, driven by hand** — so a test can put a text +/// block into the state a real frame leaves it in, and then run the frame that used to crash. +/// +/// `bevy_ui`'s own `UiPlugin` cannot: it wants a render app, a camera and a window to produce the +/// `ComputedNode` change a resize is. What it does *around* the node is only these two pipeline +/// calls, in this order, and both are `pub` — so the reproduction is exact without any of that. +#[cfg(test)] +pub(crate) mod harness { + use bevy::ecs::system::RunSystemOnce; + use bevy::image::TextureAtlasLayout; + use bevy::prelude::*; + use bevy::text::{ + detect_text_needs_rerender, ComputedTextBlock, CosmicFontSystem, Font, FontAtlasSet, + FontHinting, SwashCache, TextBounds, TextLayoutInfo, TextPipeline, + }; + use bevy::ui::widget::TextUiReader; + + /// A node box wide enough that the test strings lay out on one line and narrow enough that the + /// numbers stay small. Nothing in the mechanism depends on it. + const BOUNDS: TextBounds = TextBounds::new(400.0, 200.0); + + /// Everything a headless text block needs: `TextPlugin` for the pipeline resources and its + /// embedded default font (the only face an `AssetServer`-less test can shape with), plus the + /// two asset stores the glyph atlas fills. + pub(crate) fn add_text_plugins(app: &mut App) { + app.add_plugins(bevy::text::TextPlugin); + app.init_asset::(); + app.init_asset::(); + } + + /// Point the root and every span at the embedded default font, so [`shape`] can run without an + /// asset load. Runtime resolves the real face off the patch chain instead. + /// + /// **`bypass_change_detection`, and the test suite does not work without it.** This stands in + /// for the face the runtime would have resolved when the node was spawned, so it must not read + /// as a `TextFont` change — `detect_text_needs_rerender` watches exactly that, and a harness + /// that quietly asks for a re-shape every time it shapes cannot observe a missing one. It cost + /// three of these tests a false green before the flag went on. + fn use_default_font(app: &mut App, root: Entity) { + let spans: Vec = app + .world() + .get::(root) + .map(|c| c.to_vec()) + .unwrap_or_default(); + for entity in core::iter::once(root).chain(spans) { + if let Some(mut font) = app.world_mut().get_mut::(entity) { + font.bypass_change_detection().font = Handle::default(); + } + } + } + + /// Shape the block, the way `widget::measure_text_system` does — the call that (re)fills + /// `ComputedTextBlock::entities` and clears `needs_rerender`. + pub(crate) fn shape(app: &mut App, root: Entity) { + use_default_font(app, root); + app.world_mut() + .run_system_once_with(shape_one, root) + .expect("the shaping system runs"); + } + + fn shape_one( + In(root): In, + mut pipeline: ResMut, + fonts: Res>, + mut font_system: ResMut, + mut blocks: Query<(&TextLayout, &mut ComputedTextBlock, &FontHinting)>, + mut reader: TextUiReader, + ) { + let (layout, mut computed, hinting) = blocks.get_mut(root).expect("a text root"); + pipeline + .update_buffer( + &fonts, + reader.iter(root), + layout.linebreak, + layout.justify, + BOUNDS, + 1.0, + &mut computed, + &mut font_system, + *hinting, + ) + .expect("the embedded default font shapes headless"); + } + + /// **The frame a window resize is**, in the order `UiPlugin` runs it: the detector, then the + /// re-shape it asked for (`measure_text_system`), then the re-layout every node whose + /// `ComputedNode` moved gets (`text_system`). The last of the three is what panicked. + /// + /// Any system the app has scheduled `.before(detect_text_needs_rerender::)` — the net in + /// this module, when the plugin is installed — runs first, exactly as it would live. + pub(crate) fn resize_frame(app: &mut App, root: Entity) { + app.update(); + if app + .world() + .get::(root) + .is_some_and(ComputedTextBlock::needs_rerender) + { + shape(app, root); + } + app.world_mut() + .run_system_once_with(relayout_one, root) + .expect("the relayout system runs"); + } + + #[allow(clippy::too_many_arguments)] + fn relayout_one( + In(root): In, + mut pipeline: ResMut, + mut atlas_set: ResMut, + mut atlas_layouts: ResMut>, + mut images: ResMut>, + mut font_system: ResMut, + mut swash_cache: ResMut, + text_font_query: Query<&TextFont>, + mut blocks: Query<(&TextLayout, &mut TextLayoutInfo, &mut ComputedTextBlock)>, + ) { + let (layout, mut info, mut computed) = blocks.get_mut(root).expect("a text root"); + pipeline + .update_text_layout_info( + &mut info, + text_font_query, + 1.0, + &mut atlas_set, + &mut atlas_layouts, + &mut images, + &mut computed, + &mut font_system, + &mut swash_cache, + BOUNDS, + layout.justify, + ) + .expect("the relayout succeeds"); + } + + /// An app with the text stack and the real detector scheduled where `UiPlugin` puts it. + /// `net` installs [`super::TextReshapePlugin`] — the two tests below run the same world with + /// and without it. + pub(crate) fn text_app(net: bool) -> App { + let mut app = App::new(); + app.add_plugins((MinimalPlugins, bevy::asset::AssetPlugin::default())); + add_text_plugins(&mut app); + app.add_systems(PostUpdate, detect_text_needs_rerender::); + if net { + app.add_plugins(super::TextReshapePlugin); + } + app + } +} + +#[cfg(test)] +mod tests { + use super::harness::{resize_frame, shape, text_app}; + use bevy::prelude::*; + use bevy::text::ComputedTextBlock; + + /// A two-run block: the root's own text plus one `TextSpan` child, which is the shape every + /// coloured string in this tree has (`|cffffd100Tip:|r …`, the glue's markup runs). + fn two_run_root(app: &mut App) -> Entity { + let root = app + .world_mut() + .spawn(( + Text::new("Tip:"), + TextLayout::default(), + TextFont::default(), + TextColor::WHITE, + )) + .with_children(|c| { + c.spawn(( + TextSpan::new(" talk to the innkeeper."), + TextFont::default(), + )); + }) + .id(); + app.update(); + shape(app, root); + assert_eq!( + app.world() + .get::(root) + .map(|b| b.entities().len()), + Some(2), + "the shaped block lists both runs" + ); + root + } + + /// **The hole itself, named at the level the crash needs.** Despawning the last span leaves + /// `needs_rerender` false — `Children` was removed, not changed — so `bevy_ui` will re-lay-out + /// a buffer whose glyphs point at a run that no longer exists. This asserts the *broken* + /// upstream behaviour on purpose: it is the premise the net and every emptying site rest on, + /// and a bevy release that closes the hole should fail here and retire both. + #[test] + fn upstream_misses_the_last_spans_despawn() { + let mut app = text_app(false); + let root = two_run_root(&mut app); + + app.world_mut() + .entity_mut(root) + .despawn_related::(); + app.update(); + + assert!( + app.world().get::(root).is_none(), + "the last child's despawn REMOVES `Children`, which is the whole mechanism" + ); + assert!( + !app.world() + .get::(root) + .expect("the block") + .needs_rerender(), + "upstream cannot see the removal — if this ever fails, bevy fixed it" + ); + } + + /// **The crash itself.** The same sequence with the net taken out is the client Goudy ran: + /// the resize frame re-lays-out the stale buffer and `bevy_text` indexes `glyph_info[1]` of a + /// one-entry list. Pinned as a `should_panic` because it is the whole justification for the + /// net — when a bevy release closes the hole this fails, and the net (and this test, and the + /// premise test above) go with it. + #[test] + #[should_panic(expected = "index out of bounds")] + fn without_the_net_the_resize_panics_inside_bevy_text() { + let mut app = text_app(false); + let root = two_run_root(&mut app); + + app.world_mut() + .entity_mut(root) + .despawn_related::(); + app.update(); + resize_frame(&mut app, root); + } + + /// **B383, end to end.** Same world, plus the net: the emptied root is marked for re-shape, so + /// the resize frame re-shapes before it re-lays-out and the pipeline never indexes a dead run. + /// Without [`super::TextReshapePlugin`] this same sequence panics inside `bevy_text` with + /// `index out of bounds` at `pipeline.rs:398` — the client's own crash. + #[test] + fn the_net_reshapes_an_emptied_root_before_the_next_resize() { + let mut app = text_app(true); + let root = two_run_root(&mut app); + + app.world_mut() + .entity_mut(root) + .despawn_related::(); + app.update(); + + assert!( + app.world() + .get::(root) + .expect("the block") + .needs_rerender(), + "the net marks the emptied root for a re-shape" + ); + + // The frame the report is about. It must not panic, and it must leave the block honest. + resize_frame(&mut app, root); + assert_eq!( + app.world() + .get::(root) + .map(|b| b.entities().len()), + Some(1), + "the re-shaped block lists the root alone" + ); + } + + /// The control: a root that keeps a span through a resize is untouched by any of this — the + /// net must not be doing the re-shaping in the ordinary case. + #[test] + fn a_root_that_kept_its_span_is_not_reshaped() { + let mut app = text_app(true); + let root = two_run_root(&mut app); + + app.update(); + assert!( + !app.world() + .get::(root) + .expect("the block") + .needs_rerender(), + "nothing changed, so nothing is asked to re-shape" + ); + resize_frame(&mut app, root); + assert_eq!( + app.world() + .get::(root) + .map(|b| b.entities().len()), + Some(2), + "both runs are still there" + ); + } +} diff --git a/crates/benilla-app/src/ui_chat.rs b/crates/benilla-app/src/ui_chat.rs index 6810055d..ebbaad8f 100644 --- a/crates/benilla-app/src/ui_chat.rs +++ b/crates/benilla-app/src/ui_chat.rs @@ -142,7 +142,21 @@ impl Plugin for UiChatPlugin { // so an alarm or a shutdown countdown lands on the frame it decodes like every other // chat source. .add_systems(Update, broadcast::feed_broadcasts.before(feed::feed_chat)) - .add_systems(Update, feed::feed_chat.before(UiInput)) + // **Never against the boot VM** (B376's second half). `feed_chat` takes the whole + // queue with `mem::take` and fires each line as a real `CHAT_MSG_*` — so a drain + // against a VM with no ChatFrame does not defer the lines, it DESTROYS them, with no + // memo and no error. The window is the one frame `not(ingame_ui_pending)` cannot see + // (`ui_script::ingame_ui_up`): `apply_net_updates` drains `Connected` and the login + // burst behind it in one `try_iter` while the state — and with it 1978's park — is + // still a frame away. Measured: one login in six drained the burst there and ate a + // line of the realm's own welcome. The early return above `mem::take` is what holds + // the queue for the frames after it, and this is what holds it for that one. + .add_systems( + Update, + feed::feed_chat + .before(UiInput) + .run_if(crate::ui_script::ingame_ui_up), + ) // The last-input stamp `[0xcf0bc8]`. Deliberately NOT in-world-gated and // deliberately ahead of the UI pass: the reference stores it on the raw input bus, // before dispatch, so a keystroke the chat box swallows still counts as input. diff --git a/crates/benilla-app/src/ui_chat/settings.rs b/crates/benilla-app/src/ui_chat/settings.rs index 996c200e..76288545 100644 --- a/crates/benilla-app/src/ui_chat/settings.rs +++ b/crates/benilla-app/src/ui_chat/settings.rs @@ -803,7 +803,7 @@ pub(super) fn plugin(app: &mut App) { app.init_resource::() // **The restore is not here.** `UPDATE_CHAT_WINDOWS`/`UPDATE_CHAT_COLOR` have to precede // the session's first chat line and `PLAYER_LOGIN` respectively, and no `run_if` can buy - // that: 1978's `not(ingame_ui_pending)` gate put the restore on exactly the frame the + // that: 1978's UI-is-up gate (`ingame_ui_up`) puts the restore on exactly the frame the // parked VM comes back and `feed_chat` drains the whole queued login burst, with nothing // ordering the two. It is called from `ui_script::lifecycle`'s world-entry load instead — // [`restore_chat_looks`], decision 2119. Only the watcher belongs in `Update`. diff --git a/crates/benilla-app/src/ui_guild/mod.rs b/crates/benilla-app/src/ui_guild/mod.rs index 4ea5f0c5..7b7231a7 100644 --- a/crates/benilla-app/src/ui_guild/mod.rs +++ b/crates/benilla-app/src/ui_guild/mod.rs @@ -224,10 +224,20 @@ impl GuildState { if (self.guild_id, self.rank_index) == (guild_id, rank_index) { return false; } - let left = self.guild_id != guild_id; + // **`0` here is "the descriptor has not told us yet", not "a guild we were in"** (B376). + // The mirror's id starts at 0 and only moves when our own avatar streams, which at a + // login is a whole packet burst AFTER the server has already sent the guild's MOTD + // (`SMSG_GUILD_EVENT 0x02`, vmangos `CharacterHandler.cpp` at the top of the login + // handler). Reading that first 0 → N as a guild *change* threw the MOTD away as the + // property of a guild we had left — and since the wipe happens here, at the top of the + // feed, it landed in the same call that would have fired `GUILD_MOTD` further down, so + // the login line was destroyed before it could ever be taken. A move OUT of a real + // guild (N → 0, or N → M) is the edge that genuinely invalidates the mirror. + let known = self.guild_id != 0; + let moved = self.guild_id != guild_id; self.guild_id = guild_id; self.rank_index = rank_index; - if left { + if moved && known { // A different guild (or none): everything the old roster said is about a guild we are // no longer in. The identity cache survives — it is keyed by id and still true. self.motd.clear(); @@ -235,6 +245,8 @@ impl GuildState { self.rank_rights.clear(); self.members.clear(); self.selection = 0; + } + if moved { self.note_roster_update(RosterUpdate::Applied); } self.dirty = true; @@ -769,7 +781,13 @@ impl Plugin for UiGuildPlugin { ( feed::feed_guild.before(UiInput), feed::drain_guild.after(UiInput), - ), + ) + // **Never against the boot VM** (1348/1978, and B376's half of it): the feed + // takes the MOTD and the roster edges through a [`crate::ui_script::VmMemo`] + // that the entry load does not reset, so an edge spent on a VM with no frames + // is spent for the whole login — the "Guild Message of the Day:" line fired + // into nothing and never fired again. `ingame_ui_up`'s doc has the window. + .run_if(crate::ui_script::ingame_ui_up), ); } } @@ -1087,6 +1105,39 @@ mod tests { assert!(!guild.mirror_self(0, 0), "no edge when nothing moved"); } + /// **B376, at the line that caused it.** At a login the server sends `SMSG_GUILD_EVENT 0x02` + /// (the MOTD) a whole packet burst before our own avatar's descriptor carries + /// `PLAYER_GUILDID`, so the mirror's first sight of our guild id is `0 -> N` — and that is us + /// being TOLD which guild we are in, not us moving between two. Reading it as a move wiped the + /// MOTD that had already arrived, in the same `feed_guild` call that would have fired + /// `GUILD_MOTD` a few lines further down: the login line was destroyed before the edge could + /// be taken, and the text only came back when the guild pane asked for a roster — the report. + #[test] + fn learning_our_own_guild_id_is_not_leaving_a_guild() { + let mut guild = GuildState::default(); + guild.apply_event(&event( + guild_event::MOTD, + &["Raid Wednesday at eight."], + None, + )); + assert!(!guild.in_guild(), "the descriptor has not said yet"); + + assert!( + guild.mirror_self(1, 3), + "the descriptor finally names the guild" + ); + assert_eq!( + guild.motd, "Raid Wednesday at eight.", + "the MOTD is about the guild we have just been told we are in" + ); + assert_eq!( + guild.roster_event, + Some(RosterUpdate::Stale), + "the pane still learns the snapshot moved — the MOTD packet's own stale signal, \ + which outranks the `Applied` this edge notes" + ); + } + /// Our own rank's rights word is read out of the roster's array by our own descriptor rank, /// and answers 0 (no permissions) rather than guessing while no roster has arrived. #[test] diff --git a/crates/benilla-app/src/ui_script/lifecycle.rs b/crates/benilla-app/src/ui_script/lifecycle.rs index 2289ecd2..52a102c0 100644 --- a/crates/benilla-app/src/ui_script/lifecycle.rs +++ b/crates/benilla-app/src/ui_script/lifecycle.rs @@ -239,28 +239,39 @@ fn unpark_boot_vm(world: &mut World) { } } -/// **Is the in-game UI still owed for this world entry?** True from `OnEnter(InWorld)` until -/// [`run_pending_entry_load`] has built the frame tree — the window in which the VM is still the -/// *boot* VM: strings, emote tokens and fonts, and not one frame. +/// **Is the in-game UI up on the VM that is in the world?** The run condition every in-world feed +/// wants — nothing is pushed and nothing is fired until there is an interface to receive it. /// -/// The run condition on [`crate::ui_unit::UnitFeed`], and the reason it exists (1348): the feeds -/// fire the login **one-shots** — `PLAYER_ENTERING_WORLD`, the first `PLAYER_XP_UPDATE`, the first -/// `UPDATE_EXHAUSTION` — and every one of them is latched by a [`super::VmMemo`] keyed on the VM's -/// *session*, which the entry load does not change (it loads files ONTO this VM). So an event -/// fired in this window is delivered to nobody and then never fires again for the whole session: -/// the frames built moments later do their first paint with no first paint. That is not a -/// hypothetical ordering — it is a RACE against the wire, which is why it took some logins and not -/// others, and why the symptom moved between characters. The self descriptor arriving inside the -/// deferral window is all it takes. +/// The reason it exists (1348): the feeds fire the login **one-shots** — +/// `PLAYER_ENTERING_WORLD`, the first `PLAYER_XP_UPDATE`, the first `UPDATE_EXHAUSTION` — and +/// every one of them is latched by a [`super::VmMemo`] keyed on the VM's *session*, which the +/// entry load does not change (it loads files ONTO this VM). So an event fired against the boot +/// VM is delivered to nobody and then never fires again for the whole session: the frames built +/// moments later do their first paint with no first paint. That is not a hypothetical ordering — +/// it is a RACE against the wire, which is why it took some logins and not others, and why the +/// symptom moved between characters. The self descriptor arriving inside the deferral window is +/// all it takes. /// /// The reference has no such window: `UI_Init 0x48fbf0` loads all of FrameXML and *then* fires the /// world-enter cascade (`PLAYER_LOGIN` at `0x49094b`, `PLAYER_ENTERING_WORLD` at `0x490965`) from -/// inside itself, so a UI-less client never sees a unit event at all. Gating the feed here is that -/// same ordering, expressed against our deferred load: nothing is pushed and nothing is fired until -/// there is a UI to receive it, and the very next frame's feed — running against a fresh, unlatched -/// world — delivers the full set in order. -pub(crate) fn ingame_ui_pending(pending: Option>) -> bool { - pending.is_some() +/// inside itself, so a UI-less client never sees a unit event at all. This is that same ordering, +/// expressed against our deferred load: the first frame that answers true is a fresh, unlatched +/// world, and the feed delivers the full set in order. +/// +/// **TWO terms, because `not(ingame_ui_pending)` is only half of it** (B376). The latch is armed +/// at `OnEnter(InWorld)`, and that edge trails the wire by a frame: `apply_net_updates` drains +/// `Connected` and the whole login burst behind it in one `try_iter`, `enter_on_connected` sets +/// `NextState` from that same drain, and the transition — with it the park and the latch — does +/// not run until the NEXT frame's `StateTransition`. So for exactly one frame the client has +/// in-world wire state and a live *boot* VM, and 1978's park does not cover it. `InWorld` is the +/// term that does: the state flips on the same edge that arms the latch, so the pair is closed at +/// both ends. B376's guild MOTD was fired into that one frame (observed live, 1 login in 3); +/// the unit feed and the cinematic's UI edge sat on the same hole, unobserved. +pub(crate) fn ingame_ui_up( + pending: Option>, + state: Option>>, +) -> bool { + pending.is_none() && state.is_some_and(|s| *s.get() == crate::char_select::ClientState::InWorld) } /// `PreUpdate` (chained after [`run_pending_reload`] — same exclusive slot, and a reload must diff --git a/crates/benilla-app/src/ui_script/mod.rs b/crates/benilla-app/src/ui_script/mod.rs index fea1c03a..dd5a98cf 100644 --- a/crates/benilla-app/src/ui_script/mod.rs +++ b/crates/benilla-app/src/ui_script/mod.rs @@ -565,7 +565,7 @@ fn arbitrate_pointer_over_ui( /// lives there; this module keeps the per-frame bridge. See its header. mod lifecycle; pub(crate) use lifecycle::{ - end_ui_session, ingame_ui_pending, run_pending_reload, setup_script, AddOnIdentity, + end_ui_session, ingame_ui_up, run_pending_reload, setup_script, AddOnIdentity, PendingEntryUiLoad, ReloadUiPending, }; // Consumed only from other modules' test code (the emote-table checks, the harness's UI-init diff --git a/crates/benilla-app/src/ui_script/tooltip_compare_tests.rs b/crates/benilla-app/src/ui_script/tooltip_compare_tests.rs index 5b4055b4..a3291d57 100644 --- a/crates/benilla-app/src/ui_script/tooltip_compare_tests.rs +++ b/crates/benilla-app/src/ui_script/tooltip_compare_tests.rs @@ -299,21 +299,37 @@ fn doll_hover_renders_the_live_instance() { /// off plate 1 at (0, 0). The engine supplies only the two bindings and the compare render; the /// geometry, the second fill and both `Show`s are the reference's Lua. Two worn rings against a /// ring on the shelf, so BOTH plates answer — the case that pins the offset walk. +/// +/// And it pins the CONTENT the plate carries, because that is where we were wrong (2216): the +/// compare call sites pass p4 = 0, so a plate is the worn item's ordinary tooltip with one gray +/// line on top — an EPIC ring's name reads purple, not white, and its flavour text is not cut. #[test] fn shipped_merchant_row_raises_the_compare_plates() { let _data = benilla_formats::wow_data_or_skip!(); let mut s = harness(); // Two worn rings, a third on the shelf: offset 1 and 2 both find a candidate. let mut inv: InventorySlots = Default::default(); - for (slot, id, name) in [(11usize, 7000u32, "Old Loop"), (12, 7001, "Older Loop")] { + // The worn ring under plate 1 is EPIC and carries flavour text — the two things the old + // conflated compare flag took away. + for (slot, id, name, quality) in [ + (11usize, 7000u32, "Old Loop", 4u32), + (12, 7001, "Older Loop", 1), + ] { inv[slot] = Some(InvSlotView { item_id: id, count: 1, - quality: 1, + quality: quality as i32, name: Some(name.into()), ..Default::default() }); - s.set_item_template(id, armor_template(name, 11)); + s.set_item_template( + id, + ItemTemplateView { + quality, + description: "Round.".into(), + ..armor_template(name, 11) + }, + ); } s.set_inventory_slots(inv); s.set_item_template(8000, armor_template("Shiny Loop", 11)); @@ -355,6 +371,22 @@ fn shipped_merchant_row_raises_the_compare_plates() { ok, "the vendor row raises both plates at the stock geometry" ); + // p4 = 0 over the stock file: the plate is the FULL tooltip, in the worn item's own colours. + let ok: bool = s + .eval( + "local r, g, b = ShoppingTooltip1TextLeft2:GetTextColor() \ + local flavour = nil \ + for i = 1, ShoppingTooltip1:NumLines() do \ + if getglobal(\"ShoppingTooltip1TextLeft\"..i):GetText() == \"\\\"Round.\\\"\" then flavour = i end \ + end \ + return flavour ~= nil and math.abs(r - 0.639) < 0.01 \ + and math.abs(g - 0.208) < 0.01 and math.abs(b - 0.933) < 0.01", + ) + .unwrap(); + assert!( + ok, + "the epic ring's plate reads purple and keeps its flavour text — no compact cut" + ); // The plate wears the template's own small-font ladder (10px), the main tooltip its header face. let ok: bool = s .eval( diff --git a/crates/benilla-app/src/ui_script/world_entry_tests.rs b/crates/benilla-app/src/ui_script/world_entry_tests.rs index adee4c3f..f792082a 100644 --- a/crates/benilla-app/src/ui_script/world_entry_tests.rs +++ b/crates/benilla-app/src/ui_script/world_entry_tests.rs @@ -759,7 +759,20 @@ fn the_login_one_shots_wait_for_the_in_game_ui() { .expect("probe global") }; - // …but the in-game UI is still owed. Three frames inside the deferral window. + // **The frame the latch cannot see** (B376): the descriptor above came off the same drain as + // `Connected`, so the wire is in-world while the state still says glue and no load is armed + // yet — `OnEnter(InWorld)` runs next frame. A feed gated only on the latch runs here. + app.insert_resource(State::new(crate::char_select::ClientState::CharSelect)); + app.update(); + assert_eq!( + seen(&app), + 0, + "the drain's own frame: in-world wire, a boot VM, and no latch yet" + ); + + // …the transition ran, and the in-game UI is still owed. Three frames inside the deferral + // window. + app.insert_resource(State::new(crate::char_select::ClientState::InWorld)); app.insert_resource(super::PendingEntryUiLoad); for frame in 1..=3 { app.update(); @@ -1227,3 +1240,58 @@ fn a_clean_world_entry_raises_only_the_warnings_we_have_named() { "a stock world entry warned about something new — fix it or name it here: {unexpected:#?}" ); } + +// ─────────────────── The predicate every in-world feed runs on (B376) ─────────────────── + +/// **`not(ingame_ui_pending)` was only half the gate**, and the missing half is a whole frame +/// wide. +/// +/// [`super::lifecycle::PendingEntryUiLoad`] is armed at `OnEnter(InWorld)`, and that edge trails +/// the wire by one frame: `apply_net_updates` drains `Connected` and the login burst behind it in +/// a single `try_iter`, `enter_on_connected` sets `NextState` out of that same drain, and the +/// transition — with it the park (1978) and this latch — does not run until the next frame's +/// `StateTransition`. So there is exactly one frame holding in-world wire state and a live *boot* +/// VM, and a feed gated only on the latch runs straight through it: B376's `GUILD_MOTD` fired +/// into a VM with no `ChatFrame1`, spent its `VmMemo` edge, and the login line never printed. +/// +/// The `InWorld` term is what closes it — the state flips on the same edge that arms the latch, +/// so the pair is shut at both ends. +#[test] +fn the_ui_is_not_up_in_the_frame_between_the_wire_and_the_state() { + let mut world = World::new(); + + // **THE FRAME.** A live boot VM, no load owed — and the state still says glue, because + // `Connected` has only just been drained (so the guild/unit burst is already in ECS state) + // and the transition it queued runs next frame. The latch alone reads this as "the UI is up". + world.insert_resource(State::new(crate::char_select::ClientState::CharSelect)); + assert!( + !run_ingame_ui_up(&mut world), + "the wire is in-world a frame before the state is — the boot VM must stay out of reach" + ); + + // The transition ran: `OnEnter` parked the VM and armed the latch. + world.insert_resource(State::new(crate::char_select::ClientState::InWorld)); + world.insert_resource(super::PendingEntryUiLoad); + assert!( + !run_ingame_ui_up(&mut world), + "the deferral window — 1978's parked VM, and nothing to receive an event" + ); + + // The deferred load ran: the frame tree exists. + world.remove_resource::(); + assert!( + run_ingame_ui_up(&mut world), + "in the world with the in-game UI up — the first frame a feed may push" + ); + + // Leaving drops it again, before `end_ui_session`'s fresh boot VM can be fed anything. + world.insert_resource(State::new(crate::char_select::ClientState::CharSelect)); + assert!(!run_ingame_ui_up(&mut world), "the world is gone with it"); +} + +fn run_ingame_ui_up(world: &mut World) -> bool { + use bevy::ecs::system::RunSystemOnce; + world + .run_system_once(super::lifecycle::ingame_ui_up) + .expect("the condition runs") +} diff --git a/crates/benilla-app/src/ui_unit.rs b/crates/benilla-app/src/ui_unit.rs index 6547b661..76fac8cb 100644 --- a/crates/benilla-app/src/ui_unit.rs +++ b/crates/benilla-app/src/ui_unit.rs @@ -176,10 +176,8 @@ impl Plugin for UiUnitPlugin { // …and never before the in-game UI exists (1348). The whole SET, not just // `feed_units`: every feed in it either fires a login one-shot or latches a // per-VM memo, and both are lost forever against the boot VM. The window and - // the reference's own ordering: `ui_script::ingame_ui_pending`. - .run_if(bevy::ecs::schedule::common_conditions::not( - crate::ui_script::ingame_ui_pending, - )), + // the reference's own ordering: `ui_script::ingame_ui_up`. + .run_if(crate::ui_script::ingame_ui_up), ) .init_resource::() // [`feed_units`] shows catalog messages (the rest-state pair, the PvP toggle) through diff --git a/crates/benilla-ui/src/script/tests/tooltip_item/bindings.rs b/crates/benilla-ui/src/script/tests/tooltip_item/bindings.rs index 0ed2edc2..ee16e94c 100644 --- a/crates/benilla-ui/src/script/tests/tooltip_item/bindings.rs +++ b/crates/benilla-ui/src/script/tests/tooltip_item/bindings.rs @@ -7,9 +7,11 @@ use super::script; use crate::script::*; /// The compare SHAPE, driven the only way 1.12.1 drives it: the vendor row's -/// `SetMerchantCompareItem` (`MerchantFrame.xml:63-80`). The armed render carries the byte law's -/// compare mode — the gray CURRENTLY_EQUIPPED header, the name WHITE instead of quality-colored, -/// and the compact cut at `0x52e14c` (the description never prints). +/// `SetMerchantCompareItem` (`MerchantFrame.xml:63-80`). The armed render is the equipped item's +/// ORDINARY tooltip plus ONE line, first — the gray CURRENTLY_EQUIPPED header (p5). Both compare +/// call sites pass p4 (compact) ZERO, so the two things that are easy to get wrong here are +/// pinned as negatives: the name keeps its QUALITY color (this worn ring is epic, so the assert +/// bites), and nothing is cut at `0x52e14c` — the description still prints (2216). /// /// **And the negative control that keeps this engine out of it** (2210): a bag hover seats /// nothing, with shift or without, because the reference has no hover compare at all — @@ -24,7 +26,9 @@ fn merchant_compare_renders_the_compare_shape_and_no_hover_seats_a_plate() { durability: None, item_id: 7000, name: Some("Old Loop".into()), - quality: 1, + // EPIC — a white name and a quality name are the same pixel at quality 1, which is how + // the conflated flag survived a green test for as long as it did. + quality: 4, ..Default::default() }); inv[12] = Some(InvSlotView { @@ -39,7 +43,7 @@ fn merchant_compare_renders_the_compare_shape_and_no_hover_seats_a_plate() { 7000, ItemTemplateView { name: "Old Loop".into(), - quality: 1, + quality: 4, inventory_type: 11, description: "Round.".into(), ..Default::default() @@ -158,16 +162,20 @@ fn merchant_compare_renders_the_compare_shape_and_no_hover_seats_a_plate() { assert(ShoppingTooltip1:IsShown()) assert(ShoppingTooltip1TextLeft1:GetText() == "[CURRENTLY_EQUIPPED]") assert(ShoppingTooltip1TextLeft2:GetText() == "Old Loop") - -- The compact cut at 0x52e14c: the description never prints on a compare. + -- p4 = 0: there is NO compact cut here. The description prints, exactly as it does on a + -- plain SetInventoryItem of the same ring. + local described = nil for i = 1, ShoppingTooltip1:NumLines() do - assert(getglobal("ShoppingTooltip1TextLeft" .. i):GetText() ~= "\"Round.\"", - "compact cut dropped the description") + if getglobal("ShoppingTooltip1TextLeft" .. i):GetText() == "\"Round.\"" then + described = i + end end + assert(described, "the compare tooltip is the FULL tooltip — the description prints") assert(table.getn(compare_calls) == 0, "and still no dead event") "#, ) .unwrap(); - // The compare colors: gray header, WHITE name (never the quality color) — the byte law. + // The compare colors: gray header, and the name in its own QUALITY color — the byte law. s.resolve(); let quads = s.extract(); let color_of = |txt: &str| { @@ -189,7 +197,12 @@ fn merchant_compare_renders_the_compare_shape_and_no_hover_seats_a_plate() { "CURRENTLY_EQUIPPED is gray, got {gray:?}" ); let name = color_of("Old Loop"); - assert_eq!(name, [1.0, 1.0, 1.0, 1.0], "compare name is WHITE"); + assert!( + (name[0] - 0.639).abs() < 0.01 + && (name[1] - 0.208).abs() < 0.01 + && (name[2] - 0.933).abs() < 0.01, + "the compare name wears the item's own quality color (epic purple), got {name:?}" + ); assert!(s.take_errors().is_empty()); } diff --git a/crates/benilla-ui/src/script/tooltip_item/mod.rs b/crates/benilla-ui/src/script/tooltip_item/mod.rs index 0e75aca3..c08298a1 100644 --- a/crates/benilla-ui/src/script/tooltip_item/mod.rs +++ b/crates/benilla-ui/src/script/tooltip_item/mod.rs @@ -14,11 +14,14 @@ //! Residual INTERIMs cited inline: the dual-wield/off-hand proficiency exception //! (`0x5eab70`), the type cell's override red, the set-owned count source. //! -//! **Compare mode** (0274 P4, re-based by 2202, scoped back to the reference by 2210): -//! `SetInventoryItem` on an ARMED shopping tooltip renders the byte law's compare shape — gray -//! "Currently Equipped" first (`[arg+0x18]≠0`), the NAME white instead of quality-colored, and -//! the compact early-return at `0x52e14c` (`[arg+0x14]≠0`): nothing after the charges/cooldown -//! block. **Nothing in this engine seats a shopping plate**: the two callers that arm this mode +//! **Compare mode** (0274 P4, re-based by 2202, scoped back to the reference by 2210, and +//! corrected to the census by 2216): `SetInventoryItem` on an ARMED shopping tooltip renders the +//! equipped item's ORDINARY tooltip plus ONE extra line, first — the gray "Currently Equipped" +//! (`[arg+0x18]≠0`). That is the entire difference. The builder's other flag, p4 `[arg+0x14]`, +//! *is* a compact mode (white name, stat body jumped, early-return at `0x52e14c`) — but the two +//! compare call sites pass it **zero**, so a shopping plate shows a quality-colored name and the +//! full body, and a client that abbreviates here is wrong (wow-re `merchant-compare-item-law.md` +//! §6). **Nothing in this engine seats a shopping plate**: the two callers that arm this mode //! are [`compare_against_worn`]'s own bindings — `SetMerchantCompareItem` and //! `SetAuctionCompareItem` — and the FrameXML that calls them (`MerchantFrame.xml:63-80`, //! `AuctionFrame`) owns the plates' geometry and lifetime, exactly as in 1.12.1. There is no @@ -57,7 +60,7 @@ use render::render_view; /// slots (`0x809200[InventoryType]`), skipping EMPTY slots without decrementing and slots whose /// worn item's CLASS differs, until `offset` occupied matches have been passed; then fill the /// ordinary EQUIPPED-item tooltip with the compare header on — the FULL body (`p4 = 0`) plus one -/// gray `CURRENTLY_EQUIPPED` line (`p5 = 1`), by arming the `compare_armed` latch and running +/// gray `CURRENTLY_EQUIPPED` line (`p5 = 1`), by arming the `equipped_header_armed` latch and running /// `SetInventoryItem`, which is what the reference does too: /// `0x536080` calls the ordinary item builder `0x52b650` with the header flag set. The NUMBER 1 on /// success; nil on a miss, which does NOT clear the tooltip (the reference's nil exit touches @@ -100,7 +103,7 @@ fn compare_against_worn( { let mut model = lua.app_data_mut::().expect("model app_data"); if let Ok(t) = super::tooltip::tip_mut(&mut model, h) { - t.compare_armed = true; + t.equipped_header_armed = true; } } this.get::("SetInventoryItem")? @@ -446,10 +449,10 @@ pub(super) fn install_methods(lua: &Lua, m: &Table) -> mlua::Result<()> { "SetInventoryItem", lua.create_function(|lua, (this, unit, slot): (Table, String, usize)| { let h = frame_handle_of(lua, &this)?; - let (item_id, name, quality, inst, compare) = { + let (item_id, name, quality, inst, currently_equipped) = { let mut model = lua.app_data_mut::().expect("model app_data"); let armed = match super::tooltip::tip_mut(&mut model, h) { - Ok(t) => std::mem::take(&mut t.compare_armed), + Ok(t) => std::mem::take(&mut t.equipped_header_armed), Err(_) => false, }; let view = model @@ -508,23 +511,19 @@ pub(super) fn install_methods(lua: &Lua, m: &Table) -> mlua::Result<()> { } fire_cleared(lua, h); match view_of(lua, item_id) { - Some(v) => render_view(lua, &this, &v, compare, Some(&inst))?, + Some(v) => render_view(lua, &this, &v, currently_equipped, Some(&inst))?, None => { // Template in flight — the slot view's own name holds the plate (the same // 0138 posture as SetBagItem's miss path). // The compare header is a key like every other sentence (2045); this // fallback path shows it for exactly the reason the full render does. - if compare { + if currently_equipped { if let Some(t) = crate::strings::global(lua, "CURRENTLY_EQUIPPED") { append_line(lua, &this, (t, GRAY), None, false)?; } } if let Some(name) = name { - let color = if compare { - WHITE - } else { - quality_color(quality.max(0) as u32) - }; + let color = quality_color(quality.max(0) as u32); append_line(lua, &this, (name, color), None, false)?; } } diff --git a/crates/benilla-ui/src/script/tooltip_item/render.rs b/crates/benilla-ui/src/script/tooltip_item/render.rs index d74feed5..62e29a6d 100644 --- a/crates/benilla-ui/src/script/tooltip_item/render.rs +++ b/crates/benilla-ui/src/script/tooltip_item/render.rs @@ -126,7 +126,7 @@ pub(super) fn render_view( lua: &Lua, this: &Table, v: &ItemTemplateView, - compare: bool, + currently_equipped: bool, // `None` = a template/link source (the ref's no-object path). inst: Option<&ItemInstance>, ) -> mlua::Result<()> { @@ -176,20 +176,23 @@ pub(super) fn render_view( } }; - // Compare mode (the shopping tooltips): the gray CURRENTLY_EQUIPPED header (`[arg+0x18]≠0`) - // and a WHITE name instead of the quality color (`[arg+0x14]≠0`) — both byte-verified. - if compare { + // The gray CURRENTLY_EQUIPPED header (`[arg+0x18]≠0`, color ptr `0xc0d3c4`) — and it is the + // ONLY thing a shopping-tooltip fill adds. **Against a plain `SetInventoryItem` of the same + // equipped item, one extra line, first, is the whole difference**: the two callers that set + // the header (`SetMerchantCompareItem`'s arg vector at `0x5362d4`, `SetAuctionCompareItem`'s + // at `0x53603e`) pass **p4 = 0**, so compact/compare mode is OFF — the NAME keeps its quality + // color, the stat body is not jumped, and nothing is cut at `0x52e14c`. wow-re + // `merchant-compare-item-law.md` §6 says it in as many words ("a downstream client that + // renders a 'compare mode' abbreviated tooltip here is wrong"), and `tooltip-content-law.md` + // §1's per-call-site census over all 31 sites of `0x52b650` is what settles it: those two are + // the only sites in the image passing a literal non-zero p5, and both pass p4 zero. + if currently_equipped { keyed("CURRENTLY_EQUIPPED", &[], GRAY, false)?; } - let name_color = if compare { - WHITE - } else { - quality_color(v.quality) - }; let name = inst .and_then(|i| i.name.clone()) .unwrap_or_else(|| v.name.clone()); - add((name, name_color))?; + add((name, quality_color(v.quality)))?; // Line 3 — the petition block, ABOVE the green line and below the name: "Guild Name: X" then // "Guild Master: Y" for a charter, "Petition: X" / "Created by Y" for a plain petition. The // keys are picked by the record's own charter bit, the same bit `GetPetitionInfo`'s first @@ -726,7 +729,7 @@ pub(super) fn render_view( if let Some(charges) = charges_phrase(v.charges.max(0) as u32, &get) { add((charges, WHITE))?; } - // The item-SET block (§22, ABOVE the compact cut), byte-read at the builder's + // The item-SET block (§22, above the binary's p4 cut at `0x52e14c`), byte-read at the builder's // `0x52d8a0..0x52e0f5`: a blank gold line ([`SET_SPACER`]), the gold "name (owned/total)" // header, the set-level skill line (white, red when short), the member ladder (" name" — // pale-cream `0xc0d368` when equipped, gray otherwise; a member whose template is still in @@ -790,11 +793,6 @@ pub(super) fn render_view( } } } - // The compact/compare early-return (`0x52e14c`, `[arg+0x14]≠0`): everything below — - // description, made-by, openable/readable, money — is skipped on a shopping tooltip. - if compare { - return Ok(()); - } // The quoted flavor text — gold, wrapped, literal quotes (all three byte-verified). if !v.description.is_empty() { addw((format!("\"{}\"", v.description), GOLD))?; diff --git a/crates/benilla-ui/src/widget/kinds/mod.rs b/crates/benilla-ui/src/widget/kinds/mod.rs index 4ef5c309..823e3837 100644 --- a/crates/benilla-ui/src/widget/kinds/mod.rs +++ b/crates/benilla-ui/src/widget/kinds/mod.rs @@ -379,14 +379,16 @@ pub struct TooltipState { /// corpse) — the fade-on-loss gate (`world_tooltip_fade`); a window hover never fades. /// Dropped with the content. pub world_owned: bool, - /// ARMED for a shopping-compare render: the next `SetInventoryItem` on this frame renders in - /// the byte law's compare mode (`[arg+0x14]≠0` compact + `[arg+0x18]≠0` "Currently Equipped" - /// — wow-re tooltip-content-law.md). Set by the engine right before it fills a shopping - /// plate, and consumed by that render. Survives `SetOwner`'s content clear, because the - /// reference's own compare caller (`MerchantFrame.xml:67-72`) SetOwners *between* the arm and - /// the render; how the real engine plumbs the flag to `0x52b650` is unrecorded, so - /// this seam is the INTERIM model of it. - pub compare_armed: bool, + /// ARMED for a shopping-compare render: the next `SetInventoryItem` on this frame prepends the + /// gray "Currently Equipped" line — the builder's p5 `[arg+0x18]≠0` and NOTHING else. It is + /// deliberately not the neighbouring p4 `[arg+0x14]` (compact: white name, stat body jumped, + /// cut at `0x52e14c`), which both compare call sites pass as zero; conflating the two is what + /// made our shopping plates paint an epic's name white and drop its description (2216). Set by + /// the engine right before it fills a shopping plate, and consumed by that render. Survives + /// `SetOwner`'s content clear, because the reference's own compare caller + /// (`MerchantFrame.xml:67-72`) SetOwners *between* the arm and the render; how the real engine + /// plumbs the flag to `0x52b650` is unrecorded, so this seam is the INTERIM model of it. + pub equipped_header_armed: bool, /// `SetPadding(w)` — extra width beyond the measured content (ref ItemRefTooltip's /// OnLoad `SetPadding(16)`: room for the corner close button). 0 for ordinary tooltips. pub padding: f32,