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

modelframe: a unit pane holds the unit it was given

Browse Source 
`SetUnit` resolved nothing: it stored the token and the extract re-read it
every paint. unitscan's alert is the live consumer — it `SetUnit'target'`s the
rare it just found by name, and the addon restores the player's own target on
the very next line — so the card showed the rare for one frame and then went
empty. It only looked right while our by-name targeting was leaving the target
on the rare, which is the accident the previous commit removed.

The reference resolves the token in `SetUnit`'s own rebuild worker (`0x505b50`,
the one `RefreshUnit` also runs) and nothing re-resolves it per paint. Three
things say so independently: FrameXML re-issues `SetUnit` from
`PaperDollFrame_OnShow` and on `UNIT_MODEL_CHANGED`, which would be pointless
against a live token; `RefreshUnit` exists at all; and pfUI detects a portrait
change with *no event* by pinning a scratch pane and comparing `GetModel`
against the shown one (`api/unitframes.lua:2069`), a test that says nothing
unless a pane holds what it was given.

`ModelState::unit_guid` is that pin, read through one helper
(`Model::unit_pane_subject`) so the paint gate and the extract cannot disagree
about whether a pane has a subject. A pane seated *before* its unit streamed in
keeps re-reading its token — ours arrives asynchronously where the reference's
is already in the object manager — and `RefreshUnit` re-pins, as its worker
does.

