From 6ee3d88863588f54ac64b6f5eb09eb1d696d3c85 Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 17 Aug 2026 04:19:29 +0200 Subject: [PATCH] =?UTF-8?q?fix(ui):=20night-round=20review=20=E2=80=94=20F?= =?UTF-8?q?1=20real=20PlaceMarkerOnMap=20formula?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prior PlaceMarker() reading ("center at markerX0+x") was wrong. Binary Ninja elides gmMapUI::PlaceMarkerOnMap @0x004a18b0's entire FPU chain to bare, operand-less _ftol2() calls, so the pseudo-C under-specifies the function. A capstone disassembly of the raw bytes in the PDB-paired acclient.exe recovers the real formula: retail projects the AC display coordinate (range ~-102.4..102.4) onto the marker-area rect via a fixed-point-style transform, not a raw pixel add: X = m_x0 - w/2 - (int)((m_x1-m_x0+1) * (x*10+1024) * (-1/2048)) Y = m_y0 - h/2 - (int)((m_y1-m_y0+1) * (2047-(y*10+1024)) * (-1/2048)) Constants read directly from .rdata: 0x79bac8=10.0, 0x7aac78=1024.0, 0x7aac70=-1/2048, 0x7aac68=2047.0. The Y axis's FSUBR is retail's north-up flip. w/h halve with truncating integer division (matching retail's cdq;sub;sar idiom), not float division. Extracted the pure math into MapPageController.ComputeMarkerPosition so it's directly testable, and retargeted MapPageControllerTests to GOLDEN PIXEL values computed independently from the formula (never from the port's own output): the reviewer's canonical (0,0)->(122,128) case, a far-west and far-north case, and a real town-table entry (Arwic's landblock, cross-checked against RadarCoordinates). Applies to the green ring, house pin, and all 53 static town hotspots, which all resolve through the same PlaceMarker call. Corrected the recon doc's "accepted as-is" note, which had mistaken "the FPU argument-passing is BN-mangled" for a narrow issue instead of the whole-formula elision it actually was. Co-Authored-By: Claude Fable 5 --- docs/research/2026-08-17-map-house-recon.md | 35 +++++++-- .../UI/Layout/MapPageController.cs | 56 +++++++++++++- .../UI/Layout/MapPageControllerTests.cs | 74 +++++++++++++++++-- 3 files changed, 148 insertions(+), 17 deletions(-) diff --git a/docs/research/2026-08-17-map-house-recon.md b/docs/research/2026-08-17-map-house-recon.md index 9ca33f17..008dfca3 100644 --- a/docs/research/2026-08-17-map-house-recon.md +++ b/docs/research/2026-08-17-map-house-recon.md @@ -74,13 +74,34 @@ Retail source: `docs/research/named-retail/acclient_2013_pseudo_c.txt`. row TS-85** (`docs/architecture/retail-divergence-register.md`), which explicitly named `gmMapUI::AddMapNote @0x004A1C51` as the last unported `SetTooltip` call site. -- `gmMapUI::PlaceMarkerOnMap @0x004a18b0` (pc:171827): `MoveTo(m_x0 + (int)x - - width/2, m_y0 + (int)y - height/2)`, `SetVisible(1)`. The x87/FPU - argument-passing is BN-mangled in the raw decomp (the `_ftol2()` - placeholder swallows the actual `arg3`/`arg4` reads) — this formula is - the handoff's own already-verified reading and is accepted as-is; the - underlying `+x-w/2` / `+y-h/2` centering pattern is unambiguous from the - surrounding integer math. +- `gmMapUI::PlaceMarkerOnMap @0x004a18b0` (pc:171827): **CORRECTED + 2026-08-17 (night-round review, finding F1) — the "accepted as-is" + reading below was WRONG.** The BN pseudo-C's operand-less `_ftol2()` + calls are not just "argument-passing mangled" — they swallow the + ENTIRE FPU chain (constants, multiplies, the Y-axis FSUBR flip), not + merely the `arg3`/`arg4` reads. A direct capstone disassembly of the + raw bytes at `0x004a18b0` in the PDB-paired `acclient.exe` recovers the + true formula: retail projects the AC display coordinate (`x`/`y`, + range ≈ ±102.4) onto the marker-area rect via a fixed-point-style + transform, not a raw pixel offset: + `X = m_x0 - w/2 - (int)((m_x1-m_x0+1) * (x*10+1024) * (-1/2048))`, + `Y = m_y0 - h/2 - (int)((m_y1-m_y0+1) * (2047-(y*10+1024)) * (-1/2048))`, + then `SetVisible(1)`. Constants read from `.rdata`: `0x79bac8`=10.0, + `0x7aac78`=1024.0, `0x7aac70`=-1/2048, `0x7aac68`=2047.0. `w`/`h` are + `UIRegion::GetWidth`/`GetHeight` halved by INTEGER (truncating) + division, matching retail's `cdq;sub;sar` idiom. Golden case: marker + area (6,8)-(247,258), 10x10 icon, position 0.0N/0.0E → (122,128) + center — reproduced exactly. Ported at + `src/AcDream.App/UI/Layout/MapPageController.cs`'s `PlaceMarker`. The + ORIGINAL (wrong) note, kept for the historical record of how the + mistake happened: "`MoveTo(m_x0 + (int)x - width/2, m_y0 + (int)y - + height/2)`... the x87/FPU argument-passing is BN-mangled in the raw + decomp (the `_ftol2()` placeholder swallows the actual `arg3`/`arg4` + reads) — this formula is the handoff's own already-verified reading + and is accepted as-is; the underlying `+x-w/2` / `+y-h/2` centering + pattern is unambiguous from the surrounding integer math." It was not + unambiguous — the BN elision hid a whole coordinate-projection + transform behind what looked like a plain pixel add. - `gmMapUI::Update @0x004a1eb0` (pc:172084): re-arms `m_nextUpdate = Timer::cur_time + 5.0` every call (5 s cadence, driven by `ListenToGlobalMessage`'s `arg2==3` tick case). Date/time block: builds diff --git a/src/AcDream.App/UI/Layout/MapPageController.cs b/src/AcDream.App/UI/Layout/MapPageController.cs index 8541522b..0a128c5b 100644 --- a/src/AcDream.App/UI/Layout/MapPageController.cs +++ b/src/AcDream.App/UI/Layout/MapPageController.cs @@ -354,17 +354,65 @@ public sealed class MapPageController } /// - /// gmMapUI::PlaceMarkerOnMap @0x004a18b0: center the icon at - /// (markerAreaX0 + x, markerAreaY0 + y), then show it. + /// gmMapUI::PlaceMarkerOnMap @0x004a18b0, ported from a direct + /// byte-read of the PDB-paired acclient.exe (Binary Ninja elides + /// the whole FPU chain to bare, operand-less _ftol2() calls — + /// see docs/research/named-retail/acclient_2013_pseudo_c.txt + /// lines 171827-171855 — so the pseudo-C alone under-specifies this + /// function; capstone disassembly of the raw machine code at that VA + /// is the ground truth here, not the BN text). The prior "center at + /// markerX0+x" reading was WRONG — retail projects the AC display + /// coordinate (/, range + /// approximately ±102.4) onto the marker-area rect's pixel span via a + /// fixed-point-style transform, not a raw pixel add: + /// + /// X = m_x0 - w/2 - (int)( (m_x1-m_x0+1) * (x*10+1024) * (-1/2048) ) + /// Y = m_y0 - h/2 - (int)( (m_y1-m_y0+1) * (2047-(y*10+1024)) * (-1/2048) ) + /// + /// Constants read straight from the binary's .rdata: 0x79bac8 = + /// 10.0, 0x7aac78 = 1024.0, 0x7aac70 = -1/2048 (exactly + /// -0.00048828125), 0x7aac68 = 2047.0. The Y axis's FSUBR + /// (reversed subtract) is retail's north-up flip — Y increases upward + /// on the AC coordinate system but downward in screen pixels. + /// w/h are the icon's own UIRegion::GetWidth/ + /// GetHeight (@0x0069efe0/@0x0069eff0), halved with INTEGER + /// (truncating) division to match retail's cdq;sub;sar idiom — + /// not float division, which would drift by half a pixel on + /// odd-sized icons. Golden case (marker area (6,8)-(247,258), 10x10 + /// icon, position 0.0N/0.0E) reproduces exactly to (122,128) center. /// private void PlaceMarker(UiElement? icon, double x, double y) { if (icon is null) return; - icon.Left = _markerX0 + (float)x - icon.Width / 2f; - icon.Top = _markerY0 + (float)y - icon.Height / 2f; + + (float left, float top) = ComputeMarkerPosition( + _markerX0, _markerX1, _markerY0, _markerY1, + (int)icon.Width, (int)icon.Height, x, y); + icon.Left = left; + icon.Top = top; icon.Visible = true; } + /// + /// The pure PlaceMarkerOnMap math, split out from + /// so tests can assert byte-decoded GOLDEN PIXEL values directly against + /// the formula instead of round-tripping through the port's own output. + /// + internal static (float Left, float Top) ComputeMarkerPosition( + int markerX0, int markerX1, int markerY0, int markerY1, + int iconWidth, int iconHeight, double x, double y) + { + int halfWidth = iconWidth / 2; + int halfHeight = iconHeight / 2; + int extentX = markerX1 - markerX0 + 1; + int extentY = markerY1 - markerY0 + 1; + + int xOffset = (int)(extentX * (x * 10.0 + 1024.0) * (-1.0 / 2048.0)); + int yOffset = (int)(extentY * (2047.0 - (y * 10.0 + 1024.0)) * (-1.0 / 2048.0)); + + return (markerX0 - halfWidth - xOffset, markerY0 - halfHeight - yOffset); + } + private static ElementInfo? FindInfo(ElementInfo info, uint id) { if (info.Id == id) return info; diff --git a/tests/AcDream.App.Tests/UI/Layout/MapPageControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/MapPageControllerTests.cs index 9d48ef78..a81ddb0d 100644 --- a/tests/AcDream.App.Tests/UI/Layout/MapPageControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/MapPageControllerTests.cs @@ -103,6 +103,66 @@ public sealed class MapPageControllerTests } } + // ── Marker placement math (GOLDEN PIXEL values, byte-decoded formula) ── + // + // gmMapUI::PlaceMarkerOnMap @0x004a18b0. Binary Ninja elides the entire + // FPU chain to bare, operand-less _ftol2() calls; the formula below was + // recovered by disassembling the raw bytes of the PDB-paired + // acclient.exe directly (capstone) — see MapPageController.ComputeMarkerPosition's + // doc comment and docs/research/2026-08-17-map-house-recon.md's + // corrected PlaceMarkerOnMap entry. Every expected value here is a + // LITERAL computed independently from the formula (by hand / an + // external script), never by calling the port itself — that is the + // whole point of a golden-value test. + // + // X = m_x0 - w/2 - (int)((m_x1-m_x0+1) * (x*10+1024) * (-1/2048)) + // Y = m_y0 - h/2 - (int)((m_y1-m_y0+1) * (2047-(y*10+1024)) * (-1/2048)) + // + // Marker area used throughout: (6,8)-(247,258) — the live-fixture value + // (MapHousePanelSlotProbeTests). Icon: 10x10 (matches the town hotspot + // template's "plain 10x10 hotspot dot" and this test file's own fixture + // resolver). + + [Theory] + // Canonical case: dead center of Dereth (0.0N/0.0E) -> the marker + // area's own true center pixel. + [InlineData(0.0, 0.0, 122, 128)] + // Far west (x very negative): pixel X moves toward the marker area's + // left edge (m_x0=6), well below the center-case 122. + [InlineData(-100.0, 0.0, 3, 128)] + // Far north (y very positive): pixel Y moves toward the marker area's + // top edge (m_y0=8) — the FSUBR north-up flip means +Y in-game means + // SMALLER pixel Y, not larger. + [InlineData(0.0, 100.0, 122, 5)] + // A real town-table entry: Arwic's landblock cell 0x11CE0001 fed + // through the ALREADY-VERIFIED RadarCoordinates.TryFromCell (a + // different, independently-tested subsystem) to get x=-88.3/y=62.9, + // then through the formula above to get the expected pixel. + [InlineData(-88.30000000000001, 62.900000000000006, 17, 51)] + public void ComputeMarkerPosition_MatchesByteDecodedFormula_GoldenPixels( + double x, double y, int expectedLeft, int expectedTop) + { + (float left, float top) = MapPageController.ComputeMarkerPosition( + markerX0: 6, markerX1: 247, markerY0: 8, markerY1: 258, + iconWidth: 10, iconHeight: 10, x: x, y: y); + + Assert.Equal(expectedLeft, left); + Assert.Equal(expectedTop, top); + } + + [Fact] + public void ComputeMarkerPosition_ArwicCell_MatchesRadarCoordinates() + { + // Cross-check that the golden (x,y) literal used above for the + // "town-table entry" case really is what RadarCoordinates.TryFromCell + // produces for Arwic's landblock, so the golden test above can't + // silently drift from the coordinate subsystem it's chained to. + const uint cellId = 0x11CE0001u; + Assert.True(RadarCoordinates.TryFromCell(cellId, out RadarCoordinates coords)); + Assert.Equal(-88.30000000000001, coords.X, precision: 12); + Assert.Equal(62.900000000000006, coords.Y, precision: 12); + } + // ── Marker placement wiring (real fixture, no re-derivation) ──────────── [Fact] @@ -111,9 +171,10 @@ public sealed class MapPageControllerTests // Arwic's landblock cell id (0x11CE0001 — an arbitrary real outdoor // cell, picked only because RadarCoordinates.TryFromCell already // proves gid-to-lcoord conformance elsewhere; this test proves the - // WIRING, not the formula). + // WIRING, not the formula — the formula itself is golden-tested + // above). const uint cellId = 0x11CE0001u; - Assert.True(RadarCoordinates.TryFromCell(cellId, out RadarCoordinates expected)); + Assert.True(RadarCoordinates.TryFromCell(cellId, out _)); ElementInfo rootInfo = FixtureLoader.LoadMapHouseHostInfos(); ImportedLayout layout = FixtureLoader.LoadMapHouseHost(); @@ -134,10 +195,11 @@ public sealed class MapPageControllerTests Assert.True(playerIcon!.Visible); // markerArea from the live fixture (MapHousePanelSlotProbeTests): - // (6,8)-(247,258) -> m_x0=6, m_y0=8. - const int markerX0 = 6, markerY0 = 8; - Assert.Equal(markerX0 + (float)expected.X - playerIcon.Width / 2f, playerIcon.Left, precision: 3); - Assert.Equal(markerY0 + (float)expected.Y - playerIcon.Height / 2f, playerIcon.Top, precision: 3); + // (6,8)-(247,258). GOLDEN pixel value computed independently above + // (ComputeMarkerPosition_MatchesByteDecodedFormula_GoldenPixels' + // Arwic case) — (17,51) for this exact (x,y). + Assert.Equal(17f, playerIcon.Left); + Assert.Equal(51f, playerIcon.Top); } [Fact]