Four defects out of his log, and the 65 warnings that are not ours
His 19-minute session put 8 errors on screen from two causes, and 73 WARN lines of which 8 are ours. The whole triage is written down in the play-test memory, because most of the log is the addons' own and nothing should re-chase it: five files their `.toc`s name and do not ship, a QuickHeal template that is never defined, an ATSW global string that only exists in its deDE branch, inspect frames from a load-on-demand addon with no `## Dependencies` line, `<Binding>` elements the frame loader is *supposed* to log, and a world-map lag line that reported its own recovery 5.2 s later. Four of the eight are fixed here. `GameTooltip:SetSpellByID` was nil, and it is not a keypress: the traceback is pfUI's `ButtonEnter` into `SuperCleveRoidMacros/Core.lua:4217`, so it raised on every action-bar hover of a macro that resolves to a spell — 7 times, each leaving the button with no tooltip at all. `compat/classic_api/tooltip.rs` had declined the whole `Set*ByID` family pending "an argument-shape check against the stock verb each aliases". Done, and the check is why this is four lines: there is no aliasing to do, because `tooltip_spell::set_spell_by_id` is the body all four of 1.12's spell verbs already funnel through, and it takes a bare id — the book slot or list row is what *they* have to convert first. `show_rank` is the action's rather than the spellbook's, because the book draws "Rank N" itself as the button's `subSpellName` and an id-named caller has no such label. The caller's own comment says it reaches this only for a spell in neither spellbook, so the cold-view path is the common one here; that needs no special case, since the app's feed builds a view for any id in the catalog and an action button re-asks its tooltip every `TOOLTIP_UPDATE_TIME` while hovered. `resolve_spell_fx` despawned an entity the world no longer had. `FxInstance`'s root is a *stored handle*, and `attach_spell_fx` is the system that notices one lost with the unit's whole visual and re-arms or drops the instance — so between that loss and that pass, the same-slot replace, the spell-id reap and the self-termination clock all hold handles to entities that are gone. `try_despawn` at those three, as `lifecycle.rs` already does; `tend_world_plants` keeps its plain `despawn` because its entity comes from a query, not from a record. The first paste of every Wayland session could never have worked. `smithay-clipboard` sets `latest_seat` only on a `wl_keyboard` Key/Modifiers event or a pointer press (`state.rs:522`, `:386`) — `Enter` sets `has_focus` and nothing else — and refuses to load a selection without it. `HostClipboard` opened its backend on first *use*, so the backend that read the clipboard was created by the very Ctrl+V asking for it and its keyboard object was younger than that chord's own key events. It is now opened as soon as the window can give the `wl_display`. Costs +2 on the `Update` ambiguity ceiling, both pairs against the schedule's two exclusive `&mut World` systems, which pair with everything they have no declared order against. And the one `changed_windows` ERROR a login prints had no remedy behind it: no client can warp a free pointer on Wayland, so the loading screen's cursor hand-back now asks only where the answer can be yes — off Wayland, or with the pointer locked, which is winit's own rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
Claude Opus 5
@@ -671,7 +671,14 @@ fn replace_same_slot(
|
||||
if let Some(root) = i.root {
|
||||
// Through the shared teardown `0x6203e0`, so its particles finish.
|
||||
drain_instance_emitters(emitters, root);
|
||||
commands.entity(root).despawn();
|
||||
// `try_` because a recorded root is a *stored handle*, not a queried entity, and the
|
||||
// instance list is reconciled lazily: `attach_spell_fx` is the system that notices a
|
||||
// root lost with the unit's whole visual (a display swap, a despawn) and re-arms or
|
||||
// drops the instance, so between that loss and that pass every site here holds a
|
||||
// handle to an entity the world no longer has. Bevy's plain `despawn` reports one as
|
||||
// a command error, which is how this was found in grom's log, and there is nothing to
|
||||
// report: the entity going is exactly what this wanted.
|
||||
commands.entity(root).try_despawn();
|
||||
}
|
||||
false
|
||||
});
|
||||
@@ -700,7 +707,8 @@ fn reap_matching(
|
||||
if let Some(root) = i.root {
|
||||
// Destroyed at once, through `0x6203e0` like any other, so its particles drain.
|
||||
drain_instance_emitters(emitters, root);
|
||||
commands.entity(root).despawn();
|
||||
// `try_`, for the reason `replace_same_slot` gives.
|
||||
commands.entity(root).try_despawn();
|
||||
}
|
||||
return false;
|
||||
};
|
||||
@@ -751,7 +759,11 @@ pub(super) fn attach_spell_fx(
|
||||
if let Some(root) = inst.root {
|
||||
// The `0x6203e0` teardown: the particles outlive the flash.
|
||||
drain_instance_emitters(&mut emitters, root);
|
||||
commands.entity(root).despawn();
|
||||
// `try_`, for the reason `replace_same_slot` gives — and reachable from
|
||||
// here in particular because this clock is read *before* the lost-root
|
||||
// check below, so a flash whose root went with the visual and then
|
||||
// expires arrives with a dead handle.
|
||||
commands.entity(root).try_despawn();
|
||||
}
|
||||
if benilla_assets::trace::enabled() {
|
||||
benilla_assets::trace::line(
|
||||
|
||||
@@ -889,7 +889,15 @@ pub(crate) mod schedule_tests {
|
||||
/// camera belongs. It could never be registered here — `&mut Transform` on a camera is
|
||||
/// shared with everything that moves anything — which is what kept the planar-water lane
|
||||
/// behind an env var instead of a setting.
|
||||
const UPDATE_ACTIONABLE_CEILING: usize = 4_952;
|
||||
/// +2 for `textinput::open_early`, which resolves the OS pasteboard once the window can give
|
||||
/// the Wayland display pointer. Both pairs are the shape `log_new_corpses` describes two notes
|
||||
/// up: they are against the net dispatch and the collider finisher, the schedule's two
|
||||
/// exclusive `&mut World` systems, which pair with everything they have no declared order
|
||||
/// against and which bevy cannot classify because it cannot see their access. There is nothing
|
||||
/// to declare an order *about*: it reads one window's `RawHandleWrapper` and writes one
|
||||
/// `NonSend` handle neither of them can name, and its `run_if` is the handle's own
|
||||
/// "unresolved" bit, so it runs at most once in a process and then never again.
|
||||
const UPDATE_ACTIONABLE_CEILING: usize = 4_954;
|
||||
const UPDATE_ACTIONABLE_SLACK: usize = 40;
|
||||
|
||||
fn ratchet(what: &str, n: usize, ceiling: usize, slack: usize) {
|
||||
|
||||
@@ -27,7 +27,9 @@ use bevy::input::mouse::{
|
||||
};
|
||||
use bevy::math::DVec2;
|
||||
use bevy::prelude::*;
|
||||
use bevy::window::{CursorLeft, CursorMoved, PrimaryWindow};
|
||||
use bevy::window::{
|
||||
CursorGrabMode, CursorLeft, CursorMoved, CursorOptions, PrimaryWindow, RawHandleWrapper,
|
||||
};
|
||||
|
||||
use super::LoadingScreen;
|
||||
|
||||
@@ -121,6 +123,22 @@ impl Buttons<'_> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Can a `Some` written to the window's cursor position actually land?
|
||||
///
|
||||
/// On Wayland it cannot unless the pointer is **locked**: there is no protocol for a client to
|
||||
/// warp a free pointer, so winit's backend refuses outright — `cursor position can be set only
|
||||
/// for locked cursor`, which is the one `bevy_winit::system::changed_windows` ERROR in grom's log,
|
||||
/// once per login, from the hand-back below. Nothing is lost by not asking: the write could never
|
||||
/// have moved the pointer, and the position it would have restored arrives anyway with his first
|
||||
/// mouse move. What is gained is that the one ERROR a login prints is no longer one with no
|
||||
/// remedy.
|
||||
///
|
||||
/// Taken as two plain values rather than as the window, so the rule itself is testable without a
|
||||
/// real surface.
|
||||
fn a_warp_can_land(wayland: bool, grab: CursorGrabMode) -> bool {
|
||||
!wayland || grab == CursorGrabMode::Locked
|
||||
}
|
||||
|
||||
/// `PreUpdate`, in [`CoverInput`]: take the whole input plane while the cover is up. The cover
|
||||
/// rises in `Update`, so its raise frame still takes input; that frame is also the first to draw
|
||||
/// it, so input stops exactly on the frames after the cover was shown.
|
||||
@@ -128,6 +146,9 @@ fn swallow_input_under_the_cover(
|
||||
screen: Res<LoadingScreen>,
|
||||
mut pointer: ResMut<CoveredPointer>,
|
||||
mut window: Query<&mut Window, With<PrimaryWindow>>,
|
||||
// Disjoint from the `&mut Window` above, so the two queries coexist: what the platform will
|
||||
// accept, for [`a_warp_can_land`].
|
||||
surface: Query<(Option<&RawHandleWrapper>, &CursorOptions), With<PrimaryWindow>>,
|
||||
mut departures: MessageReader<CursorLeft>,
|
||||
mut buttons: Buttons,
|
||||
mut raw: RawInput,
|
||||
@@ -151,8 +172,15 @@ fn swallow_input_under_the_cover(
|
||||
pointer.blanked = false;
|
||||
let stashed = pointer.stashed.take();
|
||||
let left = std::mem::take(&mut pointer.left);
|
||||
let can_land = surface.single().is_ok_and(|(handle, cursor)| {
|
||||
let wayland = matches!(
|
||||
handle.map(RawHandleWrapper::get_display_handle),
|
||||
Some(raw_window_handle::RawDisplayHandle::Wayland(_))
|
||||
);
|
||||
a_warp_can_land(wayland, cursor.grab_mode)
|
||||
});
|
||||
if let Ok(mut window) = window.single_mut() {
|
||||
if here.is_none() && !left && window.focused {
|
||||
if can_land && here.is_none() && !left && window.focused {
|
||||
window.set_physical_cursor_position(stashed);
|
||||
}
|
||||
}
|
||||
@@ -194,6 +222,28 @@ mod tests {
|
||||
use super::*;
|
||||
use bevy::input::ButtonState;
|
||||
|
||||
/// The rule winit's Wayland backend actually enforces, so the hand-back asks only where the
|
||||
/// answer can be yes.
|
||||
#[test]
|
||||
fn a_warp_only_lands_off_wayland_or_with_the_pointer_locked() {
|
||||
assert!(
|
||||
a_warp_can_land(false, CursorGrabMode::None),
|
||||
"X11, Windows and macOS warp a free pointer"
|
||||
);
|
||||
assert!(
|
||||
!a_warp_can_land(true, CursorGrabMode::None),
|
||||
"a free pointer on Wayland cannot be warped by any client \u{2014} this is the ERROR"
|
||||
);
|
||||
assert!(
|
||||
!a_warp_can_land(true, CursorGrabMode::Confined),
|
||||
"confined is not locked: winit's gate is the lock"
|
||||
);
|
||||
assert!(
|
||||
a_warp_can_land(true, CursorGrabMode::Locked),
|
||||
"a locked pointer is the one Wayland case that works, so do not refuse it"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_press_under_the_cover_never_happened_and_a_held_one_is_released_once() {
|
||||
let mut keys = ButtonInput::<KeyCode>::default();
|
||||
|
||||
@@ -131,8 +131,29 @@ pub(crate) struct HostClipboard {
|
||||
}
|
||||
|
||||
impl HostClipboard {
|
||||
/// Resolve the backend on first use, once the window gives the Wayland pointer.
|
||||
fn ensure(&mut self, wl_display: Option<*mut c_void>) {
|
||||
/// Has the backend been resolved yet? The gate on [`open_early`](super::open_early), so the
|
||||
/// opening pass costs one `run_if` for the rest of the run rather than a system that returns.
|
||||
pub(crate) fn is_open(&self) -> bool {
|
||||
self.opened
|
||||
}
|
||||
|
||||
/// Resolve the backend, which [`open_early`](super::open_early) does as soon as the window can
|
||||
/// give the Wayland pointer — **not** on the first paste, and that is the whole point.
|
||||
///
|
||||
/// `smithay-clipboard` runs its own connection and its own `wl_seat`, and it will only read a
|
||||
/// selection for a seat it has `latest_seat` for. That field is set by a `wl_keyboard`
|
||||
/// `Key`/`Modifiers` event or a pointer press — *not* by `Enter`, which only sets `has_focus`
|
||||
/// (`smithay-clipboard-0.7.3/src/state.rs:522`, `:386`). So a backend opened on the first
|
||||
/// Ctrl+V has a keyboard object younger than the chord that asked for the paste: those key
|
||||
/// events were delivered before it existed, nothing has set `latest_seat`, and the load fails
|
||||
/// with "no events received on any seat" — which is exactly what grom's 2026-10-28 log holds,
|
||||
/// once, for his one paste of the session. Any later keypress sets it, so the *second* paste
|
||||
/// of a session worked and the first never could.
|
||||
///
|
||||
/// Opening early is right for the other backends too: arboard's X11 handle is the clipboard
|
||||
/// owner for the run (the note on the resource), so the run should hold it from the start
|
||||
/// rather than from whenever someone first copies.
|
||||
pub(crate) fn ensure(&mut self, wl_display: Option<*mut c_void>) {
|
||||
if self.opened {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -25,9 +25,33 @@ impl Plugin for TextInputPlugin {
|
||||
// Held for the whole run: on X11 dropping the handle clears the clipboard. `NonSend`: no
|
||||
// backend is `Sync`, and NSPasteboard is main-thread-only.
|
||||
app.init_non_send_resource::<HostClipboard>();
|
||||
app.add_systems(Update, open_early.run_if(clipboard_unopened));
|
||||
}
|
||||
}
|
||||
|
||||
/// Only until the backend is resolved, which is once per run: a `run_if` rather than a system
|
||||
/// that returns on line one, since a registered system still moves the frame either way.
|
||||
fn clipboard_unopened(clipboard: Option<NonSend<HostClipboard>>) -> bool {
|
||||
clipboard.is_some_and(|c| !c.is_open())
|
||||
}
|
||||
|
||||
/// Resolve the pasteboard backend as soon as the window can give the Wayland display pointer,
|
||||
/// rather than on the first paste. [`HostClipboard::ensure`] has the reason: a Wayland backend
|
||||
/// opened by the Ctrl+V that wants to read is younger than that chord's own key events, and
|
||||
/// `smithay-clipboard` reads a selection only for a seat it has seen a key or click on.
|
||||
fn open_early(
|
||||
mut clipboard: NonSendMut<HostClipboard>,
|
||||
window: Query<&bevy::window::RawHandleWrapper, With<bevy::window::PrimaryWindow>>,
|
||||
) {
|
||||
// No handle yet is not a failure: the gate holds and the next frame tries again. Calling
|
||||
// `ensure` here would spend the one attempt on a display pointer that does not exist, and
|
||||
// land every Wayland session on arboard, which here speaks only X11.
|
||||
let Ok(handle) = window.single() else {
|
||||
return;
|
||||
};
|
||||
clipboard.ensure(wayland_display(Some(handle)));
|
||||
}
|
||||
|
||||
/// The modifier snapshot for this frame, read once and handed to every [`feed_key`] call.
|
||||
pub(crate) fn mods_now(keys: &ButtonInput<KeyCode>) -> Mods {
|
||||
Mods {
|
||||
@@ -160,6 +184,26 @@ pub(crate) fn tick_caret(field: &mut EditBoxState, focused: bool, dt: f32) -> bo
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// The eager open has exactly one attempt (`HostClipboard::opened`), so spending it on a
|
||||
/// frame with no window handle would strand every Wayland session on arboard, which here
|
||||
/// speaks only X11 — the paste failure this pass exists to fix, made permanent. The gate has
|
||||
/// to still be open after a frame that had no window, and no backend may have been resolved.
|
||||
#[test]
|
||||
fn the_early_open_does_not_spend_its_one_attempt_before_there_is_a_window() {
|
||||
let mut app = App::new();
|
||||
app.add_plugins(TextInputPlugin);
|
||||
app.update();
|
||||
let clipboard = app
|
||||
.world()
|
||||
.get_non_send_resource::<HostClipboard>()
|
||||
.expect("the plugin installs the resource");
|
||||
assert!(
|
||||
!clipboard.is_open(),
|
||||
"no window handle yet, so nothing may have been opened; `clipboard_unopened` is \
|
||||
that same bit, so the gate is still open for the frame that does have one"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn letters_filter_applies_to_pasted_text() {
|
||||
assert_eq!(CharFilter::Letters.keep("Bob123"), "Bob");
|
||||
|
||||
@@ -33,12 +33,20 @@
|
||||
//! `GetGameObject`/`HasGameObject` **reverse this file's other "not built" note** on the same
|
||||
//! mechanism and for a larger measured cost; the reasoning is at the registration, not here.
|
||||
//!
|
||||
//! `SetSpellByID` **reverses that list's third decision**, and is the one entry of the `Set*ByID`
|
||||
//! family built: the shape check that list deferred found there was no aliasing to do, because
|
||||
//! `tooltip_spell::set_spell_by_id` already takes a bare id and every 1.12 spell verb funnels
|
||||
//! through it. The reasoning is at the registration; it cost 7 raises and 7 empty action-button
|
||||
//! tooltips in one session.
|
||||
//!
|
||||
//! Not built, each with the reason:
|
||||
//! - **`SetItemByID`, `SetSpellByID`, `AddSpellByID`, `SetInventoryItemByID`**: these are *setters*
|
||||
//! whose 1.12 counterparts already exist under their own names (`SetHyperlink` for an item id,
|
||||
//! - **`SetItemByID`, `AddSpellByID`, `SetInventoryItemByID`**: these are *setters* whose 1.12
|
||||
//! counterparts already exist under their own names (`SetHyperlink` for an item id,
|
||||
//! `SetSpellBookItem`/`SetTrainerService` for a spell). Adding an id-first alias is cheap but
|
||||
//! would need its own argument-shape decisions, and the readers above are what addons are
|
||||
//! actually blocked on; left for a pass that can check each against the stock verb it aliases.
|
||||
//! would need its own argument-shape decisions, and nothing installed here has asked for one:
|
||||
//! the addon that needed the spell id reaches items through the stock `SetBagItem` and
|
||||
//! `SetInventoryItem` on the very next branch (`Core.lua:4223`). Each is a few lines when a log
|
||||
//! names it, on the pattern `SetSpellByID` set.
|
||||
//! - **`SetTalentByID`, `SetEquipmentSet`, `SetTotem`**: no backing — `Talent.dbc` is not
|
||||
//! reachable from this crate, 1.12 has no equipment-set feature at all, and no totem state
|
||||
//! exists (`docs/ADDON_COMPAT.md`'s `Totem` row).
|
||||
@@ -194,6 +202,63 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
|
||||
)?,
|
||||
)?;
|
||||
|
||||
// `SetSpellByID(spellID)`: fill this tooltip from a spell's own id. **This reverses this
|
||||
// file's "not built" note on the `Set*ByID` family**, for the one member of it that costs
|
||||
// errors, and the note's objection is answered rather than waived: it said each alias needs
|
||||
// its own argument-shape check against the 1.12 verb it stands in for. Done, and the check is
|
||||
// what makes this four lines — `tooltip_spell::set_spell_by_id` *is* the body every spell
|
||||
// verb in this client already funnels through (`SetSpell`, `SetShapeshift`, `SetPetAction`,
|
||||
// `SetTrainerService`), and it takes a bare spell id, because 1.12's own verbs all have to
|
||||
// turn their book slot or list row into one first. So there is no aliasing here at all: this
|
||||
// is the shared body reached by the argument it actually wants.
|
||||
//
|
||||
// `show_rank` is the one decision the shape check had to make, and it is **true**, which is
|
||||
// `SetAction`'s choice rather than `SetSpell`'s. The split is not arbitrary: the spellbook
|
||||
// draws "Rank N" itself as the button's `subSpellName` (`SpellBookFrame.lua`), so its tooltip
|
||||
// would say it twice, while an action button has no such label and its tooltip carries the
|
||||
// rank. A caller naming a spell by id has no button label either, and the one caller here is
|
||||
// standing in for exactly an action button's tooltip, so it takes the action's shape.
|
||||
//
|
||||
// An id with no cached tooltip view renders empty and *asks* for it
|
||||
// (`tooltip_spell::spell_view_of` files a `spell_tooltip_asks` entry `ui_tooltip`'s feed
|
||||
// drains), which is how every other spell verb behaves on a cold id. Here the cold id is the
|
||||
// **common** case rather than the rare one, and the caller says so: this path is the one
|
||||
// CleveRoids falls back to for a `spell:<id>` macro naming a spell **not in either
|
||||
// spellbook** (`Extensions/Tooltip/Generic.lua:444` — if `FindSpellBookSlotByID` finds a slot
|
||||
// it uses the stock `SetSpell` instead, at `Core.lua:4244`). That is still fine without a
|
||||
// special case: the app's feed builds a view for any id in the spell catalog, book or not, and
|
||||
// an action button re-asks its own tooltip every `TOOLTIP_UPDATE_TIME` while the pointer rests
|
||||
// on it, so the fill lands within 0.2 s of the hover rather than on the next one.
|
||||
//
|
||||
// Built because it raises on **every action-bar hover** of a macro that resolves to a spell:
|
||||
// SuperCleveRoidMacros tooltips a macro's resolved spell through it
|
||||
// (`SuperCleveRoidMacros/Core.lua:4217`), reached from pfUI's own `ButtonEnter`
|
||||
// (`pfUI/modules/actionbar.lua:449`), so the hover raises *and* leaves the button with no
|
||||
// tooltip at all. 7 raises in one 19-minute session of grom's, which is 7 buttons he pointed
|
||||
// at and got nothing from.
|
||||
m.set(
|
||||
"SetSpellByID",
|
||||
lua.create_function(|lua, (this, spell_id): (Table, Value)| {
|
||||
// `lua_isnumber`'s acceptance, as the rest of this surface uses; a nil or a
|
||||
// non-number is nothing to show, not a raise, because every caller reaches this from
|
||||
// a lookup that may have missed.
|
||||
let Some(id) = as_number(&spell_id).filter(|n| *n >= 0.0) else {
|
||||
return Ok(());
|
||||
};
|
||||
crate::script::tooltip_spell::set_spell_by_id(
|
||||
lua,
|
||||
&this,
|
||||
id as u32,
|
||||
None,
|
||||
crate::script::tooltip_spell::SpellRenderOpts {
|
||||
show_rank: true,
|
||||
..Default::default()
|
||||
},
|
||||
None,
|
||||
)
|
||||
})?,
|
||||
)?;
|
||||
|
||||
// `GetItem()` → `(name, link, itemID)`, nothing at all for a tooltip not showing an item.
|
||||
// Returning no values (rather than nils) is the upstream's documented shape and is what
|
||||
// `local name, link = tip:GetItem() if name then` branches on.
|
||||
@@ -641,6 +706,93 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// The defect this verb was built for: an action-bar hover of a macro that resolves to a
|
||||
/// spell. The addon reaches it with nothing but the id (`SuperCleveRoidMacros/Core.lua:4217`),
|
||||
/// so the whole plate has to come from that one argument.
|
||||
#[test]
|
||||
fn set_spell_by_id_renders_a_spell_from_its_id_alone() {
|
||||
use crate::script::SpellTooltipView;
|
||||
let mut s = UiScript::new().unwrap();
|
||||
s.set_screen_size(800.0, 600.0);
|
||||
s.set_spell_tooltip(
|
||||
133,
|
||||
SpellTooltipView {
|
||||
name: "Fireball".into(),
|
||||
rank: Some("Rank 1".into()),
|
||||
cost: Some("30 Mana".into()),
|
||||
range: Some("30 yd range".into()),
|
||||
cast_time: Some("1.5 sec cast".into()),
|
||||
description: "Hurls a fiery ball.".into(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
s.run(
|
||||
r#"
|
||||
local b = CreateFrame("Button", "CAIdHost"); b:SetPoint("CENTER", 0, 0)
|
||||
b:SetWidth(10); b:SetHeight(10)
|
||||
tip = CreateFrame("GameTooltip", "CAIdTip")
|
||||
tip:SetOwner(b, "ANCHOR_RIGHT")
|
||||
tip:SetSpellByID(133)
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
assert!(
|
||||
s.eval::<bool>("return tip:IsShown()").unwrap(),
|
||||
"the plate has to come up, not just fill: the addon's own Show() follows but the \
|
||||
button had no tooltip at all while this raised"
|
||||
);
|
||||
assert_eq!(
|
||||
s.eval::<String>("return CAIdTipTextLeft1:GetText()")
|
||||
.unwrap(),
|
||||
"Fireball"
|
||||
);
|
||||
// The one shape decision: the action's `showRank`, not the spellbook's, because a caller
|
||||
// naming a spell by id has no `subSpellName` label to read the rank off.
|
||||
assert!(
|
||||
s.eval::<bool>("return CAIdTipTextRight1:IsShown()")
|
||||
.unwrap(),
|
||||
"the rank column shows, as it does on an action button"
|
||||
);
|
||||
assert_eq!(
|
||||
s.eval::<String>("return CAIdTipTextRight1:GetText()")
|
||||
.unwrap(),
|
||||
"Rank 1"
|
||||
);
|
||||
// Through the one renderer, so the subject is recorded and this file's own readers answer.
|
||||
let (name, rank, id) = s
|
||||
.eval::<(String, String, i64)>("return tip:GetSpell()")
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
(name.as_str(), rank.as_str(), id),
|
||||
("Fireball", "Rank 1", 133)
|
||||
);
|
||||
assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
|
||||
}
|
||||
|
||||
/// Every caller reaches this from a lookup that can miss, so neither a nil nor an id with no
|
||||
/// cached view may raise — the raise is the whole defect.
|
||||
#[test]
|
||||
fn set_spell_by_id_takes_a_miss_and_a_nil_without_raising() {
|
||||
let s = UiScript::new().unwrap();
|
||||
s.run(
|
||||
r#"
|
||||
local b = CreateFrame("Button", "CAMissHost"); b:SetPoint("CENTER", 0, 0)
|
||||
tip = CreateFrame("GameTooltip", "CAMissTip")
|
||||
tip:SetOwner(b, "ANCHOR_RIGHT")
|
||||
tip:SetSpellByID(nil)
|
||||
tip:SetSpellByID(133)
|
||||
tip:SetSpellByID("not a spell")
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
|
||||
assert_eq!(
|
||||
s.arity("tip:GetSpell()").unwrap(),
|
||||
0,
|
||||
"nothing was shown, so nothing is the subject"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clearing_the_tooltip_drops_the_subject() {
|
||||
let s = tip_with_sword();
|
||||
|
||||
@@ -209,6 +209,11 @@ const ALLOWED_BEYOND: &[(&str, &str, &str)] = &[
|
||||
"GameTooltip",
|
||||
"ClassicAPI: whether the plate is showing a world object, the GetGameObject sibling Has* every other subject already has",
|
||||
),
|
||||
(
|
||||
"SetSpellByID",
|
||||
"GameTooltip",
|
||||
"ClassicAPI: fill the plate from a spell id alone. The one Set*ByID the family's \"needs an argument-shape check first\" note was lifted for, and the check is why it is allowed rather than removed: tooltip_spell::set_spell_by_id is the body all four of 1.12's own spell verbs funnel through and it already takes a bare id, so this adds a door onto existing behaviour and no behaviour of its own. SuperCleveRoidMacros/Core.lua:4217 reaches it from pfUI's ButtonEnter for a spell:<id> macro naming a spell in neither spellbook; its absence was 7 raises in one session and 7 action buttons with no tooltip at all",
|
||||
),
|
||||
(
|
||||
"GetOwner",
|
||||
"GameTooltip",
|
||||
|
||||
@@ -256,7 +256,7 @@ separately, so "cannot say" has to be distinguishable from "no".
|
||||
| CreatureInfo (`C_CreatureInfo`) | partial | `compat/classic_api/creature_info.rs`, the 3 of 9 anything installed actually calls. `GetRaceInfo` and `GetFactionInfo` read a **new app→VM feed of the player's own `ChrRaces.dbc`** (`benilla-formats`' `chr_races.rs` → `benilla-app`'s `chr_races.rs` → `UiScript::set_race_info`), built because this crate holds no DBC and a frozen race table gets a modified realm's names wrong: **OctoWoW's row 10 displays "High Elf" where Blizzard's says "Blood Elf"**, while both keep `clientFileString "BloodElf"` for the art paths — the two columns genuinely disagree, so neither can be derived from the other, and a test pins that. The side on `GetFactionInfo` comes from `ui_unit.rs`'s `race_faction_group`, the same answer `UnitFactionGroup("player")` gives, so the two cannot disagree about a race; a non-playable row carries no side and reads nil rather than a guess. **`GetRaceInfo` answering nil past the last race is load-bearing** — pfUI walks `i = 1, 2, 3…` until nil, so an always-non-nil answer hangs the client at load; its own test says so. `GetCreatureID` decodes the template entry out of bits 24-47 of the 16-hex-digit GUID token the rest of `compat/` passes around, accepting **only** the creature high word so a player or pet GUID answers nil instead of a plausible-looking wrong number (the decode is inlined rather than calling `benilla_protocol::guid::entry` — this crate depends on no protocol or engine crate, and one shift is not worth breaking that boundary for). Not built: `GetCreatureInfoByID`, `RequestLoadCreatureByID`, `GetCreatureFamilyInfo`/`IDs`, `GetCreatureTypeInfo`/`IDs` (each needs a creature/family/type table this VM is not fed, and nothing installed asks) and `GetClassInfo` (the class table exists app-side as `ChrClasses` but has no VM feed yet — the same shape of work this row's race feed just did, if an addon ever wants it). With no install the race table stays empty and every reader answers nil, so an addon's race walk finds none and still loads |
|
||||
| Item (`C_Item`) | partial | `compat/classic_api/item.rs`, 37 entries. Modern id-first and `ItemLocation`-keyed readers over the same state the stock item verbs read: `Model::item_templates` (the one `ItemTemplateView` cache every item tooltip renders through — a miss records the same `CMSG_ITEM_QUERY` ask the stock `GetItemInfo` does, so a modern caller warms the one shared cache, not a parallel one), `Model::containers` and `Model::inventory_slots`. **`ItemLocation` is accepted as the plain table addons build** (`{bagID=, slotIndex=}` / `{equipmentSlotIndex=}`): modern WoW's mixin is pure Lua sugar over exactly those two fields and every entry point only reads them, so the table *is* the contract; the mixin constructors themselves belong with `Mixin`/`CreateFromMixins` in the `Lua` namespace and are not faked here. Built: the existence/identity trio, `GetItemData`/`GetItemDataByID` (the upstream's kitchen-sink table, including the `isConjured`/`isOpenable`/`isLootable`/`isWrapper` flag bools off the template's raw `flags`), `GetItemInfoInstant`, the by-id and by-location getter pairs (name, quality, max stack, sell price, set id, inventory type, icon), `GetItemLink`, `GetStackCount`, `GetItemCount` (bags + worn, with the `includeBank` and charge-multiplying `includeUses` flags), `GetItemSpell` (the on-use trigger only — triggers 0/5, not an equip or chance-on-hit effect), `IsUsableItem`/`IsConsumableItem`/`IsEquippableItem`/`IsEquippedItem`/`IsItemOpenable`/`IsBound`, the `IsItemDataCached` pair (which deliberately record **no** ask — polling is not asking), the two inventory-slot name getters, and `UseItemByName`/`EquipItemByName` over the stock container-use queue. Not built, each with the reason in that file's header: the **guid** family (`GetItemGUID`, `GetItemLocation`, `IsItemGUIDInInventory`, …) — nothing in the model keys items by instance guid, and the modern `"Item-Server-…"` string has no source here; the **item-level** family (`GetCurrentItemLevel`, `GetDetailedItemLevelInfo`, `GetAverageItemLevel`) — item level is not in `SMSG_ITEM_QUERY_SINGLE_RESPONSE` and 1.12 exposes no such concept to the client; `GetItemStats`/`GetItemStatDelta` — the modern shape is keyed by `ITEM_MOD_*` *string* names resolved app-side, and returning the raw numeric keys under the modern name would silently mismatch; `GetItemUniqueness` (wants a limit-category pair 1.12 has no counterpart for); the `LockItem`/`UnlockItem` family (`ContainerSlot::locked` is a pending-operation flag the app owns, not a lock Lua may set); and `UseAtCursor`/`UseAtUnit`/`PickupItem`/`RequestLoadItemData*` (each needs an app-drained queue that does not exist yet) |
|
||||
| AddOns (`C_AddOns`) | done | `compat/classic_api/addons.rs`, 10 entries, and deliberately **pure delegation**: every one calls the stock global (`script/addon.rs`) rather than reading `Model::addons` a second time, so the index-or-name parsing, the unknown-name placeholder answers and the load-gate verdicts keep one owner — a caller cannot get a different answer from `C_AddOns.IsAddOnLoaded` than from `IsAddOnLoaded`. `GetAddOnName`/`Title`/`Notes` and `IsAddOnLoadable` are `GetAddOnInfo` returns 1/2/3/5; `IsAddOnLoaded`/`LoadAddOn` pass straight through; `DoesAddOnExist` reads the `"MISSING"` reason rather than "something came back"; `GetAddOnSecurity` maps the stock `SECURE`/`INSECURE` string onto the upstream's `Enum.AddOnSecurityStatus`, answering `NotAvailable` (3) for an absent addon rather than defaulting to insecure (`Banned` (2) is unreachable — it comes from a server addon-banlist this client does not model). `GetAddOnOptionalDependencies` answers **none**, not the required list: 1.12's `.toc` has no optional-dependency directive at all, and returning the required ones would make a caller treat a hard dependency as skippable. `GetAddOnLocalTable` is not built — modern WoW passes each addon file a private table as a load argument and 1.12 passes addon files no arguments, so there is no such table to hand back; inventing an empty one would read as "my locals are empty" rather than "this client has no such feature". A fixture note worth keeping: the addon *index* array stays empty until the server's `SMSG_ADDON_INFO` reply lands, so any by-index read raises before it — faithful to 1.12, and what a test must call `note_addon_info_reply` to exercise |
|
||||
| GameTooltip (reader methods) | partial | `compat/classic_api/tooltip.rs`, 8 widget **methods** (not globals) on the GameTooltip class table, same linking mechanism `frame.rs` documents. The valuable half of the namespace: addons constantly ask "what is this tooltip showing?" before annotating it. This needed **new state** — `TooltipState::subject` (`widget/kinds/mod.rs`, with a `TooltipSubject` item/spell enum): 1.12 keeps the same facts (the reference holds the item id at `tooltip+0x398`) but exposes no getter, so nothing in this client read them. Recorded at the **two renderers every verb funnels through** — `tooltip_item/render.rs`'s `render_view` (12 call sites, each now passing its own `item_id`, so the compiler enforced coverage) and `tooltip_spell.rs`'s `render_spell` — which is why `SetBagItem`, `SetMerchantItem`, `SetLootItem`, `SetHyperlink` and the rest all answer `GetItem` without each needing its own wiring; cleared with the content, so a `ClearLines` drops it. Built: `GetItem` (name, link, itemID), `GetSpell` (name, rank, spellID), `HasItem`, `HasSpell`, `GetUnit` (over the `unit_token` the stock `SetUnit` already kept), `HasUnit`, `GetOwner` (returns the real frame object through the same `frame_wrapper` `GetParent` uses), `IsEquippedItem`. Each returns *no values* rather than nils when there is no subject, the upstream's documented shape and what `if name then` branches on. Not built, each with the reason: `GetUnitGUID` — the modern getter promises the guid as of when the tooltip was set, and resolving the stored *token* now would be right most of the time and silently wrong exactly when it matters (a `"mouseover"` tooltip outliving its mouseover); `GetGameObject`/`HasGameObject` — no GameObject subject is recorded anywhere and there is no object-template cache keyed like `item_templates`; the `Set*ByID` aliases (`SetItemByID`, `SetSpellByID`, `AddSpellByID`, `SetInventoryItemByID`) — the stock verbs they alias already exist under 1.12 names and each needs its own argument-shape check against them; `SetTalentByID`/`SetEquipmentSet`/`SetTotem` — no backing (`Talent.dbc` unreachable from this crate, no equipment-set feature in 1.12 at all, no totem state). Since built: `SetHyperlinkCompareItem`, which this list had turned down on the grounds that the compare chain needs a shopping-tooltip pair — wrong, because 1.12's own merchant and auction compares drive that pair already (its own row below) |
|
||||
| GameTooltip (reader methods) | partial | `compat/classic_api/tooltip.rs`, 8 widget **methods** (not globals) on the GameTooltip class table, same linking mechanism `frame.rs` documents. The valuable half of the namespace: addons constantly ask "what is this tooltip showing?" before annotating it. This needed **new state** — `TooltipState::subject` (`widget/kinds/mod.rs`, with a `TooltipSubject` item/spell enum): 1.12 keeps the same facts (the reference holds the item id at `tooltip+0x398`) but exposes no getter, so nothing in this client read them. Recorded at the **two renderers every verb funnels through** — `tooltip_item/render.rs`'s `render_view` (12 call sites, each now passing its own `item_id`, so the compiler enforced coverage) and `tooltip_spell.rs`'s `render_spell` — which is why `SetBagItem`, `SetMerchantItem`, `SetLootItem`, `SetHyperlink` and the rest all answer `GetItem` without each needing its own wiring; cleared with the content, so a `ClearLines` drops it. Built: `GetItem` (name, link, itemID), `GetSpell` (name, rank, spellID), `HasItem`, `HasSpell`, `GetUnit` (over the `unit_token` the stock `SetUnit` already kept), `HasUnit`, `GetOwner` (returns the real frame object through the same `frame_wrapper` `GetParent` uses), `IsEquippedItem`. Each returns *no values* rather than nils when there is no subject, the upstream's documented shape and what `if name then` branches on. Not built, each with the reason: `GetUnitGUID` — the modern getter promises the guid as of when the tooltip was set, and resolving the stored *token* now would be right most of the time and silently wrong exactly when it matters (a `"mouseover"` tooltip outliving its mouseover); `GetGameObject`/`HasGameObject` — no GameObject subject is recorded anywhere and there is no object-template cache keyed like `item_templates`; the rest of the `Set*ByID` aliases (`SetItemByID`, `AddSpellByID`, `SetInventoryItemByID`) — the stock verbs they alias already exist under 1.12 names, each needs its own argument-shape check against them, and no installed addon has asked for one; `SetTalentByID`/`SetEquipmentSet`/`SetTotem` — no backing (`Talent.dbc` unreachable from this crate, no equipment-set feature in 1.12 at all, no totem state). Since built: `SetHyperlinkCompareItem`, which this list had turned down on the grounds that the compare chain needs a shopping-tooltip pair — wrong, because 1.12's own merchant and auction compares drive that pair already (its own row below); and `SetSpellByID`, where the deferred shape check found there was no aliasing to do at all — `tooltip_spell::set_spell_by_id` is the shared body every 1.12 spell verb already funnels through and it takes a bare id, so the verb is the existing body reached by the argument it wants. `showRank` is the action's, not the spellbook's, because a caller naming a spell by id has no `subSpellName` label to read the rank off. 7 raises and 7 empty action-button tooltips in one session of grom's: SuperCleveRoidMacros tooltips a macro's resolved spell through it (`Core.lua:4217`), off pfUI's `ButtonEnter` |
|
||||
| Quest (`C_QuestLog`) | partial | `compat/classic_api/quest_log.rs`, questID-keyed over the same log rows the stock verbs index by position — the gap being that 1.12's API never hands Lua a questID at all, so an addon can otherwise only key by an index that shifts when the log changes. Built: `GetInfo`, `GetQuestLogTitle` (the documented positional list, whose third value is `suggestedGroup` where the stock global keeps the tag — both coexist), `GetQuestIDForLogIndex` (0 for a header, nil out of range), `GetLogIndexForQuestID`, `GetHeaderIndexForQuest`, `IsOnQuest` (counts rows folded under a collapsed header, via `hidden_quest_ids`), `IsUnitOnQuest` (answers for `"player"`; false for any other token, since another unit's quest list is not tracked — the same blocker the stock `IsUnitOnQuest` already has), `GetNumQuestObjectives`. Not built: `RequestLoadQuestByID`/`IsQuestDataCachedByID`/`GetTitleForQuestID`/`GetQuestDetails` — all four probe a *standalone* questID-keyed static-info cache fed by `CMSG_QUEST_QUERY`, which benilla has none of (`QuestLogEntryView::detail` is per-log-row, so nothing is knowable about a quest not in the log, and `QUEST_DATA_LOAD_RESULT` has no sender); a cache plus its query is app-side plumbing. The bare `GetQuestLogLeaderBoardID` is also not built: `QuestLogObjectiveView` carries `kind`/`text`/`cur`/`req` but no creature/object/item id, so there is nothing to return — closing it means carrying that id along from the app's template read |
|
||||
| Faction (`C_Reputation` + 3 bare globals) | partial | `compat/classic_api/faction.rs`. Bare: `GetFactionIDByIndex`, `GetFactionInfoByID` (the same eleven values `GetFactionInfo` returns, keyed by id), `GetFactionParentID`. `C_Reputation`: `GetFactionStandings`, `GetFactionDataByID`/`ByIndex`, `GetWatchedFactionData`, `SetSelectedFactionByID`, `SetWatchedFactionByID`, `ToggleFactionAtWarByID`, `IsFactionActive`/`ByID`, `SetFactionInactiveByID`/`SetFactionActiveByID`. Every setter queues onto the **same** `reputation_sends` queue the stock pane's own toggles use, so an id-keyed call and an index-keyed one are indistinguishable downstream and cannot race; a faction not in the list sends nothing rather than a wrong slot. **One scope limit, stated once in that file's header:** upstream answers for any `Faction.dbc` row including unencountered factions (DBC name + Neutral defaults), and `benilla-ui` holds no DBC, so these answer for a **listed** faction and nil otherwise — narrower reach, identical shape for anything a caller reaches by walking `GetNumFactions()`. Not built: `GetLastStandingChange` — `ReputationState` is a whole-list snapshot with no delta or history to answer from |
|
||||
| FriendList (`C_FriendList`) | partial | `compat/classic_api/friend_list.rs`, named tables over `Model.social`: `GetNumFriends`, `GetNumOnlineFriends`, `GetFriendInfoByIndex`, `GetFriendInfo` (by name, case-insensitive), `IsIgnored`, `SendWhoQueryByName` (queues the stock `SocialRequest::Who`, so modern and stock `/who` share one request path), `GetNumWhoResults` (both counts), `GetWhoInfo`. Not built: `SetFriendNotes`/`ByIndex` — friend notes are a 2.x feature and `SMSG_FRIEND_LIST` carries no note field, so accepting one would make a notes UI look like it worked; `IsFriend`/`IsIgnoredByGuid` — both key on a GUID and the social lists hold names only (the app resolves guid→name before pushing, keeping no reverse map), while the name-keyed `IsIgnored` sibling is built. `IsWhoQueryPending` answers false always: the app drains the request and pushes the reply as a whole new state, keeping no in-flight marker |
|
||||
|
||||
Reference in New Issue
Block a user