diff --git a/crates/benilla-app/assets/ui/MapCanvas.lua b/crates/benilla-app/assets/ui/MapCanvas.lua index aa6fa061..9b392708 100644 --- a/crates/benilla-app/assets/ui/MapCanvas.lua +++ b/crates/benilla-app/assets/ui/MapCanvas.lua @@ -1370,6 +1370,38 @@ function MapCanvasMixin:OnMapChanged() self:CallDataProviders("OnMapChanged") end +-- The same news, for the addons that were never written to this API. +-- +-- A 1.12 map addon anchors its icons to `WorldMapDetailFrame` or `WorldMapButton` and keeps them +-- a constant size on screen by counter-scaling them whenever the map's scale moves. On 1.12 the +-- only thing that can move it is `SetScale` on one of those frames -- Magnify, Mapster, +-- `WorldMapFrame_Minimize` -- so that is what such an addon hooks, and it reads the new scale off +-- `GetEffectiveScale` *inside* the hook rather than from the argument (pfQuest hooks all three of +-- `WorldMapFrame`, `WorldMapDetailFrame` and `WorldMapButton`, `pfQuest/map.lua:1622-1630`, and +-- recomputes from `WorldMapButton:GetEffectiveScale()`). +-- +-- The canvas zooms by scaling its *child*, so the effective scale those addons would read is +-- already right and the hook that would make them read it never fires: measured over a glide from +-- 8x to 30x with OpenMapClassic driving, `pfMap.ResizeNodes` ran **0** times, and a 16 px icon on +-- `WorldMapButton` drew 25.3 px at one zoom and 53.1 px at another. That is grom's "can only see +-- the pfui pins at certain distances" -- icons sized for whichever zoom they were last built at. +-- With the announcement the same glide runs it 24 times, once per easing frame, which is what a +-- 1.12 smooth-zoom addon already costs it. +-- +-- So announce it the only way 1.12 can hear: `SetScale` with the frame's *own* scale. It is a +-- notification and nothing else -- the value is unchanged, so nothing moves, nothing is scaled +-- twice, and an addon that ignores `SetScale` is exactly as it was. +local function AnnounceScaleToTwelve() + for _, f in ipairs({ WorldMapDetailFrame, WorldMapButton }) do + if f and f.GetScale and f.SetScale then + local scale = f:GetScale() + if scale and scale > 0 then + f:SetScale(scale) + end + end + end +end + function MapCanvasMixin:OnCanvasScaleChanged() local pins = self.activePins for i = 1, table.getn(pins) do @@ -1379,6 +1411,7 @@ function MapCanvasMixin:OnCanvasScaleChanged() end end self:CallDataProviders("OnCanvasScaleChanged") + AnnounceScaleToTwelve() end function MapCanvasMixin:OnCanvasPanChanged() diff --git a/crates/benilla-app/src/game_plugins.rs b/crates/benilla-app/src/game_plugins.rs index 4f7407e8..7e8fe01b 100644 --- a/crates/benilla-app/src/game_plugins.rs +++ b/crates/benilla-app/src/game_plugins.rs @@ -907,7 +907,16 @@ pub(crate) mod schedule_tests { /// `Res` reader in the frame, which measured 43 pairs under /// `WOW_AMBIGUITY_DUMP=1`. The orders that *could* decide something are declared: after the /// Lua pass that arms the edge, before the sound drain that plays what that pass queued. - const UPDATE_ACTIONABLE_CEILING: usize = 4_955; + /// +1 for `ui_script::demo_unit_feed`, which now reads `Res` so the server-less + /// capture's synthetic player carries its own **world position** — the field + /// `C_Map.GetPlayerMapPosition` and SuperWoW's `UnitPosition` answer from, and without it a + /// capture was the one client where the 1.12 map verb and the modern one disagreed. Its one + /// pair is against `player::camera_saved::load_camera_pose`, the one-shot that writes `Player` + /// when a character's saved camera is restored. The two cannot both do anything in the same + /// process: this one is `run_if(capture_ui_active)`, and a capture has no roster identity, so + /// `load_camera_pose` returns at its first line. Were they ever both live, the order would + /// decide only which frame the synthetic snapshot first carries the restored position. + const UPDATE_ACTIONABLE_CEILING: usize = 4_956; const UPDATE_ACTIONABLE_SLACK: usize = 40; fn ratchet(what: &str, n: usize, ceiling: usize, slack: usize) { diff --git a/crates/benilla-app/src/ui_script/map_canvas_tests.rs b/crates/benilla-app/src/ui_script/map_canvas_tests.rs index 1ab03c48..ac786e2d 100644 --- a/crates/benilla-app/src/ui_script/map_canvas_tests.rs +++ b/crates/benilla-app/src/ui_script/map_canvas_tests.rs @@ -586,3 +586,46 @@ fn the_container_takes_the_mouse_and_out_levels_the_window() { keeps every press it does reach" ); } + +/// A 1.12 map addon learns the map's scale moved from `SetScale` on the frames it anchors to, and +/// from nothing else — on 1.12 that is the only thing that *can* move it. The canvas zooms its own +/// child, so the effective scale such an addon reads is right while the hook that would make it +/// read it never fires: measured with OpenMapClassic driving a glide from 8x to 30x, +/// `pfMap.ResizeNodes` ran 0 times and a 16 px icon on `WorldMapButton` drew 25.3 px at one zoom +/// and 53.1 px at another. With the announcement the same glide runs it 24 times, once an easing +/// frame. +/// +/// The announcement passes the frame's **own** scale, so it moves nothing: that is asserted here +/// too, because a value that is not the frame's own would scale the art a second time. +#[test] +fn a_canvas_zoom_tells_the_1_12_frames_their_scale_moved() { + let Some(mut s) = harness() else { return }; + s.run("ShowUIPanel(WorldMapFrame)").unwrap(); + s.resolve(); + // Hooked the way an addon hooks it (pfQuest/map.lua:1622-1630), argument and all. + s.run( + "BenillaScaleCalls, BenillaScaleArgs = 0, {} for _, f in ipairs({ WorldMapDetailFrame, WorldMapButton }) do local old = f.SetScale f.SetScale = function(self, v) BenillaScaleCalls = BenillaScaleCalls + 1 table.insert(BenillaScaleArgs, v) return old(self, v) end end", + ) + .unwrap(); + s.run("WorldMapFrame.ScrollContainer:InstantPanAndZoom(2, 0.5, 0.5)") + .unwrap(); + s.resolve(); + let (calls, detail, button, worst): (i64, f64, f64, f64) = s + .eval( + "local worst = 0 for _, v in ipairs(BenillaScaleArgs) do worst = math.max(worst, math.abs((v or 0) - 1)) end return BenillaScaleCalls, WorldMapDetailFrame:GetScale(), WorldMapButton:GetScale(), worst", + ) + .unwrap(); + assert!( + calls >= 2, + "a zoom has to reach both 1.12 frames (saw {calls} SetScale calls)" + ); + assert!( + worst < 1e-6, + "the announcement carries the frame's own scale, not a new one (worst |v - 1| = {worst})" + ); + assert_eq!( + (detail, button), + (1.0, 1.0), + "...so both frames are still at scale 1 afterwards" + ); +} diff --git a/crates/benilla-app/src/ui_script/mod.rs b/crates/benilla-app/src/ui_script/mod.rs index 0e91d3e0..c116b4d3 100644 --- a/crates/benilla-app/src/ui_script/mod.rs +++ b/crates/benilla-app/src/ui_script/mod.rs @@ -527,7 +527,11 @@ fn feed_framerate( } /// Synthetic player and target snapshots for a server-less UI capture, and its first events once. -fn demo_unit_feed(script: Option>, mut fired: Local>) { +fn demo_unit_feed( + script: Option>, + player: Res, + mut fired: Local>, +) { /// The synthetic target's guid: a creature high part; only its distinctness matters. const DEMO_TARGET_GUID: u64 = 0xF130_0000_0000_0001; @@ -561,6 +565,16 @@ fn demo_unit_feed(script: Option>, mut fired: Local (f32, f32, f32) { + let w = benilla_assets::coords::bevy_to_wow(t.translation); + (w[0], w[1], w[2]) +} + pub(crate) fn snapshot( store: &ObjectStore, name: Option, @@ -1487,11 +1504,7 @@ fn feed_units( let name = names .resolve_unit(guid.0, Some(store), &commands) .map(str::to_string); - let position = stores - .positions - .get(entity) - .ok() - .map(|t| (t.translation.x, t.translation.y, t.translation.z)); + let position = stores.positions.get(entity).ok().map(wow_position); let mut s = snapshot(store, name, 0, chr, position); s.is_player = true; // Every token pushed here is streamed, so connected; real link-death rides only the group @@ -1531,11 +1544,7 @@ fn feed_units( store, self_pair.map(|(_, s, _)| s), ); - let position = stores - .positions - .get(e) - .ok() - .map(|t| (t.translation.x, t.translation.y, t.translation.z)); + let position = stores.positions.get(e).ok().map(wow_position); let mut s = snapshot(store, name, reaction, chr, position); s.guid = guid; s.is_connected = true; @@ -1574,11 +1583,7 @@ fn feed_units( }; let token_snapshot = |entity: Entity, guid: u64| -> Option { let store = stores.all.get(entity).ok()?; - let position = stores - .positions - .get(entity) - .ok() - .map(|t| (t.translation.x, t.translation.y, t.translation.z)); + let position = stores.positions.get(entity).ok().map(wow_position); Some(snapshot_ctx.full(guid, store, position)) }; // The guid→entity hop (`0x468460`); `index` is absent in a UI-only harness, which resolves @@ -1940,6 +1945,65 @@ fn combo_edge(last: Option<(u8, u64)>, now: (u8, u64)) -> Option { mod tests { use super::*; + /// [`UnitState::position`] is published in **world yards**, the frame every reader of it is + /// written against: SuperWoW's `UnitPosition` and UnitXP_SP3's `getUnitPosition` answer the + /// game's own `(x, y, z)`, and `C_Map.GetPlayerMapPosition` un-lerps the point against + /// `WorldMapArea`'s yard rect. + /// + /// A streamed object's `Transform` is Bevy's frame. Read raw, as this did, the two map axes + /// are swapped and negated and the third is the *height* — so `GetPlayerMapPosition` answered + /// nil against a zone's rect (OpenMapClassic's "You aren't on the world map here", and every + /// stitched-map arrow declining to place itself) and answered a plausible wrong fraction + /// against a continent's. Live, the 1.12 verb and the modern one then disagreed about where + /// the same player was, which is not a state a real client can be in. + #[test] + fn a_units_published_position_is_world_yards_not_bevys_frame() { + use bevy::ecs::system::RunSystemOnce; + + /// Northshire Abbey, the capture's own parking spot. + const WOW: [f32; 3] = [-8949.95, -132.49, 83.53]; + + let mut app = App::new(); + app.add_message::(); + app.init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::(); + let (tx, _rx) = crossbeam_channel::unbounded(); + app.insert_resource(NetCommands(tx)); + app.insert_non_send_resource(UiScript::new().unwrap()); + app.world_mut().spawn(( + SelfPlayer, + Guid(0x77), + ObjectStore( + benilla_protocol::ObjectFields::from_pairs(&[]) + .into_created(benilla_protocol::messages::ObjectType::Player), + ), + Transform::from_translation(benilla_assets::coords::wow_to_bevy(WOW)), + )); + + app.world_mut().run_system_once(feed_units).unwrap(); + let mut script = app.world_mut().non_send_resource_mut::(); + script.resolve(); + let (x, y, z): (f32, f32, f32) = script + .eval(r#"return UnitPosition("player")"#) + .expect("the player's position is published"); + for (got, want, axis) in [(x, WOW[0], "x"), (y, WOW[1], "y"), (z, WOW[2], "z")] { + assert!( + (got - want).abs() < 0.01, + "{axis}: published {got}, the player is at {want} — a Bevy-frame read answers \ + ({}, {}, {})", + -WOW[1], + WOW[2], + -WOW[0], + ); + } + } + /// The installed `ChrRaces.dbc` outranks the table benilla carries: every race the chain has /// a row for answers that row's `name_lang`, in the world (`UnitRace`) and in the glue /// caption alike. On this OctoWoW install race 10's row reads **"High Elf"** where the 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 bf47cbc8..04c5c1a0 100644 --- a/crates/benilla-ui/src/script/compat/classic_api/mod.rs +++ b/crates/benilla-ui/src/script/compat/classic_api/mod.rs @@ -98,19 +98,28 @@ mod unit_menu; /// `CLASSIC_API_VERSION`, packed `X*10000 + Y*100 + Z` as upstream packs it. /// -/// **What this number claims, and why it is this number.** It is not a version of benilla's code: -/// it is a claim about which ClassicAPI *API surface* is present, and real addons gate on it. pfUI -/// 9.0.25 refuses to load at all below `11304` (its `API_Check.lua`'s `PFUI_CLASSIC_API_MIN`), so -/// a lower number is not a humbler claim — it is "pfUI is disabled", which is strictly worse for a -/// player than a few missing functions that error where they are actually used. +/// **What this number claims.** Not a version of benilla's code: a claim about which ClassicAPI +/// *API surface* is present, which real addons gate on. /// -/// So this says v1.13.4: the surface pfUI asks for. We do **not** implement every function of that -/// release (`docs/ADDON_COMPAT.md` has the per-namespace truth, and several namespaces are partial -/// or absent). An addon reaching for something unbuilt gets a clean `nil`-call error naming the -/// function, which is how the rest of this compat layer already behaves and is a far better -/// failure than a silent wrong answer. Raise this only alongside real coverage, and never to -/// quiet a specific addon without checking what else keys on it. -const CLASSIC_API_VERSION: i64 = 11304; +/// **v1.18.0, at grom's ask** — "need classic api to report version 1.18 in our client please". +/// What is on his disk beside the real client is older: `ClassicAPI.dll`'s PE version resource +/// reads `FileVersion 1.15.0.0`, and the highest number any addon in his folder tests is pfUI's +/// `PFUI_CLASSIC_API_LATEST = 11400`, which is only the threshold for its "an update is +/// available" chat line. So nothing measured here *needs* 11800; it is what the player asked the +/// client to say, and the risk it carries is written down rather than argued with: an addon that +/// feature-gates on a version instead of on the function takes a branch into whatever 1.16-1.18 +/// added, and reaches for names this layer may not have. +/// +/// That risk is bounded by how this layer fails. An addon reaching for something unbuilt gets a +/// clean `nil`-call error naming the function, which is how the rest of `compat/` already behaves +/// and is a far better failure than a silent wrong answer. We do **not** implement every function +/// of that release — `docs/ADDON_COMPAT.md` has the per-namespace truth, and several namespaces +/// are partial or absent — and this number has never claimed we did. +/// +/// The floor is unchanged and still load-bearing: pfUI 9.0.25 refuses to load at all below +/// `11304` (`API_Check.lua`'s `PFUI_CLASSIC_API_MIN`), so a *lower* number is not a humbler +/// claim, it is "pfUI is disabled". +const CLASSIC_API_VERSION: i64 = 11800; /// `INTERFACE_VERSION`: the client's own interface number, 1.12.1's `11200` — the `## Interface` /// value a 1.12 `.toc` carries. Unrelated to [`CLASSIC_API_VERSION`] despite living beside it. diff --git a/docs/ADDON_COMPAT.md b/docs/ADDON_COMPAT.md index 397bf229..0bd30281 100644 --- a/docs/ADDON_COMPAT.md +++ b/docs/ADDON_COMPAT.md @@ -3473,3 +3473,119 @@ entry. Measured: Dun Morogh has 20 overlays, 9 unexplored, and `ns.MapReveal[142 those 9 as `Interface\WorldMap\DunMorogh\…` paths where it used to carry retail ids. The render is **not** verified — the stitch is built before a capture's setup chunk can run, and the addon's own switch says `/reload required (overlays built on map open)`. + +## Two map verbs disagreed about where the player was + +> "even when I hit find me it says I'm not on the map, and where it shows the arrow is still +> incorrect" — grom + +`UnitState::position` is the field three different compat verbs answer from: SuperWoW's +`UnitPosition`, UnitXP_SP3's `getUnitPosition`, and — through `player_on_continent` — +`C_Map.GetPlayerMapPosition`. It was fed from a streamed object's `Transform`, **read raw**, and a +`Transform` is Bevy's frame: `wow_to_bevy` is `(-y, z, -x)`. So the two map axes arrived swapped +and negated and the third was the player's *height*. + +What that does depends on the size of the rect it is un-lerped against, which is why it looked +like two unrelated defects: + +* against a **zone**'s rect — a few thousand yards — the height lands far outside `0..1` and + `player_position_on` answers `None`. `C_Map.GetPlayerMapPosition(zone, "player")` → nil, which + is OpenMapClassic's `UnitWorld` returning nothing, which is *"You aren't on the world map here + (an instance or a place it doesn't cover)."* It is also `Map/StockArrow.lua` declining to place + anything, leaving 1.12's own placement — the wrong one — standing. Both reports, one cause. +* against a **continent**'s rect — sixteen thousand yards — a height of a few hundred yards is + comfortably inside `0..1`, so it answers a perfectly plausible wrong fraction. + +1.12's own `GetPlayerMapPosition` was never affected: it comes off the per-frame world-map feed +(`ui_world_map.rs`, `bevy_to_wow` at the top), not off any unit record. So the client was in a +state no real client can be in — the 1.12 verb placing the arrow and the modern verb saying the +player was on no map at all. `minimap::blips::party_member_pos` had the conversion all along, +three feet away. + +Fixed at the source, `ui_unit.rs`'s `wow_position`, so every reader gets world yards — which is +also what the real DLLs answer, so `UnitPosition` stopped being wrong for every addon that does +its own map arithmetic. Distance was the one read this never broke, the transform being rigid. + +Measured in a `ui-worldmap` capture, the player parked at Northshire: + +``` +UnitPosition = -8980, -160, 110 (was nil,nil,nil: see below) +1.12 GetPlayerMapPosition = 0.4590921998024, 0.70085144042969 +C_Map …(1415 Eastern Kingdoms) = 0.45909222960472, 0.70085144042969 ← agrees to 7 places +C_Map …(1429 Elwynn) = 0.48847541213036, 0.4495050907135 +C_Map …(1426 Dun Morogh) = nil ← correct: he is not in it +``` + +and the arrow, centred on its own world point at 8x, lands on the container's centre: +`arrow 92.61986541748 * 6.323 = 585.64` against `container 585.66674804688` — a third of a pixel. + +**The capture could not have shown any of this.** Its synthetic player +(`ui_script::demo_unit_feed`, the server-less UI fixture) carried no position at all, so every +`UnitState::position` reader answered nil there and the disagreement was invisible. It now reads +`Res` and publishes the capture's own parking spot in world yards; that costs one +ambiguous pair against `camera_saved::load_camera_pose`, written up at `UPDATE_ACTIONABLE_CEILING`. + +## A 1.12 zone's base art is blank parchment, so only the cities drew + +> "it will only paint the actual map art for cities it seems, everything else outside of coldridge +> valley, which I've discovered, is empty, I want to be able to see all of it" — grom + +Two shots of the same view (Dun Morogh at 6x, `WOW_CAPTURE_EXPLORED=none`) settle what this is: +with the fog `default` the map is empty paper carrying nothing but zone names, coastlines and — +in the middle of it — Ironforge, fully painted. With `reveal` the whole continent is there. + +That is not a defect in the art pipeline, it is what the data says. A 1.12 **zone** sheet +(`Interface\WorldMap\DunMorogh\DunMorogh1..12`) is parchment with the outline on it; every scrap +of painted terrain is a `WorldMapOverlay` piece, and the client hands out only the explored ones. +A **city** sheet has no overlays at all — `C_MapExplorationInfo.GetExploredMapTextures(1455)` is +empty and `BenillaMapTextures(1455)` is empty too — so the whole city is in the base art and draws +whatever the player has seen. Hence "only the cities", exactly. + +So the port's `unexploredMode` default moves to `"reveal"`, with the one-shot migration this +addon already uses for the kind (`revealMigratedBenilla`, beside `tileModeMigratedBenilla`): a +save written before the default moved still pins `"default"`, and grom's does. Map fog in the +menu, or `/omc unexplored default`, puts Blizzard's fog back. + +The "Darken unexplored areas" look is now load-bearing where it used to be nearly inert, and it +has a defect of its own worth writing down before someone rediscovers it: in `dim`, two of Dun +Morogh's and the Badlands' dungeon-entrance insets draw as solid black rectangles. In `show` the +same pieces are the small painted insets they should be. Not chased. + +## pfQuest's pins were sized for one zoom, because nothing told it the zoom had moved + +> "can only see the pfui pins at certain distances" — grom + +A 1.12 map addon anchors its icons to `WorldMapDetailFrame`/`WorldMapButton` and keeps them a +constant size on screen by counter-scaling them whenever the map's scale moves. On 1.12 the only +thing that can move it is `SetScale` on one of those frames (Magnify, Mapster, +`WorldMapFrame_Minimize`), so that is what the addon hooks — pfQuest hooks all three of +`WorldMapFrame`, `WorldMapDetailFrame` and `WorldMapButton` (`pfQuest/map.lua:1622-1630`) and +recomputes from `WorldMapButton:GetEffectiveScale()` *inside* the hook, which is the right +arithmetic. + +The canvas zooms by scaling its **child**. The effective scale pfQuest would read is therefore +correct, and the hook that would make it read it never fires. Measured over a glide from 8x to +30x with the counters armed at a capture's setup chunk: + +``` +before pokes detail=0 button=0 pfMap.ResizeNodes=0 16 px icon → 25.3 px at 2x, 53.1 px at 30x +after pokes detail=24 button=24 pfMap.ResizeNodes=24 (one per easing frame) +``` + +`MapCanvasMixin:OnCanvasScaleChanged` now announces the same news to 1.12 the only way 1.12 can +hear it: `SetScale` with each frame's *own* scale. It is a notification and nothing else — the +value is unchanged, so nothing moves, nothing is scaled twice, and an addon that ignores +`SetScale` is exactly as it was. `a_canvas_zoom_tells_the_1_12_frames_their_scale_moved` asserts +both halves, the calls and the unchanged value. + +## `CLASSIC_API_VERSION` now says 1.18.0 + +At grom's ask: *"also need classic api to report version 1.18 in our client please"*. What sits +beside the real client is older — `ClassicAPI.dll`'s PE version resource reads `FileVersion +1.15.0.0` — and the highest number any addon in his folder tests is pfUI's +`PFUI_CLASSIC_API_LATEST = 11400`, the threshold for its "an update is available" chat line, not a +gate. So nothing measured needs 11800; it is what the player asked the client to say, and the +trade is the one that constant's own comment has always named: an addon that gates on a version +rather than on a function takes the newer branch and may reach for a name this layer has not +built, where it gets a clean `nil`-call error naming the function. The floor is unchanged and +still load-bearing — below `11304`, pfUI does not load at all.