A tabard or taxi close in its open's frame now runs after the open (#2737)

A window's close that came in the same frame as its open still did part of its work ahead of the open's queued event. The tabard close ended the preview at the handler (`0x4f58ce` → `0x5e0830`). The open's handler then started it again, and the body kept wearing the preview tabard with no designer open. The taxi close held behind nothing, so `TAXIMAP_CLOSED` (`0x4dc15f`) fired before `TAXIMAP_OPENED` and the map stayed open with no flight master.

Each close core now closes its session at once, as the trade window's does. Its interface half queues on the window's lane behind the open it follows: the preview's end and `CLOSE_TABARD_FRAME` for the tabard, and `TAXIMAP_CLOSED` for the taxi. Each has a one-frame open-then-close test that fails on main.

Fixes #2730
Fixes #2731

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Sam Sung
2026-10-28 06:27:51 +07:00
committed by GitHub
parent c1507a281c
commit 4cbd96a34b
6 changed files with 181 additions and 41 deletions
@@ -130,3 +130,46 @@ fn the_open_and_the_reply_fire_on_the_lane_at_their_handlers() {
["UI_ERROR_MESSAGE:not master", "TABARD_SAVE_PENDING:nil"]
);
}
/// A close in the open's own frame runs after the open: the open's listener starts the preview
/// (`TabardFrame.lua`, `InitializeTabardColors`) and the close core's undo (`0x4f58ce` →
/// `0x5e0830`) comes after it, ahead of `CLOSE_TABARD_FRAME` (`0x4f58f8`), so no preview is left
/// on the body with no designer open.
#[test]
fn a_close_after_the_open_ends_the_preview_the_open_started() {
let mut app = client();
eval::<()>(
&mut app,
false,
r#"
Order = {}
local g = CreateFrame("Frame")
g:RegisterEvent("OPEN_TABARD_FRAME")
g:RegisterEvent("CLOSE_TABARD_FRAME")
g:SetScript("OnEvent", function()
table.insert(Order, event)
if event == "OPEN_TABARD_FRAME" then TM:InitializeTabardColors() end
end)
"#,
);
let preview = |app: &App| app.world().non_send_resource::<UiScript>().tabard_design();
crate::net::handlers::dispatch(
app.world_mut(),
vec![SessionEvent::TabardVendorActivate(VENDOR)],
);
app.world_mut().flush();
// Another window's open takes the latch in the same frame, which closes the designer
// (`0x493310`).
crate::ui_session::register(app.world_mut(), 0xF130_0000_0000_0043);
app.world_mut().flush();
assert_eq!(app.world().resource::<TabardOpen>().npc(), None, "closed");
assert_eq!(preview(&app), None, "nothing has fired yet");
crate::lanes::fire_in_order(app.world_mut());
let order: Vec<String> = eval(&mut app, false, "return Order");
assert_eq!(order, ["OPEN_TABARD_FRAME", "CLOSE_TABARD_FRAME"]);
assert_eq!(
preview(&app),
None,
"the open's preview ended with the close"
);
}
@@ -244,3 +244,68 @@ fn the_open_is_stamped_at_its_handler() {
.open
.is_none());
}
/// A map and its close in one frame signal in packet order: the activate's `OK` (`0x5ed203`) or a
/// map from another flight master (`0x4db73a`) runs the close core (`0x4dc0f0`) after the
/// `TAXIMAP_OPENED` of the map it closes, so a listener that shows the frame on the open sees it
/// hidden by the close, and no map is left shown with no flight master.
#[test]
fn a_close_after_the_open_fires_after_it() {
let data = benilla_formats::wow_data_or_skip!();
let mut c = client(&data);
c.eval::<()>(
false,
r#"
Shown = false
local g = CreateFrame("Frame")
g:RegisterEvent("TAXIMAP_OPENED")
g:RegisterEvent("TAXIMAP_CLOSED")
g:SetScript("OnEvent", function() Shown = (event == "TAXIMAP_OPENED") end)
"#,
);
c.frame(vec![
vec![shown()],
vec![SessionEvent::ActivateTaxiReply {
code: benilla_protocol::messages::taxi_reply::OK,
}],
]);
// The map holds the latch, so the release re-enters the core: two closes (`0x4dc105`).
assert_eq!(
c.seen(),
[
"TAXIMAP_OPENED:nil",
"TAXIMAP_CLOSED:nil",
"TAXIMAP_CLOSED:nil"
]
);
assert!(!c.eval::<bool>(false, "return Shown"), "the map is hidden");
let state = c.app.world().resource::<crate::ui_taxi::TaxiState>();
assert!(state.open.is_none(), "no cell");
// Another flight master's map closes the first through the core (`0x4db73a`) and opens its
// own, in order; the drain runs with no frame, which would have the leash watch close a
// master the world does not hold.
c.handle(vec![
shown(),
SessionEvent::TaxiNodesShown {
flightmaster: MASTER + 1,
nearest_node: 2,
known_mask: KNOWN,
},
]);
c.app.world_mut().flush();
crate::lanes::fire_in_order(c.app.world_mut());
assert_eq!(
c.seen(),
[
"TAXIMAP_OPENED:nil",
"TAXIMAP_CLOSED:nil",
"TAXIMAP_CLOSED:nil",
"TAXIMAP_OPENED:nil"
]
);
assert!(
c.eval::<bool>(false, "return Shown"),
"the second map is up"
);
}
+32 -19
View File
@@ -23,9 +23,9 @@ pub(crate) enum TabardEvent {
SavePending,
/// A save reply's message line, by its `GlobalStrings` key.
Line(&'static str),
/// What a close core run outside a call signalled while this lane held items, parked in the
/// VM of the session under the key ([`crate::script_calls::park_since`]).
Parked { session: u64, key: u64 },
/// The close core's VM half: the preview's undo (`0x4f58ce` → `0x5e0830`), then
/// `CLOSE_TABARD_FRAME` (`0x4f58f8`).
Closed,
}
/// The tabard lane's queue.
@@ -114,23 +114,36 @@ fn fire(script: &mut UiScript, sink: &mut MessageSink, event: TabardEvent) {
crate::ui_action::show_now(script, sink, "ui_tabard", line);
}
}
TabardEvent::Parked { session, key } => {
if script.session() == session {
script.run_parked(key);
}
}
TabardEvent::Closed => close_vm(script),
}
}
/// Run the close `core` behind the lane ([`crate::lanes::behind`]), so a close never fires ahead
/// of the open it closes.
pub(super) fn behind_lane(world: &mut World, core: impl FnOnce(&mut World)) {
let head = world
.get_resource::<TabardLane>()
.and_then(TabardLane::head);
crate::lanes::behind(world, head, core, |world, session, key| {
world
.resource_mut::<TabardLane>()
.push(TabardEvent::Parked { session, key });
});
/// The close core's VM half: the preview ends, then the frame's close is signalled, at the
/// handler or at its place on the lane.
fn close_vm(script: &mut UiScript) {
script.end_tabard_preview();
script.fire_event("CLOSE_TABARD_FRAME", vec![]);
}
/// Whether a close starting now waits behind the lane ([`crate::lanes::waits_behind`]): the open it
/// closes, or any earlier signal, has not fired yet.
pub(super) fn close_waits(world: &World) -> bool {
crate::lanes::waits_behind(
world
.get_resource::<TabardLane>()
.and_then(TabardLane::head),
)
}
/// The close core's VM half: queued on the lane when the close `waits` ([`close_waits`]), so the
/// preview the open starts ends after it and not before; else run now, its signal fired with the
/// host lent.
pub(super) fn close_step(world: &mut World, waits: bool) {
if waits {
if let Some(mut lane) = world.get_resource_mut::<TabardLane>() {
lane.push(TabardEvent::Closed);
return;
}
}
crate::ui_session::fire(world, close_vm);
}
+12 -18
View File
@@ -94,25 +94,19 @@ pub(crate) fn open_tabard(world: &mut World, vendor: u64) {
/// The close core `0x4f58a0` as `0x493310` and the leave-world sweep enter it: with a vendor
/// stored, the preview ends (`0x4f58ce` → `0x5e0830`), the latch is released (`0x4f58da`), the cell
/// cleared, then `CLOSE_TABARD_FRAME` (`0x4f58f8`), held behind what the lane still holds
/// ([`lane::behind_lane`]). `CloseTabardCreation` runs it inside its call
/// cleared, then `CLOSE_TABARD_FRAME` (`0x4f58f8`). The session closes here; the preview's end
/// and the signal wait behind what the lane still holds ([`lane::close_step`]), so the open the
/// close follows has started its preview first. `CloseTabardCreation` runs it inside its call
/// ([`crate::script_calls::ScriptCallHost`]).
pub(crate) fn close_core(world: &mut World) {
if world.resource::<TabardOpen>().vendor.is_none() {
let Some(vendor) = world.resource::<TabardOpen>().vendor else {
return;
}
lane::behind_lane(world, |world| {
let Some(vendor) = world.resource::<TabardOpen>().vendor else {
return;
};
crate::ui_session::release(world, vendor);
world.resource_mut::<TabardOpen>().vendor = None;
crate::ui_session::note_closed(world, NpcWindow::Tabard);
crate::ui_session::fire(world, |script| {
script.end_tabard_preview();
script.fire_event("CLOSE_TABARD_FRAME", vec![]);
});
});
};
let waits = lane::close_waits(world);
crate::ui_session::release(world, vendor);
world.resource_mut::<TabardOpen>().vendor = None;
crate::ui_session::note_closed(world, NpcWindow::Tabard);
lane::close_step(world, waits);
}
/// The design our own body wears while the frame is open.
@@ -541,8 +535,8 @@ mod tests {
.into_iter()
.map(|(_, e)| e)
.collect::<Vec<_>>(),
[TabardEvent::Open, TabardEvent::Open],
"each activate signals its open"
[TabardEvent::Open, TabardEvent::Closed, TabardEvent::Open],
"each activate signals its open, the second after the close it made"
);
}
}
+24
View File
@@ -23,6 +23,9 @@ pub(crate) enum TaxiEvent {
Opened,
/// A message line, by its `GlobalStrings` key.
Line(&'static str),
/// The close core's `TAXIMAP_CLOSED` (`0x4dc15f`), queued by a close that follows the open
/// still on this lane.
Closed,
/// What a core the handler ran signalled, parked in the VM of the session under the key
/// ([`crate::script_calls::park_since`]).
Parked { session: u64, key: u64 },
@@ -127,6 +130,7 @@ fn fire(
crate::ui_action::show_now(script, sink, "ui_taxi", line);
}
}
TaxiEvent::Closed => script.fire_event("TAXIMAP_CLOSED", vec![]),
TaxiEvent::Parked { session, key } => {
if script.session() == session {
script.run_parked(key);
@@ -134,3 +138,23 @@ fn fire(
}
}
}
/// Whether a close starting now waits behind the lane ([`crate::lanes::waits_behind`]): the open it
/// closes, or any earlier signal, has not fired yet.
pub(super) fn close_waits(world: &World) -> bool {
crate::lanes::waits_behind(world.get_resource::<TaxiLane>().and_then(TaxiLane::head))
}
/// The close core's `TAXIMAP_CLOSED`: queued on the lane when the close `waits` ([`close_waits`]),
/// so it fires after the `TAXIMAP_OPENED` it closes; else fired now with the host lent.
pub(super) fn close_signal(world: &mut World, waits: bool) {
if waits {
if let Some(mut lane) = world.get_resource_mut::<TaxiLane>() {
lane.push(TaxiEvent::Closed);
return;
}
}
crate::ui_session::fire(world, |script| {
script.fire_event("TAXIMAP_CLOSED", vec![]);
});
}
+5 -4
View File
@@ -82,11 +82,14 @@ impl LatchedWindow for TaxiState {
/// (`0x4dc15f`). The slots stay, their fares 0 with no flight master (`0x4dc423`). An activate's
/// `OK` (`0x5ed203`), a map from another flight master
/// (`0x4db73a`) and the leave-world sweep's leg ([`leave_world`]) call it as it is;
/// `CloseTaxiMap` runs it inside its call ([`TaxiSteps`]).
/// `CloseTaxiMap` runs it inside its call ([`TaxiSteps`]). The session closes here; the signal
/// waits behind what the lane still holds ([`lane::close_signal`]), so it never fires ahead of the
/// open it closes.
pub(crate) fn close_core(world: &mut World) {
let Some(flightmaster) = world.resource::<TaxiState>().cell() else {
return;
};
let waits = lane::close_waits(world);
crate::ui_session::release(world, flightmaster);
world.resource_mut::<TaxiState>().clear();
if let Some(mut cache) = world.get_resource_mut::<TaxiRouteCache>() {
@@ -95,9 +98,7 @@ pub(crate) fn close_core(world: &mut World) {
}
}
crate::ui_session::note_closed(world, NpcWindow::Taxi);
crate::ui_session::fire(world, |script| {
script.fire_event("TAXIMAP_CLOSED", vec![]);
});
lane::close_signal(world, waits);
}
/// The leave-world sweep's leg `0x4db700`: the free `0x4db630` (the map art at `0x4db6d0`, the