diff --git a/crates/benilla-app/src/ui_script/lifecycle.rs b/crates/benilla-app/src/ui_script/lifecycle.rs index a8c40d68..bd42798f 100644 --- a/crates/benilla-app/src/ui_script/lifecycle.rs +++ b/crates/benilla-app/src/ui_script/lifecycle.rs @@ -558,6 +558,11 @@ pub(crate) fn load_ingame_ui_on_world_entry(world: &mut World) { // The whole `AreaTable` name column, beside it and for the same reason: pfQuest reads it at // file scope for `C_Map.GetAreas` and caches the answer for the session. crate::ui_world_map::seed_area_names(world, &mut script); + // The flight-master catalog, which reads the rects the map catalog above just built — so it + // has to follow that call, not just share the edge. Its own deadline is tighter than file + // scope: pfQuest replays its saved map-tracking selections on `PLAYER_ENTERING_WORLD` and + // unregisters every event, so a push after that is a push nothing reads. + crate::ui_taxi::seed_taxi_map_catalog(world, &mut script); // Screen size and measurer before the first ``: this VM is loaded before `tick_script` // seats either, and the load measures text (the stock tab template's `OnLoad` sizes from its // text width, `UIPanelTemplates.xml:370-373`), which answers 0 with no measurer. diff --git a/crates/benilla-app/src/ui_taxi/map_nodes.rs b/crates/benilla-app/src/ui_taxi/map_nodes.rs new file mode 100644 index 00000000..63f19fb8 --- /dev/null +++ b/crates/benilla-app/src/ui_taxi/map_nodes.rs @@ -0,0 +1,364 @@ +//! The whole `TaxiNodes.dbc` table, resolved to zones and map percentages, for ClassicAPI's +//! `C_TaxiMap.GetTaxiNodesForMap` (`benilla-ui`'s `compat/classic_api/taxi_map.rs`, which carries +//! the contract and why each field is the shape it is). +//! +//! Separate from [`super::routing`], which answers for the *open* taxi window: that list is the +//! player's known mask, typed and priced against where they stand, and it exists only while a +//! flight master has the frame up. This one is file data — every node, known or not — and it is +//! what an addon drawing flight masters on a map needs. +//! +//! **Seeded at world entry, before the interface loads**, and that timing is not incidental: the +//! one consumer replays its saved tracking selections once on `PLAYER_ENTERING_WORLD` and then +//! calls `this:UnregisterAllEvents()` (`pfQuest/database.lua:131-151`). A catalog pushed a frame +//! later is a catalog pushed after the only read, and the flight-master layer then stays empty for +//! the whole session with the option still ticked — which is exactly the shape of the bug this +//! answers. + +use benilla_formats::{load_taxi_nodes, TaxiNode, TaxiNodes}; +use benilla_ui::script::{TaxiMapNode, UiScript}; +use bevy::prelude::*; + +use crate::ui_world_map::WorldMapUiData; + +/// ClassicAPI's `Enum.FlightPathFaction`, whose three names are in the bundled `ClassicAPI.dll`'s +/// string pool (`Neutral`, `Horde`, `Alliance`) in this order. +const FLIGHT_NEUTRAL: u8 = 0; +const FLIGHT_HORDE: u8 = 1; +const FLIGHT_ALLIANCE: u8 = 2; + +/// Whom a node serves, from its two `MountCreatureID` columns; `None` is "not a flight master". +/// +/// That pair *is* the faction gate in the reference: the taxi window asks for the mount of the +/// player's own half (`0x4a2650`), so a node with only the Horde column filled is a Horde node and +/// a node with both serves either side, which is neutral. +/// +/// A row with **neither** filled is not reported at all, because in the reference's own terms it +/// is not a flight master: nobody can be given a mount there, and the real table uses those rows +/// for boat and zeppelin endpoints — on this install they are exactly the eight `Transport, …`, +/// `… Ferry` and `Generic, World target for Zeppelin Paths` rows. The consumer labels every row it +/// receives "Flight Master" (`SearchFlightNodes` sets `meta.spawntype` unconditionally), so +/// passing them on would put flight-master pins on the Menethil docks. The risk runs the other +/// way too and was weighed: a *custom* OctoWoW flight point with unfilled mount columns would be +/// dropped here — but such a row cannot work on the reference client either, since the flight +/// itself needs the mount, so a server shipping one has a broken node, not a hidden one. The seed +/// logs the count either way. +fn faction_of(node: &TaxiNode) -> Option { + match (node.mount_horde != 0, node.mount_alliance != 0) { + (true, false) => Some(FLIGHT_HORDE), + (false, true) => Some(FLIGHT_ALLIANCE), + (true, true) => Some(FLIGHT_NEUTRAL), + (false, false) => None, + } +} + +/// Whether a zone called `name` is the zone a `TaxiNodes.dbc` name part claims. +/// +/// A prefix at a word boundary, because the file abbreviates: "Stranglethorn" for "Stranglethorn +/// Vale", "Hillsbrad" for "Hillsbrad Foothills", "Stormwind" for "Stormwind City". The boundary +/// is what keeps it from being a loose `starts_with` — one part must not claim a zone whose name +/// merely begins with the same letters. +fn zone_name_claimed_by(name: &str, part: &str) -> bool { + name.len() >= part.len() + && name[..part.len()].eq_ignore_ascii_case(part) + && name[part.len()..] + .chars() + .next() + .is_none_or(char::is_whitespace) +} + +/// The zone a node belongs to, and where it sits on that zone's map. +/// +/// Geometry proposes and the **name decides**, in that order, and the order is the point. These +/// rects are axis-aligned boxes around irregular zones, so a node near a border is often inside +/// two or three of them and no purely geometric rule gets every row right: "smallest box wins" +/// misfiled a dozen of this install's ~70 rows (Crossroads under Mulgore, Aerie Peak under +/// Alterac Mountains), and "most central box wins" — much better, and what +/// [`WorldMapUiData::zones_at`] sorts by — still puts Talonbranch Glade in Darkshore and Nijel's +/// Point in Stonetalon, each losing to a neighbour by about one percent of inset. +/// +/// But the file names its own answer: `TaxiNodes.dbc` calls these rows "`, `" +/// ("Crossroads, The Barrens") and a city's row leads with the city, which is itself a zone +/// ("Ironforge, Dun Morogh"). So each comma-separated part is tried in order against the +/// candidates' `AreaTable` names, and the first that matches one wins — which picks Ironforge over +/// Dun Morogh for the city rows *and* Felwood over Darkshore for the border ones, from the same +/// rule. The match allows the file's abbreviations (the zone name may continue past the part: +/// "Stranglethorn" names "Stranglethorn Vale", "Stormwind" names "Stormwind City") but only on a +/// word boundary. +/// +/// A name that matches no candidate falls back to the most central box: that is every `Transport, +/// …` and `Generic, …` row, and any custom node whose name does not follow the convention. +/// Geometry stays the final authority on *placement* either way — a zone is only ever considered +/// if the node is inside its rect, because `mapX`/`mapY` are percentages of that rect and a point +/// outside it has no meaningful ones. +fn resolve( + node: &TaxiNode, + data: &WorldMapUiData, + areas: &benilla_formats::AreaTableCatalog, +) -> Option { + let faction = faction_of(node)?; + let candidates = data.zones_at(node.map_id, node.pos[0], node.pos[1]); + let named = node.name.split(", ").find_map(|part| { + candidates.iter().find(|(area, _)| { + areas + .name(*area) + .is_some_and(|name| zone_name_claimed_by(name, part)) + }) + }); + let placed = named.or(candidates.first()); + Some(TaxiMapNode { + node_id: node.id, + name: node.name.clone(), + faction, + area_id: placed.map(|(area, _)| *area), + // Percent, not UV, because that is the unit the consumer stores and divides by 100 again + // when it places a pin (`pfMap:AddNode` → `UpdateNodes`). + map_percent: placed.map(|(_, (u, v))| (u * 100.0, v * 100.0)), + }) +} + +/// Build the catalog in `TaxiNodes.dbc` row order, lowest id first. +/// +/// File order rather than the resolver's walk order, so the array index a caller keys by is stable +/// across sessions: [`TaxiNodes::rows`] iterates a `HashMap`. +pub(crate) fn taxi_map_catalog( + nodes: &TaxiNodes, + data: &WorldMapUiData, + areas: &benilla_formats::AreaTableCatalog, +) -> Vec { + let mut rows: Vec<&TaxiNode> = nodes.rows().collect(); + rows.sort_by_key(|n| n.id); + rows.into_iter() + .filter_map(|n| resolve(n, data, areas)) + .collect() +} + +/// Push the catalog into a freshly loaded VM, from +/// [`crate::ui_script::load_ingame_ui_on_world_entry`]. +pub(crate) fn seed_taxi_map_catalog(world: &mut World, script: &mut UiScript) { + let (Some(data), Some(areas)) = ( + world.get_resource::(), + world.get_resource::(), + ) else { + // No map catalog means no rects to resolve against, and a node list with no zone is a + // list the caller's own gate drops row by row — nothing to push. + return; + }; + // The window's own catalog when it has already loaded, else one read of the file: this edge + // runs before `load_taxi_catalogs` has necessarily had a frame, and the read is a single DBC. + let loaded; + let nodes = match world.get_resource::() { + Some(catalogs) => &catalogs.nodes, + None => { + let Some(assets) = world.get_resource::() else { + return; + }; + let mut chain = { + use benilla_assets::LockRecover; + assets.chain.lock_recover() + }; + match load_taxi_nodes(&mut chain) { + Ok(n) => { + loaded = n; + &loaded + } + Err(e) => { + warn!("taxi map: TaxiNodes.dbc unavailable ({e:#}) — no flight-master catalog"); + return; + } + } + } + }; + let catalog = taxi_map_catalog(nodes, data, &areas.0); + let placed = catalog.iter().filter(|n| n.area_id.is_some()).count(); + info!( + "taxi map: catalog — {} of {} row(s) serve a faction, {placed} placed on a zone map, \ + {} on no map", + catalog.len(), + nodes.len(), + catalog.len() - placed, + ); + script.set_taxi_map_catalog(catalog); +} + +#[cfg(test)] +mod tests { + use super::*; + + fn row(id: u32, map_id: u32, pos: [f32; 3], horde: u32, alliance: u32) -> TaxiNode { + TaxiNode { + id, + map_id, + pos, + name: format!("node {id}"), + mount_horde: horde, + mount_alliance: alliance, + } + } + + /// The two mount columns are the faction gate, and a row with neither is not a flight master + /// at all — see [`faction_of`]. + #[test] + fn the_mount_columns_decide_which_faction_a_node_serves() { + for (horde, alliance, want) in [ + (295u32, 0u32, Some(FLIGHT_HORDE)), + (0, 479, Some(FLIGHT_ALLIANCE)), + (295, 479, Some(FLIGHT_NEUTRAL)), + (0, 0, None), + ] { + assert_eq!( + faction_of(&row(1, 0, [0.0; 3], horde, alliance)), + want, + "mounts ({horde}, {alliance})" + ); + } + } + + /// The real table, held to the oracle the file itself carries: `TaxiNodes.dbc` names its rows + /// "`, `", so for every row whose name ends in a zone this catalog maps, the + /// resolved zone must be that zone — or the zone the row *leads* with, which is how a city row + /// reads ("Ironforge, Dun Morogh" belongs on the Ironforge map). + /// + /// This is the test that rejected two earlier purely geometric rules; the per-row message + /// names the node, so a failure says which one moved. + #[test] + fn every_named_row_lands_on_the_zone_its_own_name_claims() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let areas = benilla_formats::load_area_table_catalog(&mut chain).expect("AreaTable"); + let maps = benilla_assets::MapCatalogRes( + benilla_formats::load_map_catalog(&mut chain).expect("Map"), + ); + let (_views, ui_data) = crate::ui_world_map::build_catalog(&mut chain, &areas, &maps) + .expect("the real catalog"); + let nodes = benilla_formats::load_taxi_nodes(&mut chain).expect("load TaxiNodes"); + let catalog = taxi_map_catalog(&nodes, &ui_data, &areas); + + assert!( + catalog.len() > 50, + "1.12 ships ~70 taxi nodes that serve a faction, got {}", + catalog.len() + ); + // Every row the name rule could place, and the ones it could not, named exactly. + let mut checked = 0; + let mut by_geometry: Vec<&str> = Vec::new(); + let mut unplaced: Vec<&str> = Vec::new(); + for n in &catalog { + let Some(got) = n.area_id else { + unplaced.push(n.name.as_str()); + continue; + }; + let got_name = areas.name(got).unwrap_or("?"); + if n.name + .split(", ") + .any(|part| zone_name_claimed_by(got_name, part)) + { + checked += 1; + } else { + by_geometry.push(n.name.as_str()); + } + } + // Exactly four rows name no zone, and naming them is the point: three are **this realm's + // own** additions (OctoWoW's Northshire Abbey flight point and the two Darkshore canoe + // routes), which is the shape a custom node takes — a place name with no ", " tail — + // and the fourth is 1.12's dummy world-target row. A new entry here is a new custom node + // that the name rule cannot speak for, placed by geometry alone; worth knowing, not a + // failure in itself. + assert_eq!( + by_geometry, + [ + "Northshire Abbey", + "Generic, World target", + "Canoe: Darkshore - Icepoint Rock", + "Canoe: Icepoint Rock - Darkshore", + ], + "only rows whose name names no zone may fall back to geometry" + ); + assert!( + unplaced.is_empty(), + "every row on a map this catalog knows must place: {unplaced:?}" + ); + assert!( + checked > 60, + "the name rule must place nearly the whole table, placed {checked}" + ); + // Both halves of a placement, together: the consumer's gate reads all three fields at once. + for n in &catalog { + assert_eq!( + n.area_id.is_some(), + n.map_percent.is_some(), + "{} has half an answer", + n.name + ); + if let Some((x, y)) = n.map_percent { + assert!( + (0.0..=100.0).contains(&x) && (0.0..=100.0).contains(&y), + "{} is at ({x}, {y}), outside its own zone map", + n.name + ); + } + } + } + + /// The four capital cities and two border nodes by hand, because they are the rows the + /// resolution rule exists for: a city row must land on the *city* map, and a node a + /// neighbouring zone's box also contains must land where its name says. + #[test] + fn the_rows_the_rule_exists_for() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let areas = benilla_formats::load_area_table_catalog(&mut chain).expect("AreaTable"); + let maps = benilla_assets::MapCatalogRes( + benilla_formats::load_map_catalog(&mut chain).expect("Map"), + ); + let (_views, ui_data) = crate::ui_world_map::build_catalog(&mut chain, &areas, &maps) + .expect("the real catalog"); + let nodes = benilla_formats::load_taxi_nodes(&mut chain).expect("load TaxiNodes"); + let catalog = taxi_map_catalog(&nodes, &ui_data, &areas); + let find = |needle: &str| { + catalog + .iter() + .find(|n| n.name.starts_with(needle)) + .unwrap_or_else(|| panic!("no node named {needle}")) + .clone() + }; + for (name, zone) in [ + // Cities: the row leads with a zone of its own, inside its parent zone's box. + ("Stormwind", 1519u32), + ("Ironforge", 1537), + ("Orgrimmar", 1637), + ("Thunder Bluff", 1638), + ("Undercity", 1497), + // Borders: a neighbour's box contains these more centrally than their own zone's. + ("Talonbranch Glade", 361), + ("Nijel's Point", 405), + ("Crossroads", 17), + ("Aerie Peak", 47), + // A battleground: its own map, on no continent, which the candidate walk reaches + // through the instance-map list rather than a continent's zones. + ("Dun Baldar", 2597), + // This realm's own nodes, which name no zone and so are placed by geometry alone. + ("Northshire Abbey", 12), + ("Canoe: Darkshore - Icepoint Rock", 148), + ] { + let node = find(name); + assert_eq!( + node.area_id, + Some(zone), + "{} landed in {:?}", + node.name, + node.area_id.and_then(|a| areas.name(a)), + ); + } + // The boat and zeppelin endpoints are not flight masters and must not be in the list. + for dropped in [ + "Transport,", + "Southshore Ferry", + "Generic, World target for", + ] { + assert!( + !catalog.iter().any(|n| n.name.starts_with(dropped)), + "{dropped} has no mount for either faction and is not a flight master" + ); + } + } +} diff --git a/crates/benilla-app/src/ui_taxi/mod.rs b/crates/benilla-app/src/ui_taxi/mod.rs index 8903a84a..cf87c299 100644 --- a/crates/benilla-app/src/ui_taxi/mod.rs +++ b/crates/benilla-app/src/ui_taxi/mod.rs @@ -13,6 +13,8 @@ use crate::player::UNIT_FLAG_TAXI_FLIGHT; use crate::ui_script::{UiFeed, UiInput}; use crate::ui_session::{close_npc_session_out_of_range, NpcSession}; +mod map_nodes; +pub(crate) use map_nodes::{seed_taxi_map_catalog, taxi_map_catalog}; mod routing; use routing::{build_nodes, load_taxi_catalogs, taxi_error_key, TaxiCatalogs, TaxiRouteCache}; diff --git a/crates/benilla-app/src/ui_taxi/routing.rs b/crates/benilla-app/src/ui_taxi/routing.rs index ec3f463b..6dd97a1c 100644 --- a/crates/benilla-app/src/ui_taxi/routing.rs +++ b/crates/benilla-app/src/ui_taxi/routing.rs @@ -21,7 +21,9 @@ use super::TaxiOpen; /// (each continent's taxi-map rect, its `taxi_min`/`taxi_max`). #[derive(Resource)] pub(crate) struct TaxiCatalogs { - nodes: TaxiNodes, + /// Read by [`super::map_nodes`] too, which pushes the same file to the VM for `C_TaxiMap`: + /// one load, so the window and the addon catalog can never disagree about the table. + pub(super) nodes: TaxiNodes, pub(super) paths: TaxiPaths, continents: WorldMapContinentCatalog, } diff --git a/crates/benilla-app/src/ui_world_map.rs b/crates/benilla-app/src/ui_world_map.rs index 662bdce9..31451c57 100644 --- a/crates/benilla-app/src/ui_world_map.rs +++ b/crates/benilla-app/src/ui_world_map.rs @@ -524,6 +524,60 @@ fn world_enter_selection( (!synced && top_zone.is_some()).then_some(player) } +impl WorldMapUiData { + /// Every zone whose `WorldMapArea` rect contains a world point on `map_id`, and the point's + /// UV inside each — the candidates for "which zone map would draw this, and where on it", + /// most central first. + /// + /// Containment is [`map_proj::zone_uv`]'s own verdict rather than a fresh rect compare, so a + /// point this says is in a zone is a point the map window would place inside that zone's art: + /// that function zeroes both axes for anything off the rect, which is the same `(0, 0)` the + /// reference's `GetPlayerMapPosition` reports as "not on this map". Its edge cases come along + /// too — a point exactly on the rect's east or south edge projects to 0 and so reads as + /// off-map — and those are the right ones to inherit, because the alternative is calling a + /// point part of a zone the map itself would not draw it in. + /// + /// **Ordered by how far inside its rect the point sits**, `min(u, 1-u, v, 1-v)` descending, + /// because these are axis-aligned boxes around irregular zones and neighbours overlap heavily + /// at their corners: the *smallest* containing box is routinely the wrong answer (measured on + /// `TaxiNodes.dbc`, it filed Crossroads under Mulgore, at 90%/5% of Mulgore's box). Centrality + /// also gets the nesting case right, where a city sits wholly inside its parent zone's box but + /// is central in its own. + /// + /// It is a *list* because centrality is good and not perfect — two of this install's ~70 taxi + /// nodes lose to a neighbour by about one percent of inset — so a caller holding another + /// source for the same question can take the candidate that source names instead of the head. + /// `ui_taxi::map_nodes` does exactly that with the zone name `TaxiNodes.dbc` carries. 1.12 has + /// no verb that asks this question, so none of this is reference behaviour; it is ours. + pub(crate) fn zones_at(&self, map_id: u32, wx: f32, wy: f32) -> Vec<(u32, (f32, f32))> { + // How far inside the rect the point is, 0 at an edge and 0.5 dead centre. + let inset = |(u, v): (f32, f32)| (u.min(1.0 - u)).min(v.min(1.0 - v)); + // The instance maps as well as the continents' zone lists, because a point can be on one + // and on no continent at all: the three battlegrounds are their own maps, and a caller + // asking "which zone map draws this" wants Alterac Valley for a point inside it — the + // reference's own projector takes the same two legs (`0x4a73e0` beside `0x4a7360`). + let mut hits: Vec<(u32, (f32, f32))> = self + .continents + .iter() + .filter(|cont| cont.map_id == map_id) + .flat_map(|cont| cont.zones.iter()) + .map(|zone| (zone.area_id, zone.rect)) + .chain( + self.direct + .iter() + .filter(|d| d.map_id == map_id) + .map(|d| (d.view.area_id, d.rect)), + ) + .filter_map(|(area_id, rect)| { + let uv = map_proj::zone_uv(rect, wx, wy); + (uv != (0.0, 0.0)).then_some((area_id, uv)) + }) + .collect(); + hits.sort_by(|a, b| inset(b.1).total_cmp(&inset(a.1))); + hits + } +} + /// A position's UV on the displayed map (`0x4a7360`), `None` off it, which Lua sees as the /// `(0, 0)` hide: the world sheet projects through the position's own continent constants, a /// continent or zone through its rect, an instance map through its row's rect (`0x4a73e0`). The @@ -567,9 +621,10 @@ struct FeedMemos { landmarks: Option, /// The world-enter sync has run: the reference's `old == 0` gate. map_synced: bool, - /// The last `(map, leaf area, top zone, selection)` logged, so the zone line prints on change - /// rather than every frame. - zone_logged: Option<(u32, u32, u32, u32, u32)>, + /// The last `(map, leaf area, top zone, resolved pair, selected pair)` logged, so the zone + /// line prints on change rather than every frame — the selection is in the key because a + /// selection that stops following the player is the thing the line exists to show. + zone_logged: Option<(u32, u32, u32, u32, u32, u32, u32)>, } /// The `AreaPOI.dbc` half of the landmark list (`0x4a6819`'s walk), in file order: every row the @@ -653,35 +708,51 @@ fn feed_world_map( let player_sel = resolve_player_selection(&data, map.0, top_zone); // One line per change, because the whole chain is otherwise invisible: the leaf id comes off - // the terrain chunk under the player, the zone is its one-hop rollup, and the map selection is - // an index into the continent's zone list. A player reporting "the map said I was somewhere - // else" can only be answered from these four numbers at that moment — on a server with custom - // zones, any of them can be the one that is wrong. + // the terrain chunk under the player, the zone is its one-hop rollup, the *resolved* pair is + // where `SetMapToCurrentZone` would land, and the *selected* pair is where the map actually + // is. A player reporting "the map said I was somewhere else" can only be answered from those + // numbers at that moment — on a server with custom zones, any of them can be the wrong one. + // + // The last two are printed separately because they are not the same fact, and conflating them + // cost a whole diagnosis: this line used to read "world map shows" while printing the + // *resolved* pair, so a selection sitting a zone behind — which is what leaves every + // map-derived overlay stale, `GetPlayerMapPosition` included, since that projects onto the + // **displayed** map — looked from the log exactly like a selection that had followed. { let leaf = world.area().unwrap_or(0); let (ci, zi) = player_sel.zone.unwrap_or((0, 0)); - let key = (map.0, leaf, top_zone.unwrap_or(0), ci, zi); + let (sc, sz, sdirect) = script.world_map_selection(); + let key = (map.0, leaf, top_zone.unwrap_or(0), ci, zi, sc, sz); let memo = memo.get(&script); if memo.zone_logged != Some(key) { memo.zone_logged = Some(key); let name = |id: u32| areas.0.name(id).unwrap_or("?").to_string(); - let picked = data - .continents - .get(ci.wrapping_sub(1) as usize) - .and_then(|c| { - if zi == 0 { - // The continent sheet itself, named by its map. - Some(format!("the continent map of map {}", c.map_id)) - } else { - c.zones - .get(zi.wrapping_sub(1) as usize) - .map(|z| name(z.area_id)) - } - }) - .or_else(|| player_sel.direct.map(|id| format!("instance map row {id}"))) - .unwrap_or_else(|| "the world view".to_string()); + // What a `(continent, zone)` pair names, for either of the two pairs. + let label = |c: u32, z: u32, direct: Option| { + data.continents + .get(c.wrapping_sub(1) as usize) + .and_then(|cont| { + if z == 0 { + // The continent sheet itself, named by its map. + Some(format!("the continent map of map {}", cont.map_id)) + } else { + cont.zones + .get(z.wrapping_sub(1) as usize) + .map(|zone| name(zone.area_id)) + } + }) + .or_else(|| direct.map(|id| format!("instance map row {id}"))) + .unwrap_or_else(|| "the world view".to_string()) + }; + let resolved = label(ci, zi, player_sel.direct); + let selected = label(sc, sz, sdirect); + let follows = if (ci, zi, player_sel.direct) == (sc, sz, sdirect) { + "" + } else { + " ← STALE: the map is not on the player's zone" + }; info!( - "zone: map {} · chunk area {leaf} \"{}\" · zone {} \"{}\" · world map shows \"{picked}\" (continent {ci}, zone {zi})", + "zone: map {} · chunk area {leaf} \"{}\" · zone {} \"{}\" · resolves to \"{resolved}\" (continent {ci}, zone {zi}) · map shows \"{selected}\" (continent {sc}, zone {sz}){follows}", map.0, name(leaf), top_zone.unwrap_or(0), diff --git a/crates/benilla-ui/src/script/compat/classic_api/mod.rs b/crates/benilla-ui/src/script/compat/classic_api/mod.rs index 9e37b6de..de39f42d 100644 --- a/crates/benilla-ui/src/script/compat/classic_api/mod.rs +++ b/crates/benilla-ui/src/script/compat/classic_api/mod.rs @@ -68,6 +68,7 @@ mod state; mod stdlib_backport; mod string_methods; mod table_util; +mod taxi_map; mod timer; mod tooltip; mod totem; @@ -131,6 +132,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set("C_GossipInfo", gossip::install(lua)?)?; g.set("C_MerchantFrame", merchant_frame::install(lua)?)?; g.set("C_Map", map::install(lua)?)?; + taxi_map::install(lua)?; g.set("C_PlayerCache", player_cache::install(lua)?)?; g.set("C_PlayerInfo", player_info::install(lua)?)?; g.set("PlayerUtil", player_util::install(lua)?)?; diff --git a/crates/benilla-ui/src/script/compat/classic_api/taxi_map.rs b/crates/benilla-ui/src/script/compat/classic_api/taxi_map.rs new file mode 100644 index 00000000..b1dfad79 --- /dev/null +++ b/crates/benilla-ui/src/script/compat/classic_api/taxi_map.rs @@ -0,0 +1,195 @@ +//! ClassicAPI's `C_TaxiMap`, the one member an addon can answer from file data. +//! +//! 1.12 tells Lua about flight masters only while you are *standing at one*: `NumTaxiNodes` and +//! friends (`script/taxi.rs`) read the open window's session, which the server fills from your +//! known mask, and there is no verb at all for "where are the flight masters". So an addon that +//! wants to draw them has to ship its own hand-built table — which is exactly what pfQuest did, +//! and what pfQuest-octo has now **deleted** in favour of this call: `pfDB.meta.flight` is an +//! empty stub whose own comment says the data "always comes live from the client (ClassicAPI)" +//! (`db/meta.lua:181`), and `pfDatabase:SearchFlightNodes` opens with +//! +//! ```lua +//! if not (C_TaxiMap and C_TaxiMap.GetTaxiNodesForMap) then return maps end +//! ``` +//! +//! so without this namespace the layer is not broken, it is *silently empty*: the option stays +//! ticked and nothing is ever drawn. That is grom's "can't see flight master location marks, even +//! tho I have them on". +//! +//! **The contract, and how much of it is known.** `C_TaxiMap` and `GetTaxiNodesForMap` are in the +//! bundled `ClassicAPI.dll`'s string pool beside `nodeID`, `faction`, `reachable`, `state` and the +//! `FlightPathFaction`/`FlightPathState` enum names, so those five field names are this build's +//! own. The `areaID`/`mapX`/`mapY` triple is **not** separately visible there — the pool holds one +//! copy of each string and `C_Map.GetAreaTriggerInfo` already uses all three — so for those the +//! authority is the consumer: `SearchFlightNodes` reads `node.areaID`, `node.mapX`, `node.mapY`, +//! `node.nodeID`, `node.faction` and `node.name`, gates on +//! `if show and node.areaID and node.mapX and node.mapY`, and feeds the pair straight into +//! `pfMap:AddNode` as `meta.x`/`meta.y`, which `UpdateNodes` later divides by 100 — so they are +//! **percentages of the zone rect**, the same units that function's every other node carries. +//! +//! Three members of the namespace are **not** built, and the reason is the same for all three: +//! `GetTaxiRoute`, `GetTaxiPaths` and `GetTaxiPathWaypoints` are route data (`TaxiPath.dbc`, +//! `TaxiPathNode.dbc`), which the app loads for the taxi *window*'s own route search +//! (`ui_taxi/routing.rs`) and has never had reason to push into the VM. Nothing in this install's +//! corpus calls them. `GetAllTaxiNodes` is in the pool too and is presumably the same answer +//! keyed differently; with no caller to read against, guessing its shape would be inventing a +//! contract rather than matching one. +//! +//! Also deliberately absent from each node: `reachable` and `state`. Both are *session* facts — +//! whether this node is known and routable from where you stand — and the window that owns them +//! (`model.taxi`) exists only while a flight master has one open, whereas this catalog answers for +//! every node in the file at any time. A node table that carried `reachable = false` for all 60 +//! nodes because no window is open would be a confident wrong answer; absent fields are a +//! truthful "this build does not say". `docs/ADDON_COMPAT.md` carries the row. + +use mlua::Lua; + +use crate::script::Model; + +pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { + let t = lua.create_table()?; + + // GetTaxiNodesForMap([areaID]) → an array of node tables, 1-based, `ipairs`-walkable. + // + // No argument is "every node on every continent", which is how the one caller uses it + // (`pfQuest/database.lua:896`: its comment spells the no-arg case out). An argument filters to + // one zone by its `AreaTable` id — the id space the rest of `C_Map` here already speaks + // (`GetMapWorldSize(areaID)`, `GetMapOverlays(areaID)`), not a modern `uiMapID`, which 1.12 + // has no table for. + // + // The order is `TaxiNodes.dbc` row order, as the app pushed it: a stable order the caller can + // key against, and the file's own. + t.set( + "GetTaxiNodesForMap", + lua.create_function(|lua, area_id: Option| { + let model = lua.app_data_ref::().expect("model app_data"); + let out = lua.create_table()?; + let mut i = 0_i64; + for node in &model.taxi_catalog { + if let Some(want) = area_id { + if node.area_id != Some(want) { + continue; + } + } + let row = lua.create_table()?; + row.set("nodeID", node.node_id)?; + row.set("name", node.name.as_str())?; + row.set("faction", i64::from(node.faction))?; + // Written only when resolved, so the caller's `and node.areaID and node.mapX and + // node.mapY` reads a plain nil for a node on no zone map rather than a sentinel it + // would have to know about. + if let Some(area) = node.area_id { + row.set("areaID", area)?; + } + if let Some((x, y)) = node.map_percent { + row.set("mapX", f64::from(x))?; + row.set("mapY", f64::from(y))?; + } + i += 1; + out.set(i, row)?; + } + Ok(out) + })?, + )?; + + lua.globals().set("C_TaxiMap", t) +} + +#[cfg(test)] +mod tests { + use crate::script::{TaxiMapNode, UiScript}; + + fn node(node_id: u32, name: &str, faction: u8, area: Option) -> TaxiMapNode { + TaxiMapNode { + node_id, + name: name.to_owned(), + faction, + area_id: area, + map_percent: area.map(|_| (33.5, 66.5)), + } + } + + fn seeded() -> UiScript { + let mut s = UiScript::new().unwrap(); + s.set_taxi_map_catalog(vec![ + node(2, "Stormwind, Elwynn", 2, Some(12)), + node(6, "Ironforge, Dun Morogh", 2, Some(1537)), + // A node on no zone map: resolved to nothing by the app, kept in the list. + node(99, "Nowhere", 0, None), + ]); + s + } + + /// pfQuest's own loop, run verbatim against the table this hands back: the fields it reads, + /// the gate it applies, and the `ipairs` walk it applies them in. + #[test] + fn the_no_argument_call_answers_every_node_in_pfquests_own_loop() { + let s = seeded(); + let drawn = s + .eval::( + r#" + local out = "" + for _, node in ipairs(C_TaxiMap.GetTaxiNodesForMap()) do + if node.areaID and node.mapX and node.mapY then + out = out .. node.nodeID .. ":" .. node.name .. ":" .. node.faction + .. ":" .. node.areaID .. ":" .. node.mapX .. "," .. node.mapY .. " " + end + end + return out + "#, + ) + .unwrap(); + assert_eq!( + drawn, "2:Stormwind, Elwynn:2:12:33.5,66.5 6:Ironforge, Dun Morogh:2:1537:33.5,66.5 ", + "file order, every field the caller reads, and the unresolved node gated out by its \ + own test rather than missing from the list" + ); + assert_eq!( + s.eval::("return table.getn(C_TaxiMap.GetTaxiNodesForMap())") + .unwrap(), + 3, + "the unresolved node is still in the array — the caller's gate drops it, not us" + ); + } + + /// An argument narrows to one zone by `AreaTable` id, the id space the rest of this compat + /// layer's map calls speak. + #[test] + fn an_area_id_narrows_the_answer_to_that_zone() { + let s = seeded(); + assert_eq!( + s.eval::( + "local n = C_TaxiMap.GetTaxiNodesForMap(1537) \ + return table.getn(n) .. \"/\" .. n[1].name" + ) + .unwrap(), + "1/Ironforge, Dun Morogh" + ); + assert_eq!( + s.eval::("return table.getn(C_TaxiMap.GetTaxiNodesForMap(4))") + .unwrap(), + 0, + "a zone with no flight master answers an empty array, not nil" + ); + } + + /// The feature test the caller actually writes, which is the whole reason this file exists. + #[test] + fn pfquests_own_feature_test_passes() { + let s = UiScript::new().unwrap(); + assert!( + s.eval::( + "if not (C_TaxiMap and C_TaxiMap.GetTaxiNodesForMap) then return false end \ + return true" + ) + .unwrap(), + "`SearchFlightNodes` returns an empty table before this test passes" + ); + // Empty catalog, not seeded: still a table, so the loop runs zero times instead of raising. + assert_eq!( + s.eval::("return table.getn(C_TaxiMap.GetTaxiNodesForMap())") + .unwrap(), + 0 + ); + } +} diff --git a/crates/benilla-ui/src/script/mod.rs b/crates/benilla-ui/src/script/mod.rs index 54f6c0c3..ff6c0910 100644 --- a/crates/benilla-ui/src/script/mod.rs +++ b/crates/benilla-ui/src/script/mod.rs @@ -269,7 +269,7 @@ pub use spellbook::{ pub use stable::{StableIntent, StablePetSlot, StableState, NUM_STABLE_SLOTS}; pub use summon::SummonConfirmUiState; pub use talent::{TalentPrereqView, TalentTabView, TalentUiState, TalentView}; -pub use taxi::{TaxiNodeType, TaxiUiNode, TaxiUiState}; +pub use taxi::{TaxiMapNode, TaxiNodeType, TaxiUiNode, TaxiUiState}; pub use tooltip_spell::SpellTooltipView; pub use trade::{TradeSideState, TradeSlotItem, TradeState, TRADE_SLOTS}; pub use tradeskill::{TradeSkillDifficulty, TradeSkillReagent, TradeSkillRecipe, TradeSkillState}; diff --git a/crates/benilla-ui/src/script/model.rs b/crates/benilla-ui/src/script/model.rs index 5be506d0..30ef5230 100644 --- a/crates/benilla-ui/src/script/model.rs +++ b/crates/benilla-ui/src/script/model.rs @@ -952,6 +952,11 @@ pub(crate) struct Model { pub(crate) taxi_takes: Vec, pub(crate) taxi_close: bool, pub(crate) taxi_riding: bool, + /// The whole `TaxiNodes.dbc` table for ClassicAPI's `C_TaxiMap.GetTaxiNodesForMap`, resolved + /// to zones and map percentages app-side. Nothing to do with [`Self::taxi`]: that is the open + /// window's *known* nodes and is cleared when it closes, this is static file data that stands + /// for the session (`compat::classic_api::taxi_map`). + pub(crate) taxi_catalog: Vec, /// The open tradeskill window's recipes. pub(crate) trade_skill: Option, @@ -1866,6 +1871,7 @@ impl Model { taxi_takes: Vec::new(), taxi_close: false, taxi_riding: false, + taxi_catalog: Vec::new(), trade_skill: None, trade_skill_dos: Vec::new(), trade_skill_selection: 0, diff --git a/crates/benilla-ui/src/script/taxi.rs b/crates/benilla-ui/src/script/taxi.rs index 32f5b55a..799f879c 100644 --- a/crates/benilla-ui/src/script/taxi.rs +++ b/crates/benilla-ui/src/script/taxi.rs @@ -57,7 +57,41 @@ pub struct TaxiUiState { pub nodes: Vec, } +/// One `TaxiNodes.dbc` row as ClassicAPI's `C_TaxiMap.GetTaxiNodesForMap` hands it to Lua. +/// +/// Every row in the file, not the player's known mask and not the open window's list: the callers +/// draw flight masters on a map, so a node the player has never visited still has to be in the +/// answer. +#[derive(Clone, Debug, Default, PartialEq)] +pub struct TaxiMapNode { + /// `TaxiNodes.dbc`'s own id, the `nodeID` field, which both consumers key their own tables by. + pub node_id: u32, + /// The localized name, "Ironforge, Dun Morogh". + pub name: String, + /// `ClassicAPI`'s `Enum.FlightPathFaction`: 0 neutral, 1 Horde, 2 Alliance. Read off the two + /// `MountCreatureID` columns, which is how the reference decides whom a node serves + /// (`0x4a2650` reads the player's faction half): both filled is neutral. + pub faction: u8, + /// The `AreaTable` zone whose `WorldMapArea` rect contains the node, the `areaID` field; + /// `None` for a node on no zone map, which the callers skip. + pub area_id: Option, + /// The node's place inside that zone's rect, `(mapX, mapY)` in **percent** from its top-left — + /// the units pfQuest's `AddNode` stores and divides by 100 again when it places a pin. + pub map_percent: Option<(f32, f32)>, +} + impl super::UiScript { + /// Push the static `TaxiNodes.dbc` catalog for `C_TaxiMap`. + /// + /// No event: no stock frame reads it, and the one consumer replays its saved tracking + /// selections on `PLAYER_ENTERING_WORLD` and then unregisters every event + /// (`pfQuest/database.lua:131-151`), so this must be seated *before* the interface loads or + /// the flight-master layer stays empty for the whole session. Seeded beside the world-map + /// catalog for that reason. + pub fn set_taxi_map_catalog(&mut self, nodes: Vec) { + self.model_mut().taxi_catalog = nodes; + } + /// Push (or clear, with `None`) the open taxi map's snapshot. pub fn set_taxi(&mut self, state: Option) { self.model_mut().taxi = state; diff --git a/crates/benilla-ui/src/script/tests/reference_surface.rs b/crates/benilla-ui/src/script/tests/reference_surface.rs index e2006221..5757ae7b 100644 --- a/crates/benilla-ui/src/script/tests/reference_surface.rs +++ b/crates/benilla-ui/src/script/tests/reference_surface.rs @@ -516,6 +516,13 @@ fn allowed_beyond_1_12() -> HashSet<&'static str> { // all. This install's pfQuest has deleted its own zone-size and subzone tables in favour // of these. "C_Map", + // ClassicAPI's C_TaxiMap (script/compat/classic_api/taxi_map.rs), its one installed + // member: `GetTaxiNodesForMap`, the whole `TaxiNodes.dbc` table resolved to zones and map + // percentages. 1.12 has no spelling for it even in principle — its taxi verbs + // (`NumTaxiNodes` and friends) answer only about the window a flight master has open, from + // the player's known mask, so "where are the flight masters" is unaskable. This install's + // pfQuest has deleted its shipped flight table in favour of this call. + "C_TaxiMap", // ClassicAPI's C_PlayerInfo (script/compat/classic_api/player_info.rs), its one installed // member: `CanUseItem`, the same requirement gate `C_Item.IsUsableItem` and the tooltip's // red lines read (`item_stats::item_usable_by_id`). 1.12 answers the fact only as the diff --git a/docs/ADDON_COMPAT.md b/docs/ADDON_COMPAT.md index 6c8b7386..c6bb61ba 100644 --- a/docs/ADDON_COMPAT.md +++ b/docs/ADDON_COMPAT.md @@ -280,6 +280,7 @@ a loot roll"; it does not, because that module never runs.** The call path was n | `UnitSpellHaste` | done, **0** | 0 is the *answer*, not a stub: 1.12 has no haste stat anywhere — not a `UNIT_FIELD`, not an item mod, not a spell modifier — so every unit genuinely has 0% spell haste and the real DLL has nothing else to read either. pfUI's player module calls it unguarded into a `string.format("%.1f%%")` (`modules/player.lua:87`) whenever its haste readout is on, so the absence raised once per 0.25s tick and took the neighbouring "+N SP" line down with it | | `C_EquipmentSet` (13 members) | not built, **unreachable** | 1.12 has no equipment sets at all. pfUI's manager is a backport of the 4.3.4 `GearManagerDialog` and gates itself off at its first line (`if not C_EquipmentSet or not C_EquipmentSet.CanUseEquipmentSets() then return end`), so the module never loads and none of the 13 is ever called — verified in a loaded VM (`pfUI.equipmentmanager` is nil). Building the read half to answer "no sets" would load a manager that can only ever show an empty list; building the write half means a persisted set store and an equip sequencer, which is a feature, not a compat shim | | `C_LootHistory` (4 members) | not built, **unreachable** | Needs the three `LOOT_HISTORY_*` events as well as the namespace — the module's `FullUpdate` returns unless its frame is shown, and only those events show it. The data is on the wire and already parsed (`LootStartRoll`/`LootRoll`/`LootRollWon` carry the roll key, the roller, the dice value, the vote and the winner), so a real history is buildable: accumulate per `(looted_target, item_slot)`, resolve roller GUIDs to name/class, fire the three events. A feature worth doing on its own, not an error to patch | +| `C_TaxiMap.GetTaxiNodesForMap` — **the flight-master layer** | done | `compat/classic_api/taxi_map.rs` over a new feed, `ui_taxi/map_nodes.rs`; supersedes this member in the "not built" row below, which stands for the rest of its list. The guard that row cites is exactly why it mattered: pfQuest *is* guarded, so the absence produced no error at all — `pfDatabase:SearchFlightNodes` returned an empty table, the user's "flight masters" option stayed ticked, and nothing was ever drawn. Same silent-by-construction shape as `C_Map.GetAreas` above, and the same consumer. This install's pfQuest has **deleted** its shipped `pfDB.meta.flight` table for this call (`db/meta.lua:181` is a stub whose comment says the data "always comes live from the client (ClassicAPI)"), so there is no fallback behind it. **The contract is half read off the DLL and half off the caller**, and the file says which is which: `C_TaxiMap`, `GetTaxiNodesForMap`, `nodeID`, `faction`, `reachable`, `state` and the `FlightPathFaction`/`FlightPathState` enum names are all in the bundled `ClassicAPI.dll`'s string pool; the `areaID`/`mapX`/`mapY` triple is **not** separately visible there (the pool holds one copy of each string and `C_Map.GetAreaTriggerInfo` already uses all three), so for those the authority is `SearchFlightNodes` itself — it reads all six fields, gates on `if show and node.areaID and node.mapX and node.mapY`, and feeds the pair into `pfMap:AddNode` as `meta.x`/`meta.y`, which `UpdateNodes` divides by 100, so they are **percentages of the zone rect**. Faction comes from the two `MountCreatureID` columns, which is the reference's own gate (the taxi window asks for the mount of the player's half, `0x4a2650`): one column filled is that faction, both is neutral. A row with **neither** is reported neutral rather than dropped, deliberately and with the reason in the code — OctoWoW ships custom flight points and a custom row with unfilled mount columns is still a flight master standing in the world, so a stray icon is the cheaper mistake than a missing feature; the seed logs that count so the choice stays visible. Zone and percentage come from `WorldMapUiData::zone_at`, which asks `map_proj::zone_uv` for containment rather than comparing rects itself, so a node is only ever attributed to a zone whose own map the window would place it inside; where rects nest (every city sits inside its parent zone) the **smallest** wins, because a flight master in Ironforge belongs on the Ironforge map, which is also the map the player is looking at while standing there. 1.12 has no verb that asks this question, so that choice is ours and is labelled as such. **Seeded before the interface loads**, beside the world-map catalog and after it (it reads those rects), and the deadline is tighter than file scope: pfQuest replays its saved tracking selections once on `PLAYER_ENTERING_WORLD` and then calls `this:UnregisterAllEvents()` (`database.lua:131-151`), so a push one frame later is a push after the only read and the layer stays empty for the session. **Not built, each for the same reason**: `GetTaxiRoute`, `GetTaxiPaths` and `GetTaxiPathWaypoints` are `TaxiPath`/`TaxiPathNode` data, which `ui_taxi/routing.rs` loads for the window's own route search and has never pushed into the VM, and nothing in the corpus calls them; `GetAllTaxiNodes` is in the pool with no caller to read its shape against, and guessing it would be inventing a contract rather than matching one. Also absent from each node, with the reason stated in the file: `reachable` and `state`, which are session facts the open window owns (`model.taxi`, which exists only while a flight master has the frame up) — answering `reachable = false` for all 60 nodes because no window is open would be a confident wrong answer, where an absent field is a truthful "this build does not say" | | `C_EncodingUtil` (8), `C_FriendList.SetFriendNotes`, `C_Item.GetCurrentItemLevel`, `C_Map.GetAreaTriggerInfo`, `C_SpellBook.GetSkillLineRank`, `C_TaxiMap.GetTaxiNodesForMap`, `C_Spell.IsSpellInRange`/`IsNextMeleeSpell`/`IsRangedAutoAttackSpell`/`ResetsMeleeSwing`, `C_Item.RequestLoadItemDataByID`, `C_LootHistory.*` | not built | Each either guarded by its caller (all of pfQuest's) or behind a user action not yet taken: `SetFriendNotes` on editing a friend note, `SerializeJSON` on sharing a config, `GetCurrentItemLevel` behind `if ShaguScore` (an addon not installed), the hunter range pair on a hunter's bar. None raises in ordinary play as installed |