`modelframe.rs`'s own extract test passed only because its `SetUnit` ran with no
target; its comment claimed the general rule, so it now states the real one and
pins both halves.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
grom
2026-10-17 13:41:35 -06:00
co-authored by Claude Opus 5
parent 92290bb18f
commit 0807144f38
7 changed files with 174 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/benilla-ui/src/script/extract.rs
+7 -7
View File
@@ -94,14 +94,14 @@ impl UiScript {
name: frame.and_then(|f| f.name.clone()),
model: m.path.clone(),
skins: m.skins.clone(),
// Resolved here, where the token grammar lives
// ([`super::types::UnitPaneSubject`]); a token naming nothing right
// now leaves the pane with no subject, which empties its booth.
unit: m.unit.as_deref().and_then(|token| {
model.unit(token).map(|u| super::types::UnitPaneSubject {
guid: u.guid,
// The guid `SetUnit` pinned, or — for a pane seated before its unit
// existed — its token re-read here, where the token grammar lives
// ([`super::types::UnitPaneSubject`]). No subject empties the booth.
unit: model.unit_pane_subject(m).map(|guid| {
super::types::UnitPaneSubject {
guid,
camera: m.unit_camera,
})
}
}),
facing: m.facing,
model_scale: m.scale,
crates/benilla-ui/src/script/model.rs
+15
View File
@@ -1403,6 +1403,21 @@ impl Model {
self.warnings.push(msg);
}
/// A model pane's subject: the guid its `SetUnit` pinned ([`crate::widget::ModelState::unit_guid`]),
/// else its token re-read, for a pane seated before its unit streamed in.
///
/// One reader for the paint gate and the extract, so the two cannot disagree about whether the
/// pane has anything to draw. `Some(0)` is a token naming a unit with no guid, which only a
/// harness feed produces; the app resolves it to no entity, as it always has.
pub(crate) fn unit_pane_subject(&self, m: &crate::widget::ModelState) -> Option<u64> {
m.unit_guid.or_else(|| {
m.unit
.as_deref()
.and_then(|token| self.unit(token))
.map(|u| u.guid)
})
}
/// A unit token's state, folded as 1.12's resolver `0x515970` folds (`_strnicmp`: `A`..`Z`
/// only, never a byte of 0x80 or more), so `to_ascii_lowercase`, never `to_lowercase`.
pub(crate) fn unit(&self, token: &str) -> Option<&UnitState> {
crates/benilla-ui/src/script/modelframe.rs
+27 -5
View File
@@ -516,12 +516,26 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
playermodel_install(lua)
}
/// The guid a `SetUnit`/`RefreshUnit` token names right now, which the pane then holds
/// ([`crate::widget::ModelState::unit_guid`]).
///
/// A unit with no guid — only a harness feed has one — is left unpinned, so such a pane keeps
/// re-reading its token and the paint gate behaves as it did before pinning.
fn pinned_guid(lua: &Lua, token: Option<&str>) -> Option<u64> {
let token = token?;
let model = lua.app_data_ref::<Model>().expect("model app_data");
model.unit(token).map(|u| u.guid).filter(|&guid| guid != 0)
}
/// `PlayerModel`'s own three methods (table `0x84f1fc`); the rest are `Model`'s, reached on a miss.
fn playermodel_install(lua: &Lua) -> mlua::Result<()> {
let m = lua.create_table()?;
// `SetUnit(unit)` (`0x84f1fc[0]` → `0x505d70`) clears the `SetModel` path; the unit token is
// resolved at render.
// `SetUnit(unit)` (`0x84f1fc[0]` → `0x505d70`) clears the `SetModel` path and resolves the
// token **here**, pinning the guid it named: the reference's worker (`0x505b50`, the one
// `RefreshUnit` also runs) rebuilds the pane's model instance at the call, and nothing
// re-resolves the token per paint. See [`crate::widget::ModelState::unit_guid`] for the
// evidence and for the one case that keeps re-reading the token.
//
// **A number is a creature display id**, not a token. That is the realm's extension, not
// 1.12's, and OctoWoW's shop is built on it: every mount, companion and illusion card calls
@@ -567,8 +581,10 @@ fn playermodel_install(lua: &Lua) -> mlua::Result<()> {
Value::String(s) => Some(s.to_str()?.to_string()),
_ => None,
};
let guid = pinned_guid(lua, unit.as_deref());
with_model(lua, &this, |m| {
m.unit = unit;
m.unit_guid = guid;
m.path = None;
// The duplicate's load installs the characterinfo camera (`0x505890`); an addon
// that wants the portrait bust asks for index 0 after this call, as pfUI does.
@@ -579,12 +595,18 @@ fn playermodel_install(lua: &Lua) -> mlua::Result<()> {
})?,
)?;
// `RefreshUnit()` (`0x84f1fc[1]` → `0x505e40`) re-reads the pane's unit; the token already
// resolves at render, so only a dress-up pane has work to do.
// `RefreshUnit()` (`0x84f1fc[1]` → `0x505e40`) re-reads the pane's unit: the same worker as
// `SetUnit`'s (`0x505b50`), so the token is resolved again and the pane re-pinned to whatever
// it names now — a unit gone from under it leaves the pane with nothing, as the reference's
// rebuild does.
m.set(
"RefreshUnit",
lua.create_function(|lua, this: Table| {
with_model(lua, &this, |_| ())?;
let token = with_model(lua, &this, |m| m.unit.clone())?;
if token.is_some() {
let guid = pinned_guid(lua, token.as_deref());
with_model(lua, &this, |m| m.unit_guid = guid)?;
}
// The same worker as `SetUnit`'s (`0x505b50`).
super::dressup::redress_if_dressup(lua, &this)
})?,
crates/benilla-ui/src/script/tests/model_clock.rs
+78
View File
@@ -361,6 +361,84 @@ fn a_unit_panes_handler_fires_once_its_token_names_a_unit() {
assert!(s.errors().is_empty(), "{:?}", s.errors());
}
/// A pane holds the unit it was **given**, not whatever its token means later.
///
/// unitscan is the whole reason: its alert reads `SetUnit'target'` on the rare it just targeted by
/// name, and the line after that restores the player's own target. A pane that re-read `"target"`
/// per paint therefore showed the card for one frame and then went empty — which is exactly what
/// grom saw once the by-name targeting stopped leaving his target on the rare. The reference
/// resolves the token in `SetUnit`'s own rebuild (`0x505b50`), so the card keeps the rare.
#[test]
fn a_unit_pane_keeps_the_unit_it_was_given_when_the_token_moves_on() {
let mut s = script();
s.set_screen_size(800.0, 600.0);
const RARE: u64 = 0xf130_0001_0000_0001;
const SELF: u64 = 0x2a;
s.set_unit(
"target",
Some(crate::script::UnitState {
name: Some("Ghostcrawler".into()),
guid: RARE,
..Default::default()
}),
);
s.run(
r#"
seen = 0
m = CreateFrame("PlayerModel", "U", UIParent)
m:SetScript("OnUpdateModel", function() seen = seen + 1 end)
m:SetUnit("target")
"#,
)
.unwrap();
assert_eq!(s.model_pane("U").unwrap().unit_guid, Some(RARE));
// The restore, one Lua line later in the addon: the token now names someone else.
s.set_unit(
"target",
Some(crate::script::UnitState {
name: Some("Timmy".into()),
guid: SELF,
..Default::default()
}),
);
s.tick(0.1);
s.tick(0.1);
assert_eq!(
s.model_pane("U").unwrap().unit_guid,
Some(RARE),
"the card still shows the rare"
);
assert_eq!(
s.eval::<i64>("return seen").unwrap(),
2,
"and it still paints, which is where unitscan makes it visible"
);
// `RefreshUnit` is the verb that re-reads the token, as the reference's rebuild does.
s.run("U:RefreshUnit()").unwrap();
assert_eq!(s.model_pane("U").unwrap().unit_guid, Some(SELF));
// A token the pane was seated on before its unit streamed in keeps being re-read, so nothing
// is lost when `SetUnit` runs too early.
s.run(r#"e = CreateFrame("PlayerModel", "E", UIParent) e:SetUnit("party1")"#)
.unwrap();
assert_eq!(s.model_pane("E").unwrap().unit_guid, None);
s.set_unit(
"party1",
Some(crate::script::UnitState {
name: Some("Later".into()),
guid: 0x99,
..Default::default()
}),
);
assert_eq!(
s.model_ref().unit_pane_subject(&s.model_pane("E").unwrap()),
Some(0x99)
);
assert!(s.errors().is_empty(), "{:?}", s.errors());
}
/// `0x719370` enqueues the completion before it tests the loop flag, so a loop's first pass fires
/// `OnAnimFinished`; a pane with no file fails `0x76d24c`'s gate and never paints.
#[test]
crates/benilla-ui/src/script/tests/modelframe.rs
+23 -5
View File
@@ -525,11 +525,14 @@ fn a_unit_panes_camera_is_set_cameras_index_over_set_units_default() {
);
}
/// The extract resolves the pane's unit token to a GUID, because the host's booth looks a unit up
/// by GUID and the token grammar belongs to this crate. A token naming nothing carries no
/// subject, which is what empties the booth rather than leaving it on a stale body.
/// The extract carries the pane's subject as a GUID, because the host's booth looks a unit up by
/// GUID and the token grammar belongs to this crate. A pane with no subject carries none, which
/// is what empties the booth rather than leaving it on a stale body.
///
/// Which guid: the one the pane **pinned** at its `SetUnit` ([`crate::widget::ModelState::unit_guid`]),
/// or — only for a pane whose token named nothing at the call, as here — the token re-read.
#[test]
fn a_unit_pane_extracts_the_guid_its_token_names_now() {
fn a_unit_pane_extracts_the_guid_it_pinned_or_rereads_a_token_that_named_nothing() {
use crate::script::{QuadContent, UnitState};
let subject = |s: &UiScript| {
@@ -571,7 +574,8 @@ fn a_unit_pane_extracts_the_guid_its_token_names_now() {
"the installed camera travels with the subject"
);
// A new target is a new subject with no second `SetUnit`: the token is re-read every paint.
// This pane never pinned — its `SetUnit` ran before the client had a target — so it is still
// re-reading the token, and a new target is a new subject with no second `SetUnit`.
s.set_unit(
"target",
Some(UnitState {
@@ -582,6 +586,20 @@ fn a_unit_pane_extracts_the_guid_its_token_names_now() {
);
assert_eq!(subject(&s).map(|u| u.guid), Some(0xfeed_face));
// A `SetUnit` made while the token does name someone pins that someone, and the next target
// does not move it: see `model_clock.rs`'s
// `a_unit_pane_keeps_the_unit_it_was_given_when_the_token_moves_on` for why unitscan needs it.
s.run("MSubj:SetUnit(\"target\")").unwrap();
s.set_unit(
"target",
Some(UnitState {
guid: 0x0bad_c0de,
exists: true,
..Default::default()
}),
);
assert_eq!(subject(&s).map(|u| u.guid), Some(0xfeed_face));
// A file pane has no subject at all, whichever it was asked for last.
s.run(r#"MSubj:SetModel("Creature\\Wolf\\Wolf.mdx")"#)
.unwrap();
crates/benilla-ui/src/script/tick.rs
+6 -9
View File
@@ -403,15 +403,15 @@ impl UiScript {
// the handler (`0x76d1bc`) is gated on `[widget+0x318] ≠ 0` (`0x76d24c`), which is the
// pane's **loaded model**, not the file string.
//
// A `SetUnit` pane has one of those too, and keeps no path at all — its subject
// resolves per render, app-side (`modelframe.rs`'s `SetUnit` writes `unit` and nils
// A `SetUnit` pane has one of those too, and keeps no path at all — its subject is
// the unit it pinned (`modelframe.rs`'s `SetUnit` writes `unit`/`unit_guid` and nils
// `path`) — so gating on `path` alone left every unit pane's handler permanently
// inert. Not cosmetic: unitscan's alert model is made visible *only* from inside its
// own `OnUpdateModel` (`unitscan.lua:273-284` counts ten paints as a settle delay
// after `reset()` has set alpha 0), so the alert drew a correctly placed, correctly
// sized, fully transparent model and the horn played over an empty card. A unit token
// that names nothing still fires nothing, which is the `≠ 0` half of the gate. On any
// other kind the script is inert.
// sized, fully transparent model and the horn played over an empty card. A pane with
// no subject still fires nothing, which is the `≠ 0` half of the gate. On any other
// kind the script is inert.
let frames: Vec<FrameHandle> = model
.on_update_model_frames
.iter()
@@ -421,10 +421,7 @@ impl UiScript {
f.effective_visible
&& match &f.kind_state {
crate::widget::KindState::Model(m) => {
m.path.is_some()
|| m.unit
.as_deref()
.is_some_and(|t| model.unit(t).is_some())
m.path.is_some() || model.unit_pane_subject(m).is_some()
}
_ => false,
}
crates/benilla-ui/src/widget/kinds/mod.rs
+18
View File
@@ -264,6 +264,21 @@ pub struct ModelState {
pub path: Option<String>,
/// `PlayerModel:SetUnit`'s unit (`0x505d70`); it and [`Self::path`] each clear the other.
pub unit: Option<String>,
/// The guid [`Self::unit`] named **at the call**, the pane's subject from then on — not the
/// token's current meaning.
///
/// `SetUnit` and `RefreshUnit` run one worker (`0x505b50`) that resolves the token and
/// rebuilds the pane's model instance there and then; nothing re-resolves it per paint. That
/// is why FrameXML re-issues `SetUnit` on `UNIT_MODEL_CHANGED` and from
/// `PaperDollFrame_OnShow`, and why pfUI can spot a portrait change with no event at all:
/// it pins a scratch pane to the current token and compares `GetModel` against the shown one
/// (`api/unitframes.lua:2069`), which only tells it anything if a pane holds what it was
/// given. unitscan is the live consumer — it `SetUnit`s the rare it found, and the target
/// restored one line later must not take the card.
///
/// `None` while the token named nothing yet, and then the token is re-read per paint: our
/// subject streams in asynchronously, so a pane can be seated before it arrives.
pub unit_guid: Option<u64>,
/// The creature skin variations for a pane showing a display id, by M2 skin slot; empty for
/// an ordinary `SetModel` file, whose batches name their own textures. Cleared by
/// [`Self::set_file`] and written after it, so a plain file never inherits the last
@@ -375,6 +390,7 @@ impl Default for ModelState {
Self {
path: None,
unit: None,
unit_guid: None,
skins: Default::default(),
sequence: 0,
clock_ms: 0,
@@ -494,6 +510,7 @@ impl ModelState {
pub fn set_file(&mut self, path: String, facts: Option<&ModelFileFacts>) {
self.path = Some(path);
self.unit = None;
self.unit_guid = None;
self.skins = Default::default();
self.icon = None;
self.armed = None;
@@ -507,6 +524,7 @@ impl ModelState {
pub fn clear_file(&mut self) {
self.path = None;
self.unit = None;
self.unit_guid = None;
self.skins = Default::default();
self.icon = None;
self.armed = None;