diff --git a/crates/benilla-app/assets/ui/OptionsFrame.xml b/crates/benilla-app/assets/ui/OptionsFrame.xml index 6311160e..c17a65e3 100644 --- a/crates/benilla-app/assets/ui/OptionsFrame.xml +++ b/crates/benilla-app/assets/ui/OptionsFrame.xml @@ -425,6 +425,13 @@ local body = getglobal("BenillaOptionsFrameContainerBody" .. k) if k == key then body:Show() else body:Hide() end end + -- A category switch starts at the TOP (2180), which is what entering and leaving a + -- search have always done two functions down and what the era's own ScrollBox does + -- per category. It was invisible until this record, because no PAGE outran its area + -- — only the search results did, and both of their edges reset. Now that Controls + -- carries sixteen rows, leaving it scrolled and clicking Interface would land the + -- player halfway down a page they have not seen the top of. + BenillaOptionsFrameContainerScroll:SetVerticalScroll(0) -- The Keybindings page (KeyBindingsPage.xml, decision 1008) body-swaps like the -- CVar pages but runs its own machinery: entering rebuilds the section tree; -- leaving disarms a straggling capture (the host seam must never outlive the page) @@ -784,10 +791,42 @@ .. "There is no exclusive fullscreen mode. WoW itself removed one in 8.0.1, and " .. "modern Classic offers these same two." + -- Brightness (2182). 1.12's own OPTION_TOOLTIP_GAMMA cannot ride this row: its whole + -- second sentence is "Increase brightness until you can clearly see all 21 levels of gray + -- bars to the right", and those bars are the stock video window's own calibration art, + -- which this page does not have (2182 §7 names it as a follow-up). The row also has to answer the + -- question the reference never had to — why a brightness control works here at all, when + -- 1.12's did nothing outside exclusive fullscreen. + BENILLA_TOOLTIP_BRIGHTNESS = + "Lightens or darkens the whole picture — nights, caves and interiors are what it is " + .. "for.\n\n" + .. "This is the same correction 1.12 applied through the display's own gamma ramp, " + .. "which it could only do in exclusive fullscreen. benilla applies it to the " + .. "finished image instead, so it works in both display modes." + OPTIONS_PAGE_ROWS = { + -- Seven rows became sixteen at 2180, and the nine that arrived were not new + -- settings: every one names a CVar this client already registered AND read, with no + -- control anywhere a player could reach. The four camera toggles are why the record + -- exists — 2149 built their mechanisms and 2179 rebuilt water collision, and none of + -- the four could be turned off from inside the game. + -- + -- SEATS: every row that was already here keeps the era-derived seat it had; each new + -- row takes the seat its own 1.12 box gives it, that box's COLUMNS read left to + -- right and flattened into this single one (the rule this page's Block Trades + -- comment already states). BasicOptionsGeneral runs 45 -> 3 -> 16 down its middle + -- column and 14 -> 61 down its right, so Attack on assist and Auto Clear AFK follow + -- Sticky Targeting and Auto Self Cast follows Block Trades. BasicOptionsCamera runs + -- the following-style dropdown -> 24 -> 26, then Slider2 -> 27 -> 15, then Slider3 + -- -> Slider4 — which is the block below verbatim, and which leaves Max Camera + -- Distance exactly where it already sat, last. Controls = { - "RowStickyTarget", "RowBlockTrades", "RowAutoLoot", "RowInvertMouse", - "RowMouseSpeed", "RowCameraFollowStyle", "RowMaxCameraDistance", + "RowStickyTarget", "RowAssistAttack", "RowClearAFK", + "RowBlockTrades", "RowAutoSelfCast", + "RowAutoLoot", "RowInvertMouse", "RowMouseSpeed", + "RowCameraFollowStyle", "RowFollowTerrain", "RowHeadBob", + "RowAutoFollowSpeed", "RowWaterCollision", "RowSmartPivot", + "RowMouseLookSpeed", "RowMaxCameraDistance", }, Audio = { "RowEnableAll", "RowMaster", "RowSound", "RowMusic", "RowAmbience", @@ -795,17 +834,23 @@ "RowBackgroundSound", }, Graphics = { - "RowUiScale", "RowFarclip", "RowWorldDetail", + "RowUiScale", "RowFarclip", "RowWorldDetail", "RowBrightness", + "RowWeatherDensity", "RowDisplayMode", "RowVerticalSync", "RowRenderScale", }, + -- Player Guild Names (2180) rides as a CHILD of Player Names, which is the + -- reference's own relationship twice over: its CheckButton22 hangs off 21 with a +26 + -- x indent, and UIOptionsFrame_UpdateDependencies l.702-708 greys 22 (and 23) from + -- 21's own tick. Its knob is `UnitNamePlayerGuild`, built by 2149's §6. Nameplates = { "RowEnemyPlates", "RowFriendlyPlates", - "RowPlayerNames", "RowNpcNames", "RowOwnName", + "RowPlayerNames", "RowGuildNames", "RowNpcNames", "RowOwnName", }, -- The stock globals that were already defined AND consumed when 1134 opened the store -- (1136; Buff Durations 1139), plus the two equipment-display toggles (1472), in the -- reference's own on-screen order. Its Basic Options Display box is three COLUMNS read - -- left to right: A gives Status Text (CheckButton2), B gives Show Cloak → Show Helm + -- left to right: A gives Enhanced Tooltips (CheckButton12, added 2180) then Status + -- Text (2), B gives Show Cloak → Show Helm -- (31 → 20, sitting under the name toggles that live on our Nameplates page), C gives -- the 42 → 39 → 66 chain. The **Help** box closes the page, and since 2077 it is -- the box's own order rather than its one shippable row: 28 (Show Tutorials), 44 @@ -818,7 +863,7 @@ -- "Raid & Party" — so there is no column order to preserve, only a neighbour to pick. -- It sits under Status Text because that is the other unit-frame readout on the page. Interface = { - "RowStatusText", "RowTargetOfTarget", "RowTargetOfTargetMode", + "RowEnhancedTooltips", "RowStatusText", "RowTargetOfTarget", "RowTargetOfTargetMode", "RowShowCloak", "RowShowHelm", "RowInstantQuestText", "RowBuffDurations", "RowAutoQuestWatch", "RowProfanityFilter", @@ -887,6 +932,13 @@ "RowCombatState", "RowDodgeParryMiss", "RowResistances", "RowReputation", "RowReactives", "RowFriendlyNames", "RowComboPoints", "RowEnergyGains", "RowHonorGained", + -- The damage-NUMBER trio (2180) shares the reference's box and NOT its master: + -- 19 -> 11 -> 9 close the AdvancedOptionsCombatText anchor chain, and the + -- "Additional combat text dependancies" block that greys 53…69 never names them + -- — 19 runs its own pair instead (UIOptionsFrame.lua l.709-714). So Show Target + -- Damage is a second, independent master on this page, and the two under it are + -- its children. + "RowShowDamage", "RowPeriodicDamage", "RowPetDamage", }, } @@ -931,6 +983,16 @@ function OptionsRow_Get(row) if row.uvar then return getglobal(row.uvar) end if row.getFunc then + -- A NUMERIC api row (2182): the store is an engine getter whose answer is a + -- number, not a flag. 1.12's own video panel is where this shape comes from -- + -- its slider table's `func` fork calls `getglobal("Get"..func)` and feeds the + -- result straight to `slider:SetValue` (OptionsFrame.lua l.110/135) -- and the + -- Brightness row is the one benilla ships. Normalised through the SAME two-decimal + -- strip the slider writes with, so a value sitting on the default compares equal + -- to it as a string and the pending/committed test below still works. + if row.numeric then + return (string.gsub(string.format("%.2f", row.getFunc()), "%.?0+$", "")) + end if row.getFunc() then return "1" else return "0" end end return GetCVar(row.cvar) @@ -965,10 +1027,41 @@ -- straight through) — so the engine binding reads its argument numerically and "0" -- means off. No applyFunc: the setter IS the side effect. if row.setFunc then - row.setFunc(value) + -- A numeric api row hands the setter a NUMBER, which is what 1.12 does for the + -- shape it borrows from: `OptionsFrame_Save` passes `slider:GetValue()` + -- (OptionsFrame.lua l.206-226), not a string. + if row.numeric then + row.setFunc(tonumber(value)) + else + row.setFunc(value) + end return end SetCVar(row.cvar, value, row.cvarEvent) + OptionsRow_WritePartner(row, value) + end + + -- **The PARTNER write (2180).** Three of 1.12's own rows drive a SECOND CVar that + -- appears in neither option table, from the special-case arms of `UIOptionsFrame_Save`: + -- Show Pet Melee Damage writes `PetSpellDamage` beside `PetMeleeDamage` (l.334-336), and + -- the two camera-speed sliders each write their pitch twin at a fixed ratio (l.351-356, + -- yaw/4 for the follow speed and yaw/2 for the look speed). Without this a row is a HALF + -- write — the box reads "pet damage off" while pet SPELL damage keeps floating — and it + -- is silent, because `SetCVar` on a name nobody wrote is not an error, it is a value + -- that stayed where it was. + -- + -- `scale` nil means a flag: the same "1"/"0" string goes to both. A number scales, and + -- the result is formatted the way the slider path formats its own write (two decimals, + -- trailing zeros stripped) so a partner sitting at its registered default compares equal + -- to it as a STRING and stays out of the config file. + function OptionsRow_WritePartner(row, value) + if not row.partner then return end + local v = value + if row.partner.scale then + v = string.gsub( + string.format("%.2f", tonumber(value) * row.partner.scale), "%.?0+$", "") + end + SetCVar(row.partner.cvar, v) end -- The registered default — GetCVarDefault's counterpart for a global. `udefault` is the @@ -1003,6 +1096,15 @@ row.fdefault = default end + -- The same, for a pair whose value is a NUMBER rather than a flag (2182) -- 1.12's video + -- panel's own `func` fork. `default` is still a string, because every store in this kit + -- answers in strings and the row compares them; the Brightness row computes it from + -- GetCVarDefault so the CVar table stays the one source. + function OptionsNumFuncRow_OnLoad(row, getFunc, setFunc, default, label, tip, child) + OptionsFuncRow_OnLoad(row, getFunc, setFunc, default, label, tip, child) + row.numeric = 1 + end + -- ── THE LOAD-TIME LADDER IS THE REFERENCE'S OWN AGAIN (decision 2115) ────────────── -- -- The problem it solves is real and unchanged (decision 1139): the whole UI's XML runs @@ -1069,9 +1171,15 @@ -- `BENILLA_GRAPHICS_SLIDER_ROWS` is index → the child of our Graphics page that the -- reference's row drives; the reference addresses its own sliders as -- `OptionsFrameSlider1..9`, a numbering our page does not use. An index with no entry is a - -- reference row we ship no knob for (rows 4-9: TerrainMip, BaseMip, Gamma, anisotropic, - -- spellEffectLevel, weatherDensity), and the OnShow re-apply loop skips it. - BENILLA_GRAPHICS_SLIDER_ROWS = { "RowUiScale", "RowFarclip", "RowWorldDetail" } + -- reference row we ship no knob for (rows 4, 5, 7 and 8: TerrainMip, BaseMip, + -- anisotropic, spellEffectLevel), and the OnShow re-apply loop skips it. Rows 6 and 9 are + -- Brightness (2182) and Weather Intensity (2181), which is why this table now has HOLES + -- rather than a run: the loop below is `for i, name in` — Lua 5.0's old-style `pairs` — + -- so it walks the keys that exist and an `ipairs` would have stopped at 3. + BENILLA_GRAPHICS_SLIDER_ROWS = { + "RowUiScale", "RowFarclip", "RowWorldDetail", + [6] = "RowBrightness", [9] = "RowWeatherDensity", + } -- -- **Both WorldDetail verbs are bound** (2163), which is what makes pfUI's module RUN -- rather than merely load: it hooks `SetWorldDetail`/`GetWorldDetail`, and hooking a nil @@ -1104,6 +1212,8 @@ function OptionsSliderRow_Setup(row, min, max, step, fmt, stops) local slider = getglobal(row:GetName() .. "ControlSlider") row.fmt = fmt + -- Kept for the "travel" readout, which is a share of this row's own range. + row.minv, row.maxv = min, max -- A grid that IS a short named ladder passes its names here (Environment Detail's -- Low/Medium/High over 0/1/2): the groove is still the reference's slider, and the -- readout says the stop rather than its index. 1.12's own slider printed no value at @@ -1219,6 +1329,16 @@ if i > table.getn(row.stops) then i = table.getn(row.stops) end return row.stops[i] end + -- A row whose stored value is a CENTRED OFFSET: show where the thumb is, as a share + -- of the groove (2182). Brightness is the case -- its stored unit is the reference's + -- own `1 - gamma`, which runs -0.5 .. +0.5 with the default at 0, and a readout of + -- "0%" on a brightness control that is doing nothing wrong is exactly backwards. As + -- travel it reads 0 .. 100 with the default at 50, the way a brightness control reads + -- everywhere else, while the value the store keeps stays the reference's. + if row.fmt == "travel" and row.minv and row.maxv and row.maxv > row.minv then + return string.format("%d%%", + math.floor((value - row.minv) / (row.maxv - row.minv) * 100 + 0.5)) + end -- A row whose stored value is a FACTOR but whose meaning is a distance: show the -- distance (decision 1140 — cameraDistanceMaxFactor over the 15 yd base). The number -- the player is choosing is "how far out the wheel stops", not "times 1.4". @@ -1391,21 +1511,59 @@ function OptionsPage_UpdateDependencies(key) if key == "Combat" then -- The 1.12 rule, verbatim (UIOptionsFrame_UpdateDependencies' "Additional combat - -- text dependancies" block): the master off greys EVERY sub-toggle and the scroll - -- dropdown. Combo Points carries the reference's second gate too — it is disabled - -- for anyone who is not a rogue or a druid, since nobody else has combo points. + -- text dependancies" block): the master off greys every FLOATING-TEXT sub-toggle + -- and the scroll dropdown. Combo Points carries the reference's second gate too — + -- it is disabled for anyone who is not a rogue or a druid, since nobody else has + -- combo points. local on = SHOW_COMBAT_TEXT ~= "0" local _, class = UnitClass("player") class = class and string.upper(class) local combo = class == "ROGUE" or class == "DRUID" + -- The damage-number trio is NOT under this master (2180). The reference's own + -- "Additional combat text dependancies" block names 53…69 and stops; 19/11/9 are + -- governed by the separate test at l.709-714. Greying them here would be a rule + -- the reference does not have, and it would read as "floating combat text off + -- means no damage numbers", which is the opposite of what CombatDamage does. + local damage = { + RowShowDamage = 1, RowPeriodicDamage = 1, RowPetDamage = 1, + } for _, rkey in ipairs(OPTIONS_PAGE_ROWS.Combat) do - if rkey ~= "RowCombatText" then + if rkey ~= "RowCombatText" and not damage[rkey] then local live = on if rkey == "RowComboPoints" and not combo then live = nil end OptionsRow_SetEnabled( getglobal("BenillaOptionsFrameContainerBodyCombat" .. rkey), live) end end + -- 1.12's second gate on this page, verbatim (l.709-714): Show Target Damage off + -- greys Periodic Damage and Pet Damage. The CVars keep their values — this is a + -- UI dependency, not a write — and the engine agrees with the shape: at + -- `CombatDamage` "0" both of `0x607140`/`0x6128b0`'s branch targets are + -- epilogues, so nothing floats from any source and the two sub-gates below have + -- nothing left to gate. + local dmg = GetCVar("CombatDamage") ~= "0" + OptionsRow_SetEnabled(BenillaOptionsFrameContainerBodyCombatRowPeriodicDamage, dmg) + OptionsRow_SetEnabled(BenillaOptionsFrameContainerBodyCombatRowPetDamage, dmg) + return + end + if key == "Controls" then + -- 1.12's own rule (l.715-719): the Auto-Follow Speed slider is dead while the + -- following style is Never, because there is no follow to set a speed for. The + -- reference tests its dropdown's selected ID against 3; ours stores the ENGINE's + -- values, where Never is "0" — and answers to "3" as well, since that is what + -- the reference's own dropdown writes and what a config or addon carrying it + -- means (the row's `alt`, decision 1502). + local style = GetCVar("cameraSmoothStyle") + OptionsRow_SetEnabled(BenillaOptionsFrameContainerBodyControlsRowAutoFollowSpeed, + style ~= "0" and style ~= "3") + return + end + if key == "Nameplates" then + -- 1.12's own rule (l.702-708): Player Names off greys Player Guild Names. The + -- reference greys its PvP-title twin (CheckButton23) from the same test; that + -- row is still on the unbacked census, so there is one child here, not two. + OptionsRow_SetEnabled(BenillaOptionsFrameContainerBodyNameplatesRowGuildNames, + GetCVar("UnitNamePlayer") ~= "0") return end if key == "Interface" then @@ -1455,8 +1613,35 @@ local name = row:GetName() local box = getglobal(name .. "Check") local dd = getglobal(name .. "Dropdown") + -- A SLIDER row greys the way 1.12's own OptionsFrame_DisableSlider does (l.481-486): + -- the THUMB is hidden — which is what says "this groove is dead" at a glance — and + -- the readout goes gray. Its Low/High end captions have no counterpart here; this kit + -- carries one readout instead of the reference's two. + -- + -- **And the mouse bit, which the reference does not need and we do.** There is no + -- `Slider:Disable()` to call — 1.12 gives a Slider no enabled state at all, which is + -- why this kit's Slider has no such method (script/slider.rs's `begin_drag`) — and a + -- thumbless 1.12 slider is already undraggable. Ours is not: a press anywhere on the + -- track seats the value (0250 §5's jump), so a greyed groove would still answer a + -- click. `EnableMouse(false)` is how the reference keeps anything off the press, and + -- it takes the frame out of the hit test, so the row underneath keeps its hover wash + -- instead of losing it with the control. + local sl = getglobal(name .. "ControlSlider") if box then if on then box:Enable() else box:Disable() end + elseif sl then + local thumb = getglobal(name .. "ControlSliderThumb") + local value = getglobal(name .. "ControlValue") + if on then + sl:EnableMouse(true) + thumb:Show() + value:SetTextColor(NORMAL_FONT_COLOR.r, NORMAL_FONT_COLOR.g, + NORMAL_FONT_COLOR.b) + else + sl:EnableMouse(false) + thumb:Hide() + value:SetTextColor(GRAY_FONT_COLOR.r, GRAY_FONT_COLOR.g, GRAY_FONT_COLOR.b) + end elseif dd then local text = getglobal(dd:GetName() .. "Text") if on then @@ -2314,19 +2499,60 @@ down the right, so the two belong adjacent when the box is flattened to one column. Its tooltip key IS the reference's, unlike the row above. --> - + + + + OptionsRow_OnLoad(self, "assistAttack", "Attack on assist", + "OPTION_TOOLTIP_ASSIST_ATTACK") + + + + + + + + + OptionsRow_OnLoad(self, "autoClearAFK", "Auto Clear AFK", + "OPTION_TOOLTIP_CLEAR_AFK") + + + + + + + OptionsRow_OnLoad(self, "BlockTrades", "Block Trades", "OPTION_TOOLTIP_BLOCK_TRADES") - + + + + OptionsRow_OnLoad(self, "autoSelfCast", "Auto Self Cast", + "OPTION_TOOLTIP_AUTO_SELF_CAST") + + + + + + + @@ -2401,11 +2628,113 @@ }) - + + + + OptionsRow_OnLoad(self, "cameraTerrainTilt", "Follow Terrain", + "OPTION_TOOLTIP_FOLLOW_TERRAIN") + + + + + + + + + + OptionsRow_OnLoad(self, "cameraBobbing", "Head Bob", + "OPTION_TOOLTIP_HEAD_BOB") + + + + + + + + + + OptionsRow_OnLoad(self, "cameraYawSmoothSpeed", "Auto-Follow Speed", + "OPTION_TOOLTIP_AUTO_FOLLOW_SPEED") + OptionsSliderRow_Setup(self, 90, 270, 10, "raw") + + + + + + + + + + OptionsRow_OnLoad(self, "cameraWaterCollision", "Water Collision", + "OPTION_TOOLTIP_WATER_COLLISION") + + + + + + + + + OptionsRow_OnLoad(self, "cameraPivot", "Smart Pivot", + "OPTION_TOOLTIP_SMART_PIVOT") + + + + + + + + + + OptionsRow_OnLoad(self, "cameraYawMoveSpeed", "Mouse Look Speed", + "OPTION_TOOLTIP_MOUSE_LOOK_SPEED") + OptionsSliderRow_Setup(self, 90, 270, 10, "raw") + self.partner = { cvar = "cameraPitchMoveSpeed", scale = 0.5 } + + + + + + + OptionsRow_OnLoad(self, "cameraDistanceMaxFactor", "Max Camera Distance", "OPTION_TOOLTIP_MAX_FOLLOW_DIST") @@ -2614,11 +2943,82 @@ { "Low", "Medium", "High" }) - + + + + -- The default is the reference's own computation + -- rather than a second copy of the number: + -- OptionsFrame_SetDefaults displays this row as + -- `1 - GetCVarDefault("gamma")` (OptionsFrame.lua + -- l.441-445), so the CVar table stays the single + -- source and Restore Defaults here lands exactly + -- where the reference's would. The `or 1` is for a + -- VM whose host registered nothing (a bare kit), + -- where the reference would have had a record to + -- read because its own lookup creates one. + local gd = tonumber(GetCVarDefault("gamma")) or 1 + OptionsNumFuncRow_OnLoad(self, GetGamma, SetGamma, + tostring(1 - gd), "Brightness", + "BENILLA_TOOLTIP_BRIGHTNESS") + OptionsSliderRow_Setup(self, OptionsFrameSliders[6].minValue, + OptionsFrameSliders[6].maxValue, + OptionsFrameSliders[6].valueStep, "travel") + + + + + + + + + + OptionsRow_OnLoad(self, "weatherDensity", "Weather Intensity", + "OPTION_TOOLTIP_WEATHER_DETAIL") + -- The stop names are OURS, and they name the + -- reference's own quality table rather than a + -- borrowed preset ladder: `0x67b870` maps 0/1/2/3 + -- onto the spawn-rate gains {0.1, 0.33, 0.66, 1.0} + -- in `[0x8680ec]`, so the top stop really is the + -- full rate and the bottom is a tenth of it. 1.12 + -- labelled no stop on this slider -- it labelled + -- the two ENDS -- but every slider row in this kit + -- carries a readout, and "2" is the one thing that + -- seat could say that means nothing to a player. + OptionsSliderRow_Setup(self, OptionsFrameSliders[9].minValue, + OptionsFrameSliders[9].maxValue, + OptionsFrameSliders[9].valueStep, nil, + { "Minimal", "Low", "Medium", "Full" }) + + + + + + + -- Display Mode (1650): MODERN CLASSIC's row, not -- 1.12's, because 1627 already gave us modern @@ -2939,6 +3339,64 @@ self.applyFunc = "CombatText_UpdateOrLoad" -- by NAME, resolved when applied: this file loads before its definer now (1938) + + + + + + + + OptionsRow_OnLoad(self, "CombatDamage", "Show Target Damage", + "OPTION_TOOLTIP_SHOW_DAMAGE") + + + + + + + + + + OptionsRow_OnLoad(self, "CombatLogPeriodicSpells", "Periodic Damage", + "OPTION_TOOLTIP_LOG_PERIODIC_EFFECTS", 1) + self.parentRow = "RowShowDamage" + + + + + + + + + + OptionsRow_OnLoad(self, "PetMeleeDamage", "Pet Damage", + "OPTION_TOOLTIP_SHOW_PET_MELEE_DAMAGE", 1) + self.parentRow = "RowShowDamage" + self.partner = { cvar = "PetSpellDamage" } + + - + + + OptionsRow_OnLoad(self, "UberTooltips", "Enhanced Tooltips", + "OPTION_TOOLTIP_USE_UBERTOOLTIPS") + + + + + + OptionsRow_OnLoad(self, "statusBarText", "Status Bar Text", "OPTION_TOOLTIP_STATUS_BAR") @@ -3480,8 +3952,9 @@ [0xc4da34] bit 0 / bit 3), then the three 1.12 UnitName* checkboxes over the overhead-name gates (nameplates.rs NameConfig), rows in the 1.12 Interface Options' own index - order (21/30/67), labels its GlobalStrings verbatim - (SHOW_PLAYER_NAMES/SHOW_NPC_NAMES/SHOW_OWN_NAME). + order (21/22/30/67 — 22, Player Guild Names, arrived with + 2180), labels its GlobalStrings verbatim (SHOW_PLAYER_NAMES/ + SHOW_GUILD_NAMES/SHOW_NPC_NAMES/SHOW_OWN_NAME). The plate pair leads because it is what "Nameplates" means to a player pressing V, and it is the pair that had no surface at @@ -3525,11 +3998,29 @@ OptionsRow_OnLoad(self, "UnitNamePlayer", "Player Names", "OPTION_TOOLTIP_SHOW_PLAYER_NAMES") - + + + + OptionsRow_OnLoad(self, "UnitNamePlayerGuild", "Player Guild Names", + "OPTION_TOOLTIP_SHOW_GUILD_NAMES", 1) + self.parentRow = "RowPlayerNames" + + + + + + + OptionsRow_OnLoad(self, "UnitNameNPC", "NPC Names", "OPTION_TOOLTIP_SHOW_NPC_NAMES") diff --git a/crates/benilla-app/src/cvars.rs b/crates/benilla-app/src/cvars.rs index 2df9f9a8..760f8e15 100644 --- a/crates/benilla-app/src/cvars.rs +++ b/crates/benilla-app/src/cvars.rs @@ -601,7 +601,8 @@ pub(crate) const REGISTERED: &[Registered] = &[ // // `PetSpellDamage` has no row in `UIOptionsFrameCheckButtons` — the *Show Pet Melee Damage* // box writes both (`UIOptionsFrame_Save` l.334-336) — which is why 2077's census, which reads - // that table, could not see it while it saw its two siblings. + // that table, could not see it while it saw its two siblings. Our own Pet Damage row carries + // the same partner write (2180); all three are on the Combat page, under `CombatDamage`. same("CombatDamage", "1"), same("PetMeleeDamage", "1"), same("PetSpellDamage", "1"), @@ -637,7 +638,9 @@ pub(crate) const REGISTERED: &[Registered] = &[ // is a shape-A binding (`0x790980`) that raises on a nil in the reference too, so the whole // window's `_Load` (and `_SetDefaults`, through `GetCVarDefault`) stopped at slider 3. // `cameraPitchMoveSpeed` has no row of its own: `UIOptionsFrame_Save` writes it as - // `sliderValue / 2` beside the yaw one (l.352-356), which is exactly this 180/90 pair. + // `sliderValue / 2` beside the yaw one (l.352-356), which is exactly this 180/90 pair — and + // the Controls page's Mouse Look Speed row carries that partner write (2180), so dragging it + // keeps the two axes in the ratio the registrar ships them at. // // **Both defaults are the reference's, and the shipped feel does not change** — the two facts // are compatible only because the unit divergence is carried in `camera::LOOK_YAW_PER_SPEED` @@ -695,7 +698,10 @@ pub(crate) const REGISTERED: &[Registered] = &[ // The auto-follow's rate (1502), °/s — 1.12's own AUTO_FOLLOW_SPEED slider // (`UIOptionsFrameSliders`, 90..270 by 10), registered at the binary's "180.0" (`[0xbe1070]`). // It sets the transition's DURATION (`|dyaw| / rate * factor`), so it is an average rate, not a - // slew. No row yet — the slider is a one-line follow-on now that the knob exists. + // slew. Its slider is the Controls page's Auto-Follow Speed row (2180), greyed while the + // following style is Never — and it writes only this one, where the reference also writes + // `cameraPitchSmoothSpeed` at a quarter of it: that name is deliberately unregistered here, + // because `FollowRig` has a single rate and a key with no reader is 1134 §4's pretence. same("cameraYawSmoothSpeed", "180"), // **The four 1.12 camera-option toggles** (decision 2149) — the `UIOptionsFrame` checkboxes // FOLLOW_TERRAIN / HEAD_BOB / SMART_PIVOT / WATER_COLLISION, all four of which sat on the @@ -1030,6 +1036,53 @@ pub(crate) const REGISTERED: &[Registered] = &[ // **not** one of `hwDetect`'s sixteen (scan of `[0x639a60, 0x639b80)`: sixteen record-pointer // reads, `0xc7f2e4` absent), so nothing overwrites it on any path. same("anisotropic", "1"), + // **Weather Intensity** — the video panel's slider 9 (`OptionsFrame.lua:29`, + // `func = "weatherDensity"`, a real CVar name rather than an engine binding), and the nearest + // of 2177 §10's named-not-done: benilla has had the feature since 0310 and only the switch was + // missing. The reader is `benilla_world::weather::WeatherState::weather_density`, which scales + // the rain/snow/mist spawn rate through the reference's own `0x67b870` quality table + // {0.1, 0.33, 0.66, 1.0}. Rendering only — it never touches the wire grade, the two ramp + // channels, or the storm/fog blend, so no server-visible behaviour rides it. + // + // The reference registers **`"2"`** at `0x67b806` (flags 0, callback `0x67b870`, name string + // `0x8685ac`) — wow-re `cvar/scratch/graphics-cost-cvar-census.md` §4, whose §10 table also + // lists this row among the twelve the reference install's `Config.wtf` moves off its default. + deviates( + "weatherDensity", + "3", + "2", + "2181: every precipitation rate in `benilla-world`'s own precipitation module was \ + derived and graded against the reference install's own apitrace captures, and that \ + install runs \ + `SET weatherDensity \"3\"` (K = 1.0) — so 3 is the value a benilla-vs-reference \ + side-by-side is correct at, and the registered 2 would thin every rate to 0.66 against \ + the only client we compare with. The slider is how a player takes it back down", + ), + // **Brightness** (decision 2182) — the reference's `gamma`, registered at `0x402d70` with + // name `0x82e924` `"Gamma"`, default string `0x82e92c` **`"1.0"`** and flags **0** (not + // latched, so its change callback `0x4034d0` applies on the write). + // + // There the callback builds `ramp[i] = __ftol(pow(i · 1/255, gamma) · 65535)` (`0x591680`) and + // hands the 3×256 words to `GDI32!SetDeviceGammaRamp` — and **skips the upload windowed** + // (`byte[dev+0x20b]` = `CGxFormat +0x07` = `gxWindow`), which is every mode benilla has. So the + // reader here is not a ramp upload: it is [`crate::ui_gamma::DisplayGamma`], the same curve + // applied to the same values one stage later, inside the pass that already owns the composited + // image's single decode. wow-re `ffxeffects/scratch/whole-frame-grade-verdict.md` §(a) for the + // curve and `ui/scratch/video-options-verbs.md` §3 for the verbs. + // + // 1.0 is the identity ramp — load-bearing rather than tidy: at the default this client's + // output is what it was before the setting existed, so no visual golden moves. + // + // **Written `"1.000000"` rather than `"1.0"` or `"1"`, and that is not cosmetic.** Every value + // this key ever receives comes through `SetGamma`, whose `SStrPrintf(buf, 0x10, "%f", …)` is + // six decimals — so the row that Restore Defaults produces is `"1.000000"`, and a default + // string in any other spelling would make it compare *moved* and write a `config.toml` line + // holding the default value. The slider rows dodge this with their own trailing-zero strip + // (`OptionsSlider_OnValueChanged`); a row whose store is an engine verb cannot, because the + // verb owns the formatting. So the table speaks the verb's spelling instead, and + // [`sync_cvars`] seeds it the same way. `Same` is still exact: the test parse-compares, and + // the reference registers this value as `"1.0"` (`0x82e92c`). + same("gamma", "1.000000"), // **Render scale** (decision 1639) — benilla's own CVar, no 1.12 counterpart, in the // `boothHalfRate` / `SoundOutputLimiter` mould: the reference has no such dial because it has // no second buffer to hang one on. The world renders into the composite lane's off-screen image @@ -1214,6 +1267,8 @@ pub(crate) struct KnobParams<'w> { names: ResMut<'w, NameConfig>, plates: ResMut<'w, VPlateMode>, clutter: ResMut<'w, ClutterConfig>, + weather: ResMut<'w, benilla_world::weather::WeatherState>, + display_gamma: ResMut<'w, crate::ui_gamma::DisplayGamma>, minimap: ResMut<'w, MinimapZoom>, bubbles: ResMut<'w, BubbleConfig>, zoom: ResMut<'w, ZoomLimit>, @@ -1257,6 +1312,8 @@ impl KnobParams<'_> { names: &mut self.names, plates: &mut self.plates, clutter: &mut self.clutter, + weather: &mut self.weather, + display_gamma: &mut self.display_gamma, minimap: &mut self.minimap, bubbles: &mut self.bubbles, zoom: &mut self.zoom, @@ -1296,6 +1353,12 @@ struct Knobs<'a> { names: &'a mut NameConfig, plates: &'a mut VPlateMode, clutter: &'a mut ClutterConfig, + /// The weather driver's own state — `weatherDensity` writes ONE byte of it + /// ([`benilla_world::weather::WeatherState::weather_density`]), the particle-density + /// step; every other field on it is the wire's, not a setting's (2181). + weather: &'a mut benilla_world::weather::WeatherState, + /// The display-brightness ramp the UI lane's decode applies (2182). + display_gamma: &'a mut crate::ui_gamma::DisplayGamma, minimap: &'a mut MinimapZoom, bubbles: &'a mut BubbleConfig, zoom: &'a mut ZoomLimit, @@ -1508,6 +1571,23 @@ fn apply_to_knobs(name: &str, value: &str, knobs: &mut Knobs) -> bool { // resulting density change exactly as it does for the row above (0992's setter law, which // is the callback's own chunk rebuild). "frilldensity" => knobs.clutter.set_frill_density(v), + // Weather Intensity, the panel's 0..3 step 1 (2181). The reference's callback is + // `0x67b870`, a jump table (`0x67b8e8`) mapping 0/1/2/3 onto the quality cells + // {0.1, 0.33, 0.66, 1.0} in `[0x8680ec]` (wow-re + // `cvar/scratch/graphics-cost-cvar-census.md` §4). What that table does with an + // off-grid int is NOT carved, so the clamp here is this table's own standing + // posture rather than a fidelity claim — and it costs nothing either way, because + // `WeatherState::density_gain` already `.min(3)`s its own index. + "weatherdensity" => knobs.weather.weather_density = v.trunc().clamp(0.0, 3.0) as u8, + // Brightness (2182). The clamp is OURS and the reference has none — the reason it + // costs one is on [`crate::ui_gamma::GAMMA_RANGE`], and nothing a player can reach + // from the panel meets it. + "gamma" => { + knobs.display_gamma.0 = v.clamp( + *crate::ui_gamma::GAMMA_RANGE.start(), + *crate::ui_gamma::GAMMA_RANGE.end(), + ) + } // The two zoom indices (1131) clamp exactly like the client's `set_zoom` (`0x6daa10`: // clamp at 5) — the widget clamps again on the way in, so a hand-edited level lands // in range whichever path it takes. @@ -1805,6 +1885,8 @@ fn sync_cvars( names, plates, clutter, + weather, + display_gamma, minimap, bubbles, zoom, @@ -1883,7 +1965,7 @@ fn sync_cvars( hardware_cursor: true, }); let flag = |b: bool| if b { "1" } else { "0" }.to_string(); - let session: [(&str, String); 85] = [ + let session: [(&str, String); 87] = [ ("MasterVolume", sound.master.to_string()), ("SoundVolume", sound.sfx.to_string()), ("MusicVolume", sound.music.to_string()), @@ -1977,6 +2059,9 @@ fn sync_cvars( ("WorldDetail", (clutter.density - 1.0).to_string()), // …and the same density in the reference's own cells-per-chunk (2151). ("frillDensity", clutter.frill_density().to_string()), + ("weatherDensity", weather.weather_density.to_string()), + // Six decimals, matching `SetGamma`'s own `"%f"` — see the row's comment. + ("gamma", format!("{:.6}", display_gamma.0)), ("ChatBubbles", flag(bubbles.all)), ("ChatBubblesParty", flag(bubbles.party)), ("profanityFilter", flag(text_filter.profanity)), @@ -2334,6 +2419,7 @@ mod tests { "gxDepthBits", "gxResolution", "realmList", + "weatherDensity", ], ); } @@ -2490,6 +2576,14 @@ mod tests { // The V-plate pair welds to VPlateMode's defaults — both OFF, which is the reference's // own boot state on both of its halves (the `[0xc4da34]` bitmask and FrameXML's // `NAMEPLATES_ON = nil`). Enemy plates were the 0167 director pin until 1804. + // Weather Intensity (2181): the CVar's default and the weather driver's own must be + // the same rain, or a fresh config writes a row the world does not agree with. This is + // also where the DEVIATION is held honest — the reference registers "2" and the row + // above says why we ship 3; the weld makes sure it is 3 in both places. + assert_eq!( + d["weatherDensity"], + f32::from(benilla_world::weather::WeatherState::default().weather_density) + ); let plates = VPlateMode::default(); assert_eq!(d[crate::vplates::CVAR_ENEMIES] != 0.0, plates.enemies); assert_eq!(d[crate::vplates::CVAR_FRIENDS] != 0.0, plates.friends); @@ -2556,6 +2650,8 @@ mod tests { alpha_ref: 0.5, fade_far: 70.0, }; + let mut weather = benilla_world::weather::WeatherState::default(); + let mut display_gamma = crate::ui_gamma::DisplayGamma::default(); let mut minimap = MinimapZoom::default(); let mut bubbles = BubbleConfig::default(); let mut zoom = ZoomLimit::default(); @@ -2602,6 +2698,8 @@ mod tests { names: &mut names, plates: &mut plates, clutter: &mut clutter, + weather: &mut weather, + display_gamma: &mut display_gamma, minimap: &mut minimap, bubbles: &mut bubbles, zoom: &mut zoom, @@ -2800,6 +2898,23 @@ mod tests { // The row `GetCVar` answers is the same field seen the other way round. assert!(apply_to_knobs("WorldDetail", "1", &mut knobs)); assert_eq!(knobs.clutter.density, 2.0); + // Weather Intensity (2181): the panel's four stops land whole, an off-grid value + // truncates toward zero the way every int-valued row here does, and both ends clamp. + for (wrote, want) in [ + ("0", 0u8), + ("1", 1), + ("2", 2), + ("3", 3), + ("2.9", 2), + ("9", 3), + ("-4", 0), + ] { + assert!(apply_to_knobs("weatherDensity", wrote, &mut knobs)); + assert_eq!( + knobs.weather.weather_density, want, + "weatherDensity {wrote}" + ); + } assert_eq!(knobs.clutter.frill_density(), 32.0); // And the pair is NAMED as a pair, in the registered spelling and the lowercased one, so // `$WOW_CLUTTER_DENSITY` cannot take one spelling of this knob for the session and leave @@ -2828,6 +2943,13 @@ mod tests { assert!(REGISTERED .iter() .any(|r| r.name == benilla_ui::script::CVAR_FRILL_DENSITY)); + // …and the display-gamma pair's (2182), for exactly the same reason: `GetGamma` answers + // `1 - ` and `SetGamma` writes `1 - v` into it, both from a crate that cannot + // see this table, so an unregistered name would make the getter answer a constant 0 and + // the setter write into nothing. + assert!(REGISTERED + .iter() + .any(|r| r.name == benilla_ui::script::CVAR_GAMMA)); // **`RestoreVideoDefaults`' row list, welded the same way** (2177). It lives in // `benilla-ui` beside the binding that walks it and cannot see this table, so a rename or // a retirement here would turn one of its rows into a silent skip — the verb would restore @@ -2986,6 +3108,8 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .init_resource::() diff --git a/crates/benilla-app/src/player.rs b/crates/benilla-app/src/player.rs index d681a375..3171fd31 100644 --- a/crates/benilla-app/src/player.rs +++ b/crates/benilla-app/src/player.rs @@ -111,8 +111,7 @@ use controller::control; // `/follow` (decision 0890): chat asks with the message, `crate::target` resolves the subject into // the state, and this module owns the motion. use camera::{ - apply_zoom_scroll, model_pivot_height, run_look_session, CameraProbe, FlyCam, LookButton, - CAM_COLLISION_RADIUS, CAM_DIST_DEFAULT, + apply_zoom_scroll, model_pivot_height, run_look_session, FlyCam, LookButton, CAM_DIST_DEFAULT, }; pub(crate) use camera::{head_height, CameraControl, CameraPivot}; pub(crate) use follow::{FollowRequest, FollowState}; @@ -291,7 +290,7 @@ pub(super) type TransportQuery<'w, 's> = Query< /// before the controller reads them orders against this — the two scripted probe drivers do /// (`capture::probe_look` / `capture::probe_cam`, decision 1174). A set rather than the `control` /// symbol itself: an instrument may name the gameplay system it runs against, but exporting -/// `control` would drag its private parameter types (`MoveSpeed`, `CameraProbe`, `PressGesture`) +/// `control` would drag its private parameter types (`MoveSpeed`, `PlayerCapsule`, `PressGesture`) /// out with it, which is exactly the internals-publishing 1173 rejected a crate wall to avoid. #[derive(SystemSet, Debug, Clone, PartialEq, Eq, Hash)] pub(crate) struct PlayerControlSet; diff --git a/crates/benilla-app/src/player/camera.rs b/crates/benilla-app/src/player/camera.rs index 347721be..507d6d5c 100644 --- a/crates/benilla-app/src/player/camera.rs +++ b/crates/benilla-app/src/player/camera.rs @@ -10,8 +10,6 @@ use bevy::mesh::MeshTag; use bevy::prelude::*; use bevy::window::{CursorGrabMode, CursorOptions, PrimaryWindow}; -use avian3d::prelude::*; - use super::camera_channel::{Arm, SmoothChannel}; use super::camera_dynamics::{DynamicsInput, HeadBob, SmartPivot, TerrainTilt}; use crate::creature_anim::wrap_pi; @@ -628,11 +626,6 @@ impl FollowRig { /// 1.5533430576 rad; the pitch integrate `FUN_00510120`, wow-re `follow-camera`). A single uniform /// clamp at every zoom level — the reference has **no** distinct first-person look-down limit. pub(super) const CAM_PITCH_LIMIT: f32 = 89.0 * std::f32::consts::PI / 180.0; -/// Camera-collision probe radius (yd): a small sphere swept from the camera pivot toward the desired -/// camera seat each frame. Its radius is the margin kept between the camera and the surface it stops -/// at, so the near plane doesn't poke through the wall. Smaller than the player capsule — the camera -/// threads gaps the body can't fit. -pub(super) const CAM_COLLISION_RADIUS: f32 = 0.3; /// How fast the camera glides back out to the player's chosen zoom once an obstruction clears (1/s). /// Pull-*in* is instant (a wall must never sit between the camera and the character); only the /// push-*out* eases — the vanilla feel of the camera snapping close past an obstacle and easing back. @@ -778,12 +771,6 @@ impl PivotGlide { } } -/// A small sphere swept from the camera pivot toward the desired camera seat each frame to keep walls -/// from sliding between the camera and the character (camera collision). Built once at startup like -/// [`PlayerCapsule`]; smaller than the body capsule so the camera can thread gaps the player can't. -#[derive(Resource)] -pub(super) struct CameraProbe(pub(super) Collider); - /// Which mouse button is driving mouse-look, if any — the two vanilla look modes. While looking, the /// OS cursor is hidden + locked in place (relative motion drives the camera); `cursor_stash` is the /// position it's restored to on release so it reappears exactly where the user pressed. `pub(crate)` @@ -1250,7 +1237,6 @@ pub(super) fn seat_on_subject( cam: &mut FlyCam, cam_t: &mut Mut, collide: &benilla_world::collision::WorldCollision<'_, '_>, - cam_probe: &Collider, follow: &FollowInput, dynamics: &DynamicsInput, ) { @@ -1378,7 +1364,6 @@ pub(super) fn seat_on_subject( cam, cam_t, collide, - cam_probe, follow, dynamics, ); @@ -1408,7 +1393,6 @@ pub(super) fn seat_camera( cam: &mut FlyCam, cam_t: &mut Mut, collide: &benilla_world::collision::WorldCollision<'_, '_>, - cam_probe: &Collider, follow: &FollowInput, dynamics: &DynamicsInput, ) { @@ -1486,19 +1470,19 @@ pub(super) fn seat_camera( // the chunk's four MCLQ slots. Two-sided, and with no near floor — which is precisely why the // pivot corridor above is not optional: nothing in the trace itself stops a boom that starts // on the water plane, so the *origin* is what has to be lifted clear. - let hit = collide.cast_camera( - cam_probe, - head, - Quat::IDENTITY, - boom, - 0.0, - dynamics.options.water_collision, - ); + // + // **And that water leg is a RAY, not this probe sphere** (decision 2185): `0x7c2c40` is a + // ray/triangle test and `0x672170` carries no radius, so the `2/9` yd the corridor lifts the + // origin by is a clearance budgeted for a point. The probe is `0.3` — it does not fit, and a + // level boom behind a surface swimmer was coming back pinned at zero. The split lives in + // [`benilla_world::collision::WorldCollision::cast_camera`], which owns the probe now; the + // sphere still sweeps the solid world, which is what it was always for. + let hit = collide.cast_camera(head, boom, dynamics.options.water_collision); // The solver's own clip verdict (`0x50e570`'s `0x30000` return, OR'd into `[cam+0x90]` by the // driver) — [`SmartPivot`]'s sixth conjunct, and the reason an unobstructed camera never // pivots. Written here because here is the only place that knows. rig.clipped = hit.is_some(); - let open = hit.map_or(boom_len, |h| h.distance); + let open = hit.unwrap_or(boom_len); // Snap in instantly when geometry intrudes (a wall must never sit between camera and character); // ease back out to the open arm length once it clears — the vanilla snap-close-then-glide-back. rig.collision_distance = if open < rig.collision_distance { diff --git a/crates/benilla-app/src/player/controller.rs b/crates/benilla-app/src/player/controller.rs index b2d4198e..0676fcc0 100644 --- a/crates/benilla-app/src/player/controller.rs +++ b/crates/benilla-app/src/player/controller.rs @@ -88,7 +88,6 @@ pub(super) fn control( speed_capsule: ( Res, Res, - Res, Res, Res, Res, @@ -155,19 +154,18 @@ pub(super) fn control( let mouse_motion = &pointer.0; let look_cfg = *pointer.1; let zoom_max = pointer.2.max; - let (move_speed, capsule, cam_probe, inspect, ui_capture, click_consumed) = ( + let (move_speed, capsule, inspect, ui_capture, click_consumed) = ( &speed_capsule.0, &speed_capsule.1 .0, - &speed_capsule.2 .0, + &speed_capsule.2, &speed_capsule.3, &speed_capsule.4, - &speed_capsule.5, ); - let binds = &speed_capsule.6; - let view_subject = &speed_capsule.7; - let self_guid = speed_capsule.8 .0; - let scoped = &speed_capsule.9; - let covered = speed_capsule.10.covering(); + let binds = &speed_capsule.5; + let view_subject = &speed_capsule.6; + let self_guid = speed_capsule.7 .0; + let scoped = &speed_capsule.8; + let covered = speed_capsule.9.covering(); // The auto-follow knobs (decisions 1493/1502), with far sight's one exception folded in here so // both camera seats below agree: while the rig orbits somebody ELSE's body (Mind Vision, Sentry // Totem), our own facing is not what "behind" means, so the return is forced off rather than @@ -545,7 +543,6 @@ pub(super) fn control( &mut cam, &mut cam_t, &collide, - cam_probe, &camera::FollowInput { cfg: follow_cfg, face_yaw: player.face_yaw, @@ -1123,7 +1120,6 @@ pub(super) fn control( &mut cam, &mut cam_t, &collide, - cam_probe, &follow, &dynamics, ); diff --git a/crates/benilla-app/src/player/setup.rs b/crates/benilla-app/src/player/setup.rs index 0346de66..0bfda121 100644 --- a/crates/benilla-app/src/player/setup.rs +++ b/crates/benilla-app/src/player/setup.rs @@ -15,8 +15,8 @@ use benilla_world::terrain_stream::SPAWN_XY; use benilla_world::view::{WorldCamera, CAM_FAR, CAM_FOVY, NEARCLIP_DEFAULT}; use super::{ - CameraControl, CameraProbe, FlyCam, MoveSpeed, Player, PlayerCapsule, CAM_COLLISION_RADIUS, - CAM_DIST_DEFAULT, CAPSULE_HEIGHT, CAPSULE_RADIUS, + CameraControl, FlyCam, MoveSpeed, Player, PlayerCapsule, CAM_DIST_DEFAULT, CAPSULE_HEIGHT, + CAPSULE_RADIUS, }; /// Default avatar speed in yards/second — the **VERIFIED** vanilla run speed (`MOVE_RUN` 7.0). Ctrl @@ -70,7 +70,6 @@ pub(super) fn setup_player( CAPSULE_RADIUS, CAPSULE_HEIGHT - 2.0 * CAPSULE_RADIUS, ))); - commands.insert_resource(CameraProbe(Collider::sphere(CAM_COLLISION_RADIUS))); commands.insert_resource(CameraControl { distance: CAM_DIST_DEFAULT, target_distance: CAM_DIST_DEFAULT, diff --git a/crates/benilla-app/src/shaders/ui_gamma.wgsl b/crates/benilla-app/src/shaders/ui_gamma.wgsl index b7dab5ba..8e31a269 100644 --- a/crates/benilla-app/src/shaders/ui_gamma.wgsl +++ b/crates/benilla-app/src/shaders/ui_gamma.wgsl @@ -1,5 +1,6 @@ // The UI gamma lane's ONE decode (decision 0254) — the twin of `ffx_glow.wgsl`'s combine, which owns -// the world lane's single decode (0161). +// the world lane's single decode (0161) — and the client's display-gamma ramp (2182), which rides in +// front of it because the value this pass samples is exactly the byte the RAMDAC would have read. // // `ui_quad.wgsl` composites the whole UI in gamma bytes, like the reference's fixed-function // backbuffer. The UI camera's target is `Rgba8UnormSrgb`, so a stored byte round-trips through the @@ -13,6 +14,8 @@ @group(0) @binding(0) var screen_texture: texture_2d; @group(0) @binding(1) var screen_sampler: sampler; +// `.x` = the `gamma` CVar. The rest is padding — a uniform struct is 16-byte aligned. +@group(0) @binding(2) var ramp: vec4; // sRGB → linear (IEC 61966-2-1), the exact inverse of `ui_quad.wgsl`'s `linear_to_srgb`. fn srgb_to_linear(c: vec3) -> vec3 { @@ -24,5 +27,16 @@ fn srgb_to_linear(c: vec3) -> vec3 { @fragment fn fs_decode(in: FullscreenVertexOutput) -> @location(0) vec4 { let ui = textureSample(screen_texture, screen_sampler, in.uv); - return vec4(srgb_to_linear(ui.rgb), ui.a); + var rgb = ui.rgb; + // The reference's hardware ramp `ramp[i] = pow(i/255, gamma)` (`0x591680`), in its continuous + // form: `i/255` IS `rgb` here, so this is the same curve over the same values. + // + // **The branch is not an optimisation.** It is uniform across the draw, so it costs nothing — + // and at the registered default the pass has to be the exact identity it was before this + // existed, which `pow` is not: `pow(x, 1.0)` compiles to `exp2(log2(x))` and `log2(0)` is + // undefined. The floor inside the taken arm is the same guard for a non-default gamma. + if (ramp.x != 1.0) { + rgb = pow(max(rgb, vec3(1e-6)), vec3(ramp.x)); + } + return vec4(srgb_to_linear(rgb), ui.a); } diff --git a/crates/benilla-app/src/ui_chat.rs b/crates/benilla-app/src/ui_chat.rs index 04d05747..6810055d 100644 --- a/crates/benilla-app/src/ui_chat.rs +++ b/crates/benilla-app/src/ui_chat.rs @@ -45,7 +45,7 @@ mod logging; mod recruitment; /// The chat windows' saved look (B246, decision 1589) — where the tab menu's tint/alpha/font-size /// picks are read from at login and written back at logout. -mod settings; +pub(crate) mod settings; #[cfg(test)] mod tests; diff --git a/crates/benilla-app/src/ui_chat/settings.rs b/crates/benilla-app/src/ui_chat/settings.rs index 8d445ff1..996c200e 100644 --- a/crates/benilla-app/src/ui_chat/settings.rs +++ b/crates/benilla-app/src/ui_chat/settings.rs @@ -755,14 +755,48 @@ fn save_chat_looks( flush(&script, &channels, &mut file, exiting); } -/// The session-end flush — `OnExit(InWorld)`, the reference's chat teardown. -fn save_on_session_end( - script: Option>, - channels: Res, - mut file: ResMut, -) { - let Some(script) = script else { return }; - flush(&script, &channels, &mut file, true); +/// The session-end flush — the reference's chat teardown (`0x499a80` from `0x490c55`), called +/// from [`crate::ui_script::end_ui_session`] beside [`crate::cvars::fold_dying_vm_cvars`]: +/// **after** the shutdown events, **before** the VM is replaced. +/// +/// **It is not an `OnExit(InWorld)` system, and that is the whole point.** `/reload` rebuilds the +/// VM without ever leaving the world (1291), so that edge does not fire for it — and nothing else +/// covers the gap, because both halves of the debounce beside it are VM-keyed: [`ChatWindowFile::dirty`] is +/// a [`VmMemo`] that resets to `false` with the new VM, and [`restore_chat_looks`] then re-seats +/// the look table from the file **on disk**. So a window dragged, resized, recoloured or renamed +/// within [`SAVE_QUIET`] of a `/reload` was written nowhere and read straight back stale: the +/// player's change was discarded with no error and no trace. +/// +/// `end_ui_session` is the one place every root meets — it is what the `OnExit(InWorld)` +/// registration runs, and what `run_pending_reload` calls — so one call here covers the logout, +/// the character switch and the reload. It also retires the hand-stated ordering this flush used +/// to need: two unconstrained systems on one `OnExit` edge are placed by the executor, and that +/// placement was measured (bevy 0.18) to move with nothing but their registration positions. +/// Being *inside* the ender is an ordering no arrangement can lose. +/// +/// A session whose UI never loaded writes nothing, by construction rather than by a guard: +/// `restored` is false for a VM that never read the character's file, and [`owes_write`] refuses +/// on it — which is the same protection against composing the player's file out of a stock table +/// that [`ChatWindowFile::dirty`] exists for. +pub(crate) fn fold_dying_vm_chat_cache(world: &mut World) { + // A world with no chat-cache state never added the plugin — a test world or a stripped + // scenario. Checked up front so the fetch below can be non-optional, exactly as + // `fold_dying_vm_cvars` checks its own. + if !world.contains_resource::() { + return; + } + // `resource_scope` rather than a `SystemState`: it lifts the file out for the call, which + // leaves the VM and the channel roster as two plain shared borrows of the world — the whole + // fetch, with no system machinery and nothing to keep in step with the signature. + world.resource_scope(|world, mut file: Mut| { + let (Some(script), Some(channels)) = ( + world.get_non_send_resource::(), + world.get_resource::(), + ) else { + return; + }; + flush(script, channels, &mut file, true); + }); } pub(super) fn plugin(app: &mut App) { @@ -780,21 +814,11 @@ pub(super) fn plugin(app: &mut App) { .add_systems( Update, save_chat_looks.run_if(in_state(crate::char_select::ClientState::InWorld)), - ) - .add_systems( - OnExit(crate::char_select::ClientState::InWorld), - // **Explicitly before the session ender, because the edge is not an ordering.** This - // flush composes the file out of the DYING VM, and `end_ui_session` replaces that VM - // with a fresh boot one on the same edge. Two unconstrained systems there are placed - // by the executor, and that placement was measured (bevy 0.18, five systems on one - // `OnExit`) to move with nothing but their registration positions — a VM-reading - // saver ran after the exclusive ender in one arrangement and before it in another. - // Winning that race by luck is what B353 cost the layout cache one module over; the - // layout cache answered it by joining the reference's shutdown tail, which this file - // cannot do (its reference is the type-7 chat-cache saver `0x499a80`, not - // `0x490bd0`'s tail), so it says the order instead. - save_on_session_end.before(crate::ui_script::end_ui_session), ); + // **The session-end flush is not registered here.** It is called from inside + // `ui_script::end_ui_session` ([`fold_dying_vm_chat_cache`]) so that it covers `/reload` — a + // VM rebuild that never crosses `OnExit(InWorld)` — as well as the logout edge, and so that + // its position relative to the ender is structural instead of a registration-order bet. // The quit flush rides the exit edge rather than `Update` for decision 1528's reason: the // close button's `AppExit` is not written until `PostUpdate`, so a save chained beside the // watcher would lose the last second of drags to the process ending. @@ -1206,4 +1230,115 @@ mod tests { let got = parse("ADDEDVERSION 2\nWINDOW 1\nMESSAGES\nEND\nEND\n", &rows()); assert!(got.looks[0].1.messages.is_empty()); } + + /// **The `/reload` flush** (the reload half of decision 1290's class): a window renamed in the + /// last second before a `ReloadUI()` must reach the player's file. + /// + /// Against the pre-fix shape this fails, and it is worth saying exactly why, because all three + /// of the mechanisms that look like they should cover it are VM-keyed and reset together: + /// `save_on_session_end` hung on `OnExit(InWorld)`, which `run_pending_reload` never crosses + /// (1291); the debounce beside it had not fired, because [`SAVE_QUIET`] had not elapsed; and + /// the new VM's [`ChatWindowFile::dirty`] memo came up `false` while [`restore_chat_looks`] + /// re-seated the look table from the file **on disk**. So the rename was discarded with no + /// error — and the assertion on the rebuilt VM below is the half the player would actually + /// see: the window comes back under its old name. + #[test] + fn a_window_renamed_just_before_a_reload_reaches_the_file() { + use bevy::ecs::system::RunSystemOnce; + + let _l = crate::local_state::test_env::ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let tmp = std::env::temp_dir().join(format!("benilla-chat-reload-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&tmp); + std::fs::create_dir_all(tmp.join("benilla-config")).expect("hermetic home"); + let _capture = crate::local_state::test_env::EnvGuard::unset("WOW_CAPTURE"); + let _home = crate::local_state::test_env::EnvGuard::set( + "BENILLA_HOME", + tmp.join("benilla-config") + .to_str() + .expect("utf-8 temp path"), + ); + + let mut world = World::new(); + world.init_resource::(); + world.init_resource::(); + world.init_resource::(); + world.init_resource::(); + world.init_resource::(); + crate::ui_script::setup_script(&mut world); + world.insert_resource(crate::char_select::Roster::with_pending_pick( + vec![benilla_protocol::Character { + guid: 1, + name: "Reloadprobe".into(), + race: 1, // Human → Alliance + class: 1, // Warrior + gender: 0, + level: 60, + skin: 0, + face: 0, + hair_style: 0, + hair_color: 0, + facial_hair: 0, + zone: 0, + map: 0, + position: benilla_protocol::wire::Vector3d { + x: 0.0, + y: 0.0, + z: 0.0, + }, + flags: 0, + equipment: [benilla_protocol::CharEnumItem::default(); 19], + pet_display_id: 0, + pet_level: 0, + pet_family: 0, + }], + 1, + )); + crate::ui_script::load_ingame_ui_on_world_entry(&mut world); + + let path = world + .resource::() + .path + .clone() + .expect("the character's chat cache path is seated by the entry load"); + + // The player renames window 1 — a Lua write, exactly what the debounce exists to coalesce. + world + .non_send_resource_mut::() + .run(r#"SetChatWindowName(1, "Reloaded")"#) + .expect("rename"); + world + .run_system_once(watch_chat_looks) + .expect("the watcher arms the debounce"); + assert!( + world.resource::().last_change.is_some(), + "precondition: the rename armed the debounce, so the write is owed but not yet due" + ); + + // `/reload`, immediately — inside `SAVE_QUIET`, which is the whole window of the bug. + world.insert_resource(State::new(crate::char_select::ClientState::InWorld)); + world.resource_mut::().0 = true; + crate::ui_script::run_pending_reload(&mut world); + + let on_disk = std::fs::read_to_string(&path).expect("the flush wrote the file"); + assert!( + on_disk.contains("Reloaded"), + "the rename must survive the reload — the dying VM's cache is folded out by \ + `fold_dying_vm_chat_cache`; file was:\n{on_disk}" + ); + + // And the half the player sees: the rebuilt VM read it back. + let name = world + .non_send_resource_mut::() + .eval::("GetChatWindowInfo(1)") + .unwrap_or_default(); + assert_eq!( + name, "Reloaded", + "the rebuilt VM restored the renamed window, not the stale one" + ); + + drop(world); + let _ = std::fs::remove_dir_all(&tmp); + } } diff --git a/crates/benilla-app/src/ui_dialog_verbs.rs b/crates/benilla-app/src/ui_dialog_verbs.rs index 5993b112..655cf0f8 100644 --- a/crates/benilla-app/src/ui_dialog_verbs.rs +++ b/crates/benilla-app/src/ui_dialog_verbs.rs @@ -495,14 +495,42 @@ fn feed_meeting_stone( } } -/// The enter-world bring-up's meeting-stone leg: the text reset, then the empty `CMSG 0x296` -/// — once per world session, which is what the reference's run-once byte amounts to. +/// The enter-world bring-up's meeting-stone leg: the text reset, then the empty `CMSG 0x296`. +/// +/// **Once per UI LIFECYCLE, not once per world session**, and that distinction is the whole bug. +/// The reference's run-once byte `[0xb4b424]` is *cleared by the UI teardown* — `0x490bd0` calls +/// `0x490a80` at `0x490c20`, which zeroes it at `0x490a8d` — and `UI_Init` (`0x48fbf0`) then calls +/// `0x4908c0` at `0x490168` behind the same live-player gate, finds the byte clear, and re-runs +/// this entire bring-up: `0x490a14` → `0x4c9f40` → `0x4ca1c0` → `PutUInt32(0x296)` + Send. So a +/// `ReloadUI()` **re-asks the server**, and that is what brings the icon back. +/// +/// Gated on `MessageReader`, this leg never ran for a `/reload`, which never +/// leaves the world (1291) and so produces no such message. Nothing re-armed +/// [`MeetingStone::dirty`] and nothing re-queried, so `IsInMeetingStoneQueue()` and +/// `GetMeetingStoneStatusText()` answered nil for the rest of the session — and stock +/// `Minimap.xml`'s `MiniMapMeetingStoneFrame` (built `hidden="true"`, shown only by +/// `MEETINGSTONE_CHANGED`, whose single firing site image-wide is the `0x295` handler at +/// `0x4ca38f`) stayed gone while the player was still queued. +/// +/// **A [`crate::ui_script::VmMemo`] claim IS the reference's gate.** A byte the UI teardown clears +/// is exactly "once per VM", and 1290's name for that is `claim` — so this reads as the same +/// question the binary asks, rather than as a workaround for the missing message. The one +/// round trip during which the icon is genuinely absent is faithful, not a defect: the reference +/// has the same gap, because only the server's reply fires the event. +/// +/// The queued area itself is untouched here, matching `[0xb72038]`, which the RE round found is +/// referenced six times image-wide and by nothing in either reload closure — it survives, and +/// [`MeetingStone::enter_world`]'s `dirty` is what re-pushes it to the fresh VM. fn meeting_stone_enter_world( - mut entered: MessageReader, + script: Option>, mut stone: ResMut, commands: Res, + mut asked: Local>, ) { - if entered.read().next().is_none() { + let Some(script) = script else { + return; + }; + if !asked.claim(&script) { return; } stone.enter_world(); @@ -673,7 +701,11 @@ impl Plugin for UiDialogVerbsPlugin { ( close_npc_session_out_of_range::.before(feed_dialog_verbs), feed_dialog_verbs.before(UiInput), - meeting_stone_enter_world.before(feed_meeting_stone), + // In-world only: the claim is about the VM, but the query is a world + // packet, and the boot VM exists at the glue screen too. + meeting_stone_enter_world + .before(feed_meeting_stone) + .run_if(in_state(crate::char_select::ClientState::InWorld)), feed_meeting_stone.before(UiInput), drain_latch_verbs.after(UiInput), drain_queue_verbs.after(UiInput), @@ -704,6 +736,74 @@ fn cancel_gate_could_apply(attributes_ex: u32) -> bool { mod tests { use super::*; + /// **The `/reload` re-query** — the meeting-stone half of 1290's class, and the reference's + /// own behaviour rather than an invention of ours. + /// + /// wow-re's §5 trio (3/3 unanimous, every byte re-decoded from the raw image) settled that + /// the run-once byte `[0xb4b424]` is cleared by the UI teardown at `0x490a8d` and that + /// `UI_Init` re-runs the bring-up and re-sends `CMSG 0x296`. So the gate is once per **VM**, + /// not once per world session — and against the old `MessageReader` + /// shape this fails on the second VM, which is exactly the reported symptom: the queued + /// player's minimap icon never comes back after a `/reload`. + /// + /// It also corrects wow-re's own `meeting-stone-status.md` §6, which asserted the query "is + /// sent exactly once per world session … has no other trigger" — true of the entry points it + /// enumerated, but it never asked who *clears* the byte. + /// + /// **A registered schedule, not `run_system_once`**: the gate is a `Local>`, and + /// `run_system_once` builds a fresh system — and so a fresh `Local` — on every call, which + /// would make the claim look unclaimed every frame (the same trap `ui_loot`'s tests name). + #[test] + fn a_rebuilt_vm_re_asks_the_server_for_the_meeting_stone_queue() { + let (tx, rx) = crossbeam_channel::unbounded(); + let mut app = App::new(); + app.init_resource::() + .insert_resource(NetCommands(tx)) + .add_systems(Update, meeting_stone_enter_world); + + let queries = |rx: &crossbeam_channel::Receiver| { + rx.try_iter() + .filter(|c| matches!(c, ClientCommand::MeetingStoneStatusQuery)) + .count() + }; + + // No VM yet — the glue screen: nothing is asked. + app.update(); + assert_eq!(queries(&rx), 0, "no VM, no bring-up"); + + // The first VM: the bring-up runs once, however many frames pass. + app.insert_non_send_resource(UiScript::new().expect("VM")); + app.update(); + assert_eq!(queries(&rx), 1, "the first VM asks the server"); + app.update(); + app.update(); + assert_eq!(queries(&rx), 0, "…and does not ask again on later frames"); + + // The server answers: the player IS queued, and the host holds that. + app.world_mut().resource_mut::().area = 1519; + app.world_mut().resource_mut::().dirty = false; + + // `ReloadUI()`: a fresh VM, and nothing on the wire. + app.insert_non_send_resource(UiScript::new().expect("VM")); + app.update(); + assert_eq!( + queries(&rx), + 1, + "a rebuilt VM re-asks — the reference's UI teardown clears the run-once byte, so \ + `UI_Init` re-sends `CMSG 0x296` (0x490a8d / 0x490168)" + ); + + let stone = app.world().resource::(); + assert!( + stone.dirty, + "the bring-up re-armed the push, so the fresh VM is told the queued area again" + ); + assert_eq!( + stone.area, 1519, + "the queued area itself is untouched, matching `[0xb72038]` surviving the reload" + ); + } + /// Spell 2584's flags off the shipped Spell.dbc: the cancel-aura gate's first two legs. #[test] fn spell_2584_never_trips_the_cancel_gate() { diff --git a/crates/benilla-app/src/ui_gamma.rs b/crates/benilla-app/src/ui_gamma.rs index 0c6e7405..c5d6cd1d 100644 --- a/crates/benilla-app/src/ui_gamma.rs +++ b/crates/benilla-app/src/ui_gamma.rs @@ -29,9 +29,9 @@ use bevy::render::extract_component::{ExtractComponent, ExtractComponentPlugin}; use bevy::render::render_graph::{ NodeRunError, RenderGraphContext, RenderGraphExt, RenderLabel, ViewNode, ViewNodeRunner, }; -use bevy::render::render_resource::binding_types::{sampler, texture_2d}; +use bevy::render::render_resource::binding_types::{sampler, texture_2d, uniform_buffer_sized}; use bevy::render::render_resource::*; -use bevy::render::renderer::{RenderContext, RenderDevice}; +use bevy::render::renderer::{RenderContext, RenderDevice, RenderQueue}; use bevy::render::view::ViewTarget; use bevy::render::{RenderApp, RenderStartup}; use bevy::ui_render::graph::NodeUi; @@ -40,9 +40,73 @@ use bevy::ui_render::ui_texture_slice_pipeline::{ }; use bevy::ui_render::{init_ui_pipeline, UiPipeline}; -/// Marks the camera whose target holds a gamma-composited image awaiting this pass's one decode. -#[derive(Component, Clone, Copy, Default, ExtractComponent)] -pub(crate) struct UiGammaLane; +/// Marks the camera whose target holds a gamma-composited image awaiting this pass's one decode, +/// and carries the [`DisplayGamma`] exponent that decode applies on the way through (2182). +#[derive(Component, Clone, Copy, ExtractComponent)] +pub(crate) struct UiGammaLane { + /// The `gamma` CVar's value, ready for the shader. `1.0` is the identity ramp. + pub(crate) gamma: f32, +} + +impl Default for UiGammaLane { + fn default() -> Self { + Self { + gamma: DEFAULT_GAMMA, + } + } +} + +/// **The display-brightness correction** (decision 2182) — the `gamma` CVar, as the reference +/// registers it (`0x402d70`, name `0x82e924`, default string `0x82e92c` = `"1.0"`, flags 0). +/// +/// The reference applies it as an OS **hardware gamma ramp**: its change callback `0x4034d0` +/// builds `ramp[i] = __ftol(pow(i · 1/255, gamma) · 65535)` at `0x591680` and hands the 3×256 words +/// to `GDI32!SetDeviceGammaRamp` (wow-re `ffxeffects/scratch/whole-frame-grade-verdict.md` §(a); +/// the same note proves there is no other whole-frame grade in the client, and that +/// `Brightness`/`Contrast` CVars do not exist in the binary at all). That upload is **skipped +/// windowed** — `byte[dev+0x20b]` is `CGxFormat +0x07`, which is `gxWindow` — and windowed is every +/// mode benilla has (1627), so copying the mechanism byte for byte would ship a slider that never +/// moves a pixel. +/// +/// So the ramp goes where a ramp goes when you own the compositor: **the same curve, applied to the +/// same values, one stage later.** `i/255` is the framebuffer byte the RAMDAC would have read; +/// [`UiGammaNode`] samples exactly that value (the UI lane composites in gamma bytes — the module +/// doc above) and raising it to `gamma` before the one decode is the continuous form of the +/// reference's 256-entry LUT. +#[derive(Resource, Debug, Clone, Copy, PartialEq)] +pub(crate) struct DisplayGamma(pub(crate) f32); + +/// The reference's registered `"1.0"` — the identity ramp, and the value the pass is a **no-op** +/// at, byte for byte (see [`UiGammaNode`]'s uniform branch). +pub(crate) const DEFAULT_GAMMA: f32 = 1.0; + +/// **The clamp is ours, and the reference has none** — `SetGamma(5)` writes `gamma = "-4.000000"` +/// there with no arm anywhere to catch it (wow-re `ui/scratch/video-options-verbs.md` §3, with +/// `baseMip`'s validating callback `0x689090` as the positive control). The reference can afford +/// that because its ramp is a fullscreen-only OS call a player can escape by alt-tabbing; ours is +/// the image itself, and `pow(x, 12)` is a black screen with the panel that undoes it somewhere +/// inside it. This range is the widest that keeps the client legible enough to reach that panel — +/// it spans the stock slider's own `[0.5, 1.5]` four times over, so nothing a player can do from +/// the UI ever meets it, and the CVar still keeps whatever truth was written to it (0959's posture: +/// the consumer clamps at its own edge, the store does not lie). +pub(crate) const GAMMA_RANGE: std::ops::RangeInclusive = 0.25..=4.0; + +impl Default for DisplayGamma { + fn default() -> Self { + Self(DEFAULT_GAMMA) + } +} + +/// Carry the knob onto the camera the pass runs on. Change-detected on the resource, so a settled +/// session writes nothing. +fn stamp_lane_gamma(gamma: Res, mut lanes: Query<&mut UiGammaLane>) { + if !gamma.is_changed() { + return; + } + for mut lane in &mut lanes { + lane.gamma = gamma.0.clamp(*GAMMA_RANGE.start(), *GAMMA_RANGE.end()); + } +} #[derive(Debug, Hash, PartialEq, Eq, Clone, RenderLabel)] struct UiGammaLabel; @@ -52,6 +116,10 @@ struct UiGammaPipeline { layout: BindGroupLayoutDescriptor, sampler: Sampler, decode: CachedRenderPipelineId, + /// The ramp exponent's 16-byte uniform, rewritten each frame with a queue write (which lands + /// before the graph's submit executes) — the same shape `ffx_glow`'s combine uses. One buffer, + /// not one per view: [`UiGammaLane`] is on exactly one camera. + ramp: Buffer, } fn init_pipeline( @@ -68,6 +136,9 @@ fn init_pipeline( ( texture_2d(TextureSampleType::Float { filterable: true }), sampler(SamplerBindingType::Filtering), + // The ramp exponent, as a `vec4` — one scalar, but a uniform struct is + // 16-byte aligned and a vec4 says so without a padding field to keep in step. + uniform_buffer_sized(false, Some(std::num::NonZero::new(16).unwrap())), ), ), ); @@ -97,10 +168,17 @@ fn init_pipeline( }), ..default() }); + let ramp = render_device.create_buffer(&BufferDescriptor { + label: Some("ui_gamma_ramp"), + size: 16, + usage: BufferUsages::UNIFORM | BufferUsages::COPY_DST, + mapped_at_creation: false, + }); commands.insert_resource(UiGammaPipeline { layout, sampler, decode, + ramp, }); } @@ -114,7 +192,7 @@ impl ViewNode for UiGammaNode { &self, _graph: &mut RenderGraphContext, render_context: &mut RenderContext<'w>, - (view_target, _): QueryItem<'w, '_, Self::ViewQuery>, + (view_target, lane): QueryItem<'w, '_, Self::ViewQuery>, world: &'w World, ) -> Result<(), NodeRunError> { let pipelines = world.resource::(); @@ -124,12 +202,22 @@ impl ViewNode for UiGammaNode { // which beats dropping the UI entirely. return Ok(()); }; + // The ramp exponent for this frame (2182). `[g, 0, 0, 0]` — the shader reads `.x`. + world.resource::().write_buffer( + &pipelines.ramp, + 0, + bytemuck::cast_slice(&[lane.gamma, 0.0, 0.0, 0.0]), + ); let post = view_target.post_process_write(); let layout = pipeline_cache.get_bind_group_layout(&pipelines.layout); let bind = render_context.render_device().create_bind_group( "ui_gamma_decode", &layout, - &BindGroupEntries::sequential((post.source, &pipelines.sampler)), + &BindGroupEntries::sequential(( + post.source, + &pipelines.sampler, + pipelines.ramp.as_entire_binding(), + )), ); // Its diagnostic span: the journal's `gpu_ui` column counts this decode (2008). let diagnostics = render_context.diagnostic_recorder(); @@ -188,7 +276,9 @@ pub(crate) struct UiGammaPlugin; impl Plugin for UiGammaPlugin { fn build(&self, app: &mut App) { - app.add_plugins(ExtractComponentPlugin::::default()); + app.init_resource::() + .add_plugins(ExtractComponentPlugin::::default()) + .add_systems(Update, stamp_lane_gamma); let Some(render_app) = app.get_sub_app_mut(RenderApp) else { return; }; diff --git a/crates/benilla-app/src/ui_pass.rs b/crates/benilla-app/src/ui_pass.rs index 3431f857..95b85efe 100644 --- a/crates/benilla-app/src/ui_pass.rs +++ b/crates/benilla-app/src/ui_pass.rs @@ -633,7 +633,7 @@ fn spawn_ui_camera(mut commands: Commands) { bevy::render::view::Msaa::Off, // The gamma composite lane's mandatory decode (decision 0254) — without it the UI presents // ~2.2× bright, since the quad pass leaves gamma values in the target. - crate::ui_gamma::UiGammaLane, + crate::ui_gamma::UiGammaLane::default(), // Every Bevy UI tree renders HERE (decision 0541) — the glue screens and the loading screen. // Without the marker, Bevy UI picks the highest-order camera targeting the window, which is // the egui dev overlay (order 2): the glue screens rode the dev camera, outside the gamma diff --git a/crates/benilla-app/src/ui_script/lifecycle.rs b/crates/benilla-app/src/ui_script/lifecycle.rs index b12ff93b..097db0db 100644 --- a/crates/benilla-app/src/ui_script/lifecycle.rs +++ b/crates/benilla-app/src/ui_script/lifecycle.rs @@ -668,6 +668,11 @@ pub(crate) fn end_ui_session(world: &mut World) { // that lands in an engine-side store that survives), before the VM is replaced. The next // VM's registration seeds from what this writes ([`crate::cvars`]'s saved base). crate::cvars::fold_dying_vm_cvars(world); + // The chat cache, on the same terms and for the same reason (decision NNNN): it composes the + // player's file out of the DYING VM, and `/reload` never crosses the `OnExit(InWorld)` edge + // its flush used to hang on — so a window moved in the last second before a reload was + // written nowhere and re-read stale from disk. + crate::ui_chat::settings::fold_dying_vm_chat_cache(world); world.insert_resource(AddOnIdentity(None)); // **Everything the host is holding that came OUT of the dying VM goes with it.** A change memo diff --git a/crates/benilla-app/src/ui_script/options_tests.rs b/crates/benilla-app/src/ui_script/options_tests.rs index b1a26801..fb05ac7c 100644 --- a/crates/benilla-app/src/ui_script/options_tests.rs +++ b/crates/benilla-app/src/ui_script/options_tests.rs @@ -2039,6 +2039,12 @@ fn settle(s: &mut UiScript) { fn a_broad_search_scrolls_the_page_instead_of_overflowing_it() { let mut s = harness_on(audio_harness()); s.run("ShowUIPanel(BenillaOptionsFrame)").unwrap(); + // On NAMEPLATES, not the default Controls page: since 2180 seated the four camera toggles and + // the two camera-speed sliders, Controls is sixteen rows and overflows its area on its own — + // which is the scroll body doing its job, and no longer a control for "nothing overflows". + // Nameplates is six rows and fits. + s.run("BenillaOptionsFrameCategoryListRowNameplates:Click()") + .unwrap(); settle(&mut s); // Control first: a settled page fits, so there is no scroll and no bar at all — and the body @@ -2047,7 +2053,7 @@ fn a_broad_search_scrolls_the_page_instead_of_overflowing_it() { s.eval::("return BenillaOptionsFrameContainerScroll:GetVerticalScrollRange()") .unwrap(), 0.0, - "the Controls page fits its area" + "the Nameplates page fits its area" ); for f in [ "BenillaOptionsFrameContainerScrollBar", @@ -2276,6 +2282,32 @@ fn hover_label(s: &mut UiScript, frame: &str) { s.mouse_move(l + 60.0, (t + b) * 0.5); } +/// Bring `frame` inside the page's scroll rect, if the page scrolls at all. A page longer than +/// its area clips everything past the fold (B217), so a row down there cannot be hovered where its +/// rect says it is — the cursor lands outside the scroll frame and hits nothing. Nine of the +/// Controls page's sixteen rows and the last of Combat's seventeen live there since 2180. +fn scroll_into_view(s: &mut UiScript, frame: &str) { + s.resolve(); + s.run(&format!( + "local sf = BenillaOptionsFrameContainerScroll \ + local range = sf:GetVerticalScrollRange() \ + if range and range > 0 then \ + local row = getglobal(\"{frame}\") \ + local want = sf:GetVerticalScroll() \ + if row:GetBottom() < sf:GetBottom() then \ + want = want + (sf:GetBottom() - row:GetBottom()) + 2 \ + elseif row:GetTop() > sf:GetTop() then \ + want = want - (row:GetTop() - sf:GetTop()) - 2 \ + end \ + if want < 0 then want = 0 end \ + if want > range then want = range end \ + sf:SetVerticalScroll(want) \ + end" + )) + .unwrap(); + s.resolve(); +} + /// A hovered row raises its 1.12 description, on the era's seat, and drops it on leave — the row /// itself, its checkbox, and (B223's report) the label the cursor actually crosses. The string /// resolves by KEY at hover, so seeding it AFTER the window loaded still paints: that is the @@ -2433,11 +2465,13 @@ fn every_row_tooltip_key_resolves_in_the_real_global_strings() { continue; } // The deliberate exceptions (1639 Render Scale, 1650 Display Mode, 1847 Enable Sound in - // Background). None has a 1.12 counterpart whose `OPTION_TOOLTIP_*` could be resolved — - // Render Scale has no era row at all, Display Mode's era row was a CHECKBOX whose string - // says "Check to…", and 1.12 has no background-sound setting at all (it mutes on its - // window-activation event and offers no way out) — and each is a row a player needs a - // description for. Each carries one under a `BENILLA_` prefix so the reference's + // Background, 2182 Brightness). None has a 1.12 counterpart whose `OPTION_TOOLTIP_*` could + // be resolved — Render Scale has no era row at all, Display Mode's era row was a CHECKBOX + // whose string says "Check to…", 1.12 has no background-sound setting at all (it mutes on + // its window-activation event and offers no way out), and `OPTION_TOOLTIP_GAMMA` spends + // its second sentence on "all 21 levels of gray bars to the right", which is the stock + // video window's own calibration art and not something this page has — and each is a row a + // player needs a description for. Each carries one under a `BENILLA_` prefix so the reference's // namespace stays the reference's, which is exactly what this guard is here to protect. // Everything the guard was built to catch — an invented or typo'd `OPTION_TOOLTIP_` key // that silently resolves to nothing — is untouched: the pairing below is exact, so a @@ -2449,6 +2483,7 @@ fn every_row_tooltip_key_resolves_in_the_real_global_strings() { "BENILLA_TOOLTIP_BACKGROUND_SOUND", "AudioRowBackgroundSound", ), + ("BENILLA_TOOLTIP_BRIGHTNESS", "GraphicsRowBrightness"), ]; if let Some((_, want_row)) = BENILLA_OWNED.iter().find(|(k, _)| *k == key) { assert_eq!(row, *want_row, "{row}: not this row's string"); @@ -2498,7 +2533,16 @@ fn every_row_tooltip_key_resolves_in_the_real_global_strings() { // key OPTION_TOOLTIP_SHOW_TUTORIALS is 1.12's own. // The 30th and 31st CVar rows are the two text filters (2077): Profanity Filter on the // Interface page and Disable Spam Filter on the Chat page, both keys 1.12's own. - assert_eq!(checked, 62, "every tipped row carries a live key"); + // 2180 added fourteen rows, every one of them tipped with the reference's own key: the + // four camera toggles and two camera-speed sliders plus Attack on assist, Auto Clear AFK and + // Auto Self Cast on Controls, Enhanced Tooltips on Interface, Player Guild Names on + // Nameplates, and the damage-number trio on Combat. 62 -> 76. + // …and Weather Intensity (2181), on the Graphics page, whose OPTION_TOOLTIP_WEATHER_DETAIL is + // 1.12's own and rides its own slider-9 row verbatim. 76 -> 77. + // …and Brightness (2182), the FOURTH row whose description is benilla's own rather than 1.12 + // GlobalStrings — OPTION_TOOLTIP_GAMMA spends its second sentence on the stock window's + // 21-step grey ramp, which this page does not have (see the guard above). 77 -> 78. + assert_eq!(checked, 78, "every tipped row carries a live key"); assert_eq!( untipped, vec![ @@ -2564,6 +2608,7 @@ fn every_flavor_of_row_raises_its_plate_from_the_page_it_lives_on() { .unwrap(); for rkey in rows.split(',') { let row = format!("BenillaOptionsFrameContainerBody{page}{rkey}"); + scroll_into_view(&mut s, &row); hover_label(&mut s, &row); s.resolve(); let tipped: bool = s.eval(&format!("return {row}.tip ~= nil")).unwrap(); @@ -2610,7 +2655,14 @@ fn every_flavor_of_row_raises_its_plate_from_the_page_it_lives_on() { // (1815), the Audio page's fourth checkbox and 1.12's own. // …and Show Tutorials (2077), the Interface page's seventh API row. // …and the two text-filter rows (2077). - assert_eq!(raised, 62, "every row but Auto Loot raises a description"); + // …and 2180's fourteen: Attack on assist, Auto Clear AFK, Auto Self Cast, Follow Terrain, + // Head Bob, Auto-Follow Speed, Water Collision, Smart Pivot and Mouse Look Speed on Controls; + // Enhanced Tooltips on Interface; Player Guild Names on Nameplates; Show Target Damage, + // Periodic Damage and Pet Damage on Combat. Every one of them tipped with the reference's + // own key, which is what makes the count move by exactly the number of rows added. + // …and Weather Intensity (2181) and Brightness (2182), the Graphics page's fourth and fifth + // sliders. + assert_eq!(raised, 78, "every row but Auto Loot raises a description"); } /// The **Combat page** (decision 1134) — the first rows in this window whose store is a @@ -4590,3 +4642,325 @@ fn every_cvar_the_reference_table_names_is_registered_or_listed_with_its_blocker "UNBACKED_REFERENCE_CVARS names CVars the reference's table does not: {phantom:?}" ); } + +/// **Nothing the reference lets a player change, and this client backs, is out of reach** +/// (decision 2180) — the other half of the census above, and the half that was missing. +/// +/// The test above asks the ENGINE's question: is the CVar registered? It was green the entire time +/// the four camera toggles had no box on any window a player can open. 2149 built their +/// mechanisms and dropped them off `UNBACKED_REFERENCE_CVARS`; 2115 loads the stock window that +/// carries their checkboxes deliberately HIDDEN, so addons find real frames and the player never +/// sees it; and our own window — the one the ESC menu opens — simply had no rows. Two records were +/// individually right and the setting was unreachable, because no check compared them. +/// +/// This asks the UI's question. A CVar that lands on the list below is one the reference lets a +/// player change and benilla does not, and it costs a stated reason at the row. +#[test] +fn every_registered_reference_cvar_has_a_row_on_our_own_window() { + let s = harness(); + + // Both of the reference's own option tables, exactly as the registration census reads them. + let named: Vec = s + .eval::>( + "local out = {} \ + for key, v in pairs(UIOptionsFrameCheckButtons) do \ + if v.cvar then table.insert(out, v.cvar) end \ + end \ + for _, v in ipairs(UIOptionsFrameSliders) do \ + if v.cvar then table.insert(out, v.cvar) end \ + end \ + table.sort(out) \ + return out", + ) + .expect("read the reference's two option tables"); + + // What OUR window can actually move. A row's own `cvar`, plus the `partner` a handful of rows + // write beside it (`PetSpellDamage`, `cameraPitchMoveSpeed`) — those are moved by a control + // the player uses, which is what "reachable" means here, even though neither is table-named. + let ours: std::collections::HashSet = s + .eval::>( + "local out = {} \ + for page, rows in pairs(OPTIONS_PAGE_ROWS) do \ + for _, rkey in ipairs(rows) do \ + local row = getglobal(\"BenillaOptionsFrameContainerBody\" .. page .. rkey) \ + if row.cvar then table.insert(out, row.cvar) end \ + if row.partner then table.insert(out, row.partner.cvar) end \ + end \ + end \ + return out", + ) + .expect("read our own window's rows") + .into_iter() + .map(|c| c.to_ascii_lowercase()) + .collect(); + + let registered: std::collections::HashSet = crate::cvars::registered_pairs() + .map(|(n, _)| n.to_ascii_lowercase()) + .collect(); + let excused: std::collections::HashSet = UNREACHABLE_REFERENCE_CVARS + .iter() + .map(|(n, _)| n.to_ascii_lowercase()) + .collect(); + + // Two floors, because the failure this check is worth having is the one where it passes for + // the wrong reason: a harness whose stock tables did not load reads no names, and one whose + // window did not build reads no rows, and either way `missing` is empty and green. + let backed = named + .iter() + .filter(|c| registered.contains(&c.to_ascii_lowercase())) + .count(); + assert!( + backed >= 28, + "the reference's tables did not load: only {backed} of their names are registered here" + ); + assert!( + ours.len() >= 40, + "our own window's rows did not build: only {} carry a cvar", + ours.len() + ); + + let missing: Vec<&str> = named + .iter() + .filter(|c| { + let k = c.to_ascii_lowercase(); + registered.contains(&k) && !ours.contains(&k) && !excused.contains(&k) + }) + .map(String::as_str) + .collect(); + assert!( + missing.is_empty(), + "these are backed and the reference lets a player change them, and our own options \ + window has no row for them: {missing:?} — seat a row, or add them to \ + UNREACHABLE_REFERENCE_CVARS with the reason" + ); + + // The other direction, the one that keeps the list from rotting: an excuse for a row that now + // exists is an excuse describing nothing. + let stale: Vec<&str> = UNREACHABLE_REFERENCE_CVARS + .iter() + .map(|(n, _)| *n) + .filter(|n| ours.contains(&n.to_ascii_lowercase())) + .collect(); + assert!( + stale.is_empty(), + "these have a row now — drop them from UNREACHABLE_REFERENCE_CVARS: {stale:?}" + ); +} + +/// **It is empty, and that is the finding, not an omission.** Every CVar the reference's own two +/// option tables name is now either unregistered (and on `UNBACKED_REFERENCE_CVARS` above with the +/// feature it waits on) or reachable from benilla's own options window. `autointeract` and +/// `UnitNamePlayerPVPTitle`, the last two unbacked rows, will each need a row here the day their +/// feature lands — which is exactly what the census above will say when it calls them stale. +/// +/// A name arriving here needs a reason of the same shape as the list above's: what the *player* +/// cannot do, and why the row is not simply seated. +const UNREACHABLE_REFERENCE_CVARS: &[(&str, &str)] = &[]; + +/// The four camera toggles (2149's mechanisms, 2179's water-collision rebuild) on the page a +/// player can open. Each box opens showing the value the reference registers — two of them "1", +/// which is the half that makes this more than cosmetic: on `cameraPivot` and +/// `cameraWaterCollision` a player who wants them OFF had no way to say so at all. +#[test] +fn the_four_camera_toggles_read_their_shipped_defaults_and_write_on_the_click() { + let mut s = harness_on(audio_harness()); + s.run("ShowUIPanel(BenillaOptionsFrame)").unwrap(); + s.run("BenillaOptionsFrameCategoryListRowControls:Click()") + .unwrap(); + assert!( + s.take_cvar_changes().is_empty(), + "reading the four on open must not write them back" + ); + + let checked = |s: &mut UiScript, row: &str| -> bool { + s.eval::(&format!( + "return BenillaOptionsFrameContainerBodyControls{row}Check:GetChecked()" + )) + .unwrap() + }; + // The registrar's own values: pivot and water collision ON, terrain tilt and head bob OFF. + assert!(checked(&mut s, "RowSmartPivot")); + assert!(checked(&mut s, "RowWaterCollision")); + assert!(!checked(&mut s, "RowFollowTerrain")); + assert!(!checked(&mut s, "RowHeadBob")); + + // And the click writes — the first flip of each is the one that could not be expressed before. + for (row, cvar, want) in [ + ("RowFollowTerrain", "cameraTerrainTilt", "1"), + ("RowHeadBob", "cameraBobbing", "1"), + ("RowWaterCollision", "cameraWaterCollision", "0"), + ("RowSmartPivot", "cameraPivot", "0"), + ] { + s.run(&format!( + "BenillaOptionsFrameContainerBodyControls{row}Check:Click()" + )) + .unwrap(); + assert_eq!( + s.take_cvar_changes(), + vec![(cvar.to_string(), want.to_string())], + "{row} writes {cvar}" + ); + } + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); +} + +/// **The partner write** (2180): two rows drive a second CVar that appears in neither option +/// table, because `UIOptionsFrame_Save` keeps a special-case arm for each. Without it the control +/// is a half-write, and a silent one — `SetCVar` on a name nobody wrote is not an error, it is a +/// value that stayed where it was, so the box would read "pet damage off" while pet SPELL damage +/// kept floating. +#[test] +fn the_pet_damage_box_and_the_look_slider_each_write_their_unnamed_twin() { + let mut s = combat_harness(); + s.run("ShowUIPanel(BenillaOptionsFrame)").unwrap(); + s.run("BenillaOptionsFrameCategoryListRowCombat:Click()") + .unwrap(); + let _ = s.take_cvar_changes(); + + // Pet Damage off: both pet CVars move, the melee one through the row and the spell one + // through the partner (UIOptionsFrame_Save l.334-336). + s.run("BenillaOptionsFrameContainerBodyCombatRowPetDamageCheck:Click()") + .unwrap(); + assert_eq!( + s.take_cvar_changes(), + vec![ + ("PetMeleeDamage".to_string(), "0".to_string()), + ("PetSpellDamage".to_string(), "0".to_string()), + ], + "one box, both pet knobs" + ); + + // Mouse Look Speed: the pitch twin at half the yaw value (l.355-356) — which is exactly the + // 180/90 pair the two are registered at, so the ratio the reference ships is preserved by + // dragging rather than broken by it. + s.run("BenillaOptionsFrameCategoryListRowControls:Click()") + .unwrap(); + let _ = s.take_cvar_changes(); + s.run("BenillaOptionsFrameContainerBodyControlsRowMouseLookSpeedControlSlider:SetValue(240)") + .unwrap(); + assert_eq!( + s.take_cvar_changes(), + vec![ + ("cameraYawMoveSpeed".to_string(), "240".to_string()), + ("cameraPitchMoveSpeed".to_string(), "120".to_string()), + ] + ); + assert_eq!( + s.eval::( + "return BenillaOptionsFrameContainerBodyControlsRowMouseLookSpeedControlValue:GetText()" + ) + .unwrap(), + "240", + "a deg/s slider reads out the raw number, not a percent of nothing" + ); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); +} + +/// **Show Target Damage is a SECOND master on the Combat page, and the floating-text one does not +/// own it.** The reference keeps the two dependency blocks apart — +/// `UIOptionsFrame_UpdateDependencies` greys 53…69 from CheckButton52 and greys 9 and 11 from 19, +/// in two separate `if`s — and collapsing them would tell a player that turning off scrolling +/// combat text also turns off damage numbers, which is the opposite of what `CombatDamage` does. +#[test] +fn show_target_damage_greys_its_own_pair_and_the_floating_text_master_leaves_it_alone() { + let mut s = combat_harness(); + s.run("ShowUIPanel(BenillaOptionsFrame)").unwrap(); + s.run("BenillaOptionsFrameCategoryListRowCombat:Click()") + .unwrap(); + let enabled = |s: &mut UiScript, row: &str| -> bool { + s.eval::(&format!( + "return BenillaOptionsFrameContainerBodyCombat{row}Check:IsEnabled() ~= 0" + )) + .unwrap() + }; + + // `SHOW_COMBAT_TEXT` ships "0", so the page arrives with the floating-text family greyed — + // and the damage trio live beside it, which is the whole point of the split. + assert_eq!(s.eval::("return SHOW_COMBAT_TEXT").unwrap(), "0"); + assert!(!enabled(&mut s, "RowAuras")); + assert!(enabled(&mut s, "RowShowDamage")); + assert!(enabled(&mut s, "RowPeriodicDamage")); + assert!(enabled(&mut s, "RowPetDamage")); + + // The other master: Show Target Damage off greys its two children and nothing else. + s.run("BenillaOptionsFrameContainerBodyCombatRowShowDamageCheck:Click()") + .unwrap(); + assert_eq!( + s.eval::("return GetCVar(\"CombatDamage\")") + .unwrap(), + "0" + ); + assert!(!enabled(&mut s, "RowPeriodicDamage")); + assert!(!enabled(&mut s, "RowPetDamage")); + assert!( + enabled(&mut s, "RowShowDamage"), + "the master is the way back in" + ); + + // Waking the floating-text family does not wake them: the two gates are independent. + s.run("BenillaOptionsFrameContainerBodyCombatRowCombatTextCheck:Click()") + .unwrap(); + assert!(enabled(&mut s, "RowAuras")); + assert!( + !enabled(&mut s, "RowPetDamage"), + "scrolling combat text does not own the damage numbers" + ); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); +} + +/// The two other dependencies 2180 seated, both the reference's own: Player Guild Names is dead +/// while player names are off (l.702-708 — there is no overhead stack for the guild line to be +/// the second row of), and Auto-Follow Speed is dead while the following style is Never +/// (l.715-719 — there is no follow to set a speed for). +#[test] +fn the_guild_line_greys_with_player_names_and_the_follow_speed_with_the_style() { + let mut s = harness_on(audio_harness()); + s.run("ShowUIPanel(BenillaOptionsFrame)").unwrap(); + + // Nameplates: `UnitNamePlayer` ships "1", so the child arrives live. + s.run("BenillaOptionsFrameCategoryListRowNameplates:Click()") + .unwrap(); + let guild_on = |s: &mut UiScript| -> bool { + s.eval::( + "return BenillaOptionsFrameContainerBodyNameplatesRowGuildNamesCheck:IsEnabled() ~= 0", + ) + .unwrap() + }; + assert!(guild_on(&mut s)); + s.run("BenillaOptionsFrameContainerBodyNameplatesRowPlayerNamesCheck:Click()") + .unwrap(); + assert!(!guild_on(&mut s), "no names, no guild line to gate"); + s.run("BenillaOptionsFrameContainerBodyNameplatesRowPlayerNamesCheck:Click()") + .unwrap(); + assert!(guild_on(&mut s)); + + // Controls: the style ships "1" (Smart), so the slider arrives live; "0" is Never. + s.run("BenillaOptionsFrameCategoryListRowControls:Click()") + .unwrap(); + let speed_thumb = |s: &mut UiScript| -> bool { + s.eval::( + "return BenillaOptionsFrameContainerBodyControlsRowAutoFollowSpeedControlSliderThumb\ + :IsShown()", + ) + .unwrap() + }; + assert!(speed_thumb(&mut s)); + s.run("SetCVar(\"cameraSmoothStyle\", \"0\") OptionsPage_Refresh(\"Controls\")") + .unwrap(); + assert!( + !speed_thumb(&mut s), + "a Never follow leaves the speed groove thumbless, 1.12's own way of saying dead" + ); + // …and the groove refuses the press as well as looking refused, which the reference never + // needed: its slider has no track jump to refuse. + assert!(!s + .eval::( + "return BenillaOptionsFrameContainerBodyControlsRowAutoFollowSpeedControlSlider\ + :IsMouseEnabled()" + ) + .unwrap()); + s.run("SetCVar(\"cameraSmoothStyle\", \"1\") OptionsPage_Refresh(\"Controls\")") + .unwrap(); + assert!(speed_thumb(&mut s)); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); +} diff --git a/crates/benilla-app/src/ui_script/reference_ui.rs b/crates/benilla-app/src/ui_script/reference_ui.rs index 4b7f2c44..5149bd64 100644 --- a/crates/benilla-app/src/ui_script/reference_ui.rs +++ b/crates/benilla-app/src/ui_script/reference_ui.rs @@ -1512,56 +1512,6 @@ mod tests { // reads clean. Each belongs to whichever window's migration left it, and each is one // binding or one sourced file away. const KNOWN: &[(&str, &str, &str)] = &[ - ( - "OptionsFrame.xml", - "GetGamma", - "the display-gamma pair, and the only two of the video window's engine verbs \ - decision 2177 did not build. **Fully pinned and deliberately absent**, which is \ - 1203's other permitted outcome. wow-re `ui/scratch/video-options-verbs.md` §3 \ - carves both: `GetGamma 0x4891c0` returns `1.0 - gamma` and `SetGamma 0x4891f0` \ - writes `gamma := 1.0 - v` (the `dc 2d` at `0x4891d0` is FSUBR, `mem - ST(0)`), so \ - the unit is the SLIDER's offset and not the CVar's — a `gamma`-valued 1.0 passed \ - in writes `gamma = 0`, `pow(x,0) = 1`, a fully white ramp, with no clamp anywhere \ - to catch it. The value reaches the hardware synchronously inside `SetCVar`, \ - through the change callback `0x4034d0` uploading a ramp. **And that upload is \ - skipped in windowed mode** (format byte `+0x07` = `gxWindow`) — which is every \ - mode benilla has, since this client ships borderless fullscreen and no exclusive \ - mode-set at all (`crate::video`'s module doc). So a byte-faithful pair here would \ - be a setting that never moves a pixel, and the honest mechanism is a different \ - one: a final full-screen correction over the composited image. That is a render \ - feature with its own look, its own `gamma` CVar row and its own record — not a \ - line in a bindings file, and not something to register a key for with no reader \ - (1134 §4). Neither verb is reachable at LOAD; both are `OptionsFrame_Load` \ - (l.153) / `_Cancel` (l.260) / slider 6's `OnValueChanged`, and that function \ - already stops earlier — at slider 4, `TerrainMip`, whose `GetCVar` is nil, the \ - reference having a real `GetTerrainMip 0x488fb0` where we have none — so building \ - the pair alone would move the raise, not remove it.", - ), - ( - "OptionsFrame.xml", - "SetGamma", - "the display-gamma pair, and the only two of the video window's engine verbs \ - decision 2177 did not build. **Fully pinned and deliberately absent**, which is \ - 1203's other permitted outcome. wow-re `ui/scratch/video-options-verbs.md` §3 \ - carves both: `GetGamma 0x4891c0` returns `1.0 - gamma` and `SetGamma 0x4891f0` \ - writes `gamma := 1.0 - v` (the `dc 2d` at `0x4891d0` is FSUBR, `mem - ST(0)`), so \ - the unit is the SLIDER's offset and not the CVar's — a `gamma`-valued 1.0 passed \ - in writes `gamma = 0`, `pow(x,0) = 1`, a fully white ramp, with no clamp anywhere \ - to catch it. The value reaches the hardware synchronously inside `SetCVar`, \ - through the change callback `0x4034d0` uploading a ramp. **And that upload is \ - skipped in windowed mode** (format byte `+0x07` = `gxWindow`) — which is every \ - mode benilla has, since this client ships borderless fullscreen and no exclusive \ - mode-set at all (`crate::video`'s module doc). So a byte-faithful pair here would \ - be a setting that never moves a pixel, and the honest mechanism is a different \ - one: a final full-screen correction over the composited image. That is a render \ - feature with its own look, its own `gamma` CVar row and its own record — not a \ - line in a bindings file, and not something to register a key for with no reader \ - (1134 §4). Neither verb is reachable at LOAD; both are `OptionsFrame_Load` \ - (l.153) / `_Cancel` (l.260) / slider 6's `OnValueChanged`, and that function \ - already stops earlier — at slider 4, `TerrainMip`, whose `GetCVar` is nil, the \ - reference having a real `GetTerrainMip 0x488fb0` where we have none — so building \ - the pair alone would move the raise, not remove it.", - ), ( "ContainerFrame.xml", "KeyRingButtonIDToInvSlotID", diff --git a/crates/benilla-app/src/ui_script/shipped_xml_tests.rs b/crates/benilla-app/src/ui_script/shipped_xml_tests.rs index 287d5ad1..02a7c570 100644 --- a/crates/benilla-app/src/ui_script/shipped_xml_tests.rs +++ b/crates/benilla-app/src/ui_script/shipped_xml_tests.rs @@ -2072,6 +2072,119 @@ fn the_stock_video_options_window_loads_hidden_and_owns_its_own_name() { assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } +/// **The display-brightness pair** (decision 2182) — `GetGamma`/`SetGamma` and the Graphics page +/// row that drives them, held to the reference's own carve. +/// +/// The claim that needs an assertion rather than a comment is the **unit**: `0x4891d0` is FSUBR, +/// so `GetGamma()` is `1.0 − gamma` and `SetGamma(v)` writes `gamma := 1.0 − v`. Read as +/// "gamma in, gamma out" the pair still composes to the identity and still *looks* right in a +/// round-trip test — but the panel would then put `gamma = 0` (`pow(x, 0) = 1`, a white screen) +/// at the top of a slider whose top is supposed to be the brightest legible picture. So this +/// checks the two ENDS of the reference's own `[-0.5, 0.5]` against the CVar text, not just that +/// get and set agree with each other. +/// +/// And the second claim: **there is no clamp**, anywhere, in the reference (the positive control +/// is `baseMip`'s validating callback `0x689090`). `SetGamma(5)` writes `"-4.000000"` and the +/// store keeps it — benilla's clamp is at the render consumer, where it cannot lie to `GetCVar`. +/// +/// (wow-re `ui/scratch/video-options-verbs.md` §3 and +/// `ffxeffects/scratch/whole-frame-grade-verdict.md` §(a), both VERIFIED.) +#[test] +fn the_display_brightness_pair_speaks_the_reference_slider_unit() { + let _data = benilla_formats::wow_data_or_skip!(); + let mut s = benilla_ui::script::UiScript::new().unwrap(); + s.set_screen_size(1024.0, 768.0); + let failures = super::load_default_ui(&s); + assert!(failures.is_empty(), "manifest load errors: {failures:#?}"); + + // The reference registers `gamma = "1.0"` (our table spells it `"1.000000"`, the spelling + // `SetGamma` itself writes — see its row in `crate::cvars`), and 0.0 is what that reads as: + // the exact centre of the stock slider. + assert_eq!( + s.eval::("return GetGamma()").unwrap(), + 0.0, + "a fresh client sits at the centre of the stock slider's [-0.5, 0.5]" + ); + // Both ends, in the CVar's own text — `SStrPrintf(buf, 0x10, "%f", 1.0 - v)`. + for (slider, cvar) in [ + (0.5, "0.500000"), + (-0.5, "1.500000"), + (0.0, "1.000000"), + // No clamp: the reference accepts this and writes a negative gamma. + (5.0, "-4.000000"), + ] { + s.eval::<()>(&format!("SetGamma({slider})")).unwrap(); + assert_eq!( + s.eval::("return GetCVar(\"gamma\")").unwrap(), + cvar, + "SetGamma({slider}) writes 1 - v with six decimals" + ); + assert_eq!( + s.eval::("return GetGamma()").unwrap(), + slider, + "…and the getter is its exact inverse" + ); + } + // Zero return values, not nil (`eax = 0` at every `ret`). `select` is not in this VM (2171), + // so the count is read the way 1.12 Lua reads one: a multiple assignment. + assert_eq!( + s.eval::( + "local a, b = SetGamma(0) \ + if a ~= nil or b ~= nil then return 1 end \ + return 0" + ) + .unwrap(), + 0, + "SetGamma pushes nothing" + ); + // …and it REQUIRES its argument (`0x4891fe`, raising through `0x6f4940`, which never returns). + let err = s.eval::<()>("SetGamma()").unwrap_err().to_string(); + assert!( + err.contains("Usage: SetGamma(value)"), + "the reference's own usage string, verbatim: {err}" + ); + + // The row: our Graphics page drives the PAIR, not the CVar, which is 1.12's own arrangement + // for this one slider — and it reads its bounds off the reference's `OptionsFrameSliders[6]`. + s.eval::<()>("SetGamma(0)").unwrap(); + let row = "BenillaOptionsFrameContainerBodyGraphicsRowBrightness"; + let bounds = s + .eval::>(&format!( + "local r = getglobal({row:?}) \ + local sl = getglobal({row:?} .. \"ControlSlider\") \ + local lo, hi = sl:GetMinMaxValues() \ + return {{ lo, hi, sl:GetValueStep(), r.numeric }}" + )) + .unwrap(); + // The step is an f32 on the widget, so it comes back as 0.100000001…; the two bounds and the + // numeric flag are exact. + assert_eq!( + (bounds[0], bounds[1], bounds[3]), + (-0.5, 0.5, 1.0), + "the reference's slider-6 bounds, on a numeric api row" + ); + assert!( + (bounds[2] - 0.1).abs() < 1e-6, + "…and its step: {}", + bounds[2] + ); + // The readout is the thumb's share of the groove (0..100 with the default at 50), never the + // stored offset — "0%" on a brightness control doing nothing wrong is exactly backwards. It is + // written by the page's own refresh, so the window has to be up for there to be one. + s.eval::<()>( + "ShowUIPanel(BenillaOptionsFrame) \ + BenillaOptionsFrameCategoryListRowGraphics:Click()", + ) + .unwrap(); + assert_eq!( + s.eval::(&format!( + "return getglobal({row:?} .. \"ControlValue\"):GetText()" + )) + .unwrap(), + "50%", + ); +} + /// **The ten names the video window's slider walk must NOT find** (decision 2177). /// /// `OptionsFrame_Load:110` and `_Save:208-209` do `getglobal("Get"..value.func)` / diff --git a/crates/benilla-app/tests/world_api_wall.rs b/crates/benilla-app/tests/world_api_wall.rs index 951168fc..47cd34bb 100644 --- a/crates/benilla-app/tests/world_api_wall.rs +++ b/crates/benilla-app/tests/world_api_wall.rs @@ -400,7 +400,19 @@ fn is_instrument_consumer(rel: &str) -> bool { /// diffuse lobe and the MOLT points are already summed in and cannot be subtracted back out — and /// a caller that reached for the *scene* ambient instead, which is what this replaces, tinted /// every indoor trail with the sky. -const CEILING: usize = 179; +/// And 179 → 180: `weather::WeatherState`, a PUBLISH — the weather driver's own state, reached by +/// `crate::cvars` because one byte of it (`weather_density`) is a **player setting** and 2181 gave +/// it its switch. It is the same shape as `clutter::ClutterConfig` two rows of that table up: a +/// resource the engine owns and the options window writes exactly one field of, welded to its CVar +/// by `registered_defaults_mirror_the_code_truths`. Published as the state rather than as a +/// density-only door for the reason the two entries above give in reverse — there is no +/// reconstruction to get wrong here, the field is a `u8` the engine reads directly, and a +/// one-field wrapper would be a second name for the same byte that the weld test would then have +/// to hold in step with both sides. It was already on the far side of the wall as an INSTRUMENT +/// item (the `#[cfg(feature = "dev")]` weather panel is its only other namer); what changed is +/// that a game module names it now, which is exactly the crossing this gate exists to make +/// visible. +const CEILING: usize = 180; /// How far under [`CEILING`] the real count may sit before this test asks for the ceiling to be /// lowered. Slack, not tolerance: it keeps a single closure from failing the gate, while making it diff --git a/crates/benilla-ui/src/script/button.rs b/crates/benilla-ui/src/script/button.rs index dae48c6e..64fca32d 100644 --- a/crates/benilla-ui/src/script/button.rs +++ b/crates/benilla-ui/src/script/button.rs @@ -343,6 +343,45 @@ pub(super) fn edge(model: &mut Model, h: FrameHandle, f: impl FnOnce(&mut Button } } +/// Does a hover edge on frame `h` reach the **base** enter/leave notify — the HIGHLIGHT layer and +/// the Lua ``/`` script slots — or is it swallowed on the way there? +/// +/// `CSimpleButton::OnEnter 0x779490` and `OnLeave 0x7794e0` both open on the state word, and the +/// branch skips the *whole* body except the sound: +/// +/// ```text +/// 779493 mov eax,[esi+0x328] +/// 779499 test eax,eax +/// 77949b je 0x7794b0 ; DISABLED -> skip the base notify AND the label restyle +/// 77949d call 0x76b6a0 ; CSimpleFrame::OnEnter — highlight layer on, at +0x140 +/// ``` +/// +/// — so **a DISABLED Button runs neither script**, and `0x7794b0`'s hover sound is the only thing +/// past the branch (wow-re `scratch/button-state-edge-set.md` §3.1, VERIFIED; the base pair's +/// script slots `+0x140`/`+0x148` are `ledger.tsv:7944/7945`). Every path that delivers a hover +/// edge dispatches through the object's own vtable, so there is no way around the guard: the hover +/// walk (`0x766218`–`0x76623c`, `[vt+0x50]` then `[vt+0x4c]`), `SetMouseFocus 0x764dc0` +/// (`0x764ddd push 0`), and the hide/removal tail (`0x764cce mov edx,[edi]; push 1; +/// call [edx+0x50]`). `CheckButton` inherits both slots; the Hyperlink and NamePlate buttons +/// override them and tail straight back into this pair (`0x7cb8a5`), so the guard covers the +/// family. +/// +/// The hover **target** still moves — the walk reassigns `[root+0x7c]` *before* it fires either +/// notify — so `GetMouseFocus()` answers a disabled button; it is the scripts alone that stay +/// quiet. This is what makes AtlasLoot's unset QuickLook buttons safe on the reference: their +/// `` guards with `if this:IsEnabled() then`, which is a NUMBER `1`/`0` and therefore +/// always truthy (`scratch/button-enabled-state.md`), so the body would index a nil +/// `QuickLooks[n]` — the reference never runs it at all. +/// +/// `None` (the cursor over no frame) and every non-Button kind notify normally. +pub(super) fn hover_notify_runs(model: &Model, h: Option) -> bool { + let Some(h) = h else { return true }; + match model.arena.frame(h).map(|f| &f.kind_state) { + Some(KindState::Button(bs)) => bs.enabled(), + _ => true, + } +} + /// Write [`ButtonState::loot_slot`] — `LootButton:SetSlot`'s whole body, kept here beside the /// other `ButtonState` writers rather than reaching into the arena from `script::loot`. pub(super) fn set_loot_slot(lua: &Lua, this: &Table, slot: Option) -> mlua::Result<()> { diff --git a/crates/benilla-ui/src/script/cvars.rs b/crates/benilla-ui/src/script/cvars.rs index 48071c63..9d7207ce 100644 --- a/crates/benilla-ui/src/script/cvars.rs +++ b/crates/benilla-ui/src/script/cvars.rs @@ -586,7 +586,7 @@ pub const VIDEO_DEFAULT_CVARS: &[&str] = &[ "WorldDetail", ]; -/// **The Video options window's own engine verbs** — the seven `OptionsFrame.xml` needs that +/// **The Video options window's own engine verbs** — the nine `OptionsFrame.xml` needs that /// nothing else in this client provides. /// /// The window is loaded off the player's chain and kept hidden (decision 2177, the shape 2115 and @@ -594,7 +594,7 @@ pub const VIDEO_DEFAULT_CVARS: &[&str] = &[ /// an addon reaching for the reference's video-options names finds real frames and real functions /// instead of an alias onto a window of ours. /// -/// **Three of the seven run at LOAD** — `GetScreenResolutions`, `GetCurrentResolution` and +/// **Three of the nine run at LOAD** — `GetScreenResolutions`, `GetCurrentResolution` and /// `GetRefreshRates`, from the resolution and refresh dropdowns' ``, because /// `UIDropDownMenu_Initialize` calls the initializer it is handed *immediately* /// (`UIDropDownMenu.lua:48-50`). The file cannot load at all without them, and the window being @@ -603,9 +603,10 @@ pub const VIDEO_DEFAULT_CVARS: &[&str] = &[ /// window missed, is plain `GetCVar` (`OptionsFrame.lua:300`, `GetCVar("gxRefresh")`). /// /// The three multisample verbs the same window needs are above; `GetWorldDetail`/`SetWorldDetail` -/// are in [`install_world_detail_verbs`]. `GetGamma`/`SetGamma` are deliberately NOT here — see -/// `ui_script::reference_ui`'s KNOWN table for what they would cost and why they are a feature of -/// their own rather than a line in this function. +/// are in [`install_world_detail_verbs`]. `GetGamma`/`SetGamma` are the last two: 2177 shipped +/// seven and left that pair **loudly absent** under 1203, because a byte-faithful copy would have +/// driven a hardware ramp this client never uploads. 2182 built the render feature behind them, so +/// they have a real reader now and the window's engine verbs are nine of nine. /// /// **What must NOT be defined, which is as load-bearing as what is.** `OptionsFrame_Load:110` does /// `getglobal("Get"..value.func)` over the nine slider rows and branches on the result; of the @@ -855,9 +856,80 @@ fn install_video_verbs(lua: &Lua) -> mlua::Result<()> { Ok(()) })?, )?; + // ── GetGamma / SetGamma ────────────────────────────────────────────────────────────────── + // **The display-brightness pair** (2182) — `GetGamma 0x4891c0` and `SetGamma 0x4891f0`, carved + // end to end in wow-re `ui/scratch/video-options-verbs.md` §3. + // + // The one thing about them that is not obvious from the name: **the unit is the SLIDER's + // offset, not the CVar's.** `0x4891d0` is `dc 2d`, ModRM reg field 5 = **FSUBR** (`mem − ST(0)`, + // not `FSUB`), over `[0x8015b8]` = the f64 `1.0`. So `GetGamma()` answers `1.0 − gamma` and + // `SetGamma(v)` writes `gamma := 1.0 − v` — the two compose to the identity, and with the + // registered `gamma = "1.0"` the getter answers **0.0**, the exact centre of the stock + // slider's `[-0.5, 0.5]`. Reading the pair as "gamma in, gamma out" would put a 1.0 through + // `SetGamma` and write `gamma = 0`, i.e. `pow(x, 0) = 1`, a fully white ramp. + // + // **No clamp exists anywhere in the reference** — not in the binding, not in `CVar::Set`, not + // in the change callback, not in the ramp builder — and the absence is earned rather than + // assumed: the positive control is `baseMip`'s own callback `0x689090`, which *does* reject + // out-of-`[0,1]` with `al = 0`. `SetGamma(5)` is accepted and writes `gamma = "-4.000000"`. + // benilla's clamp is at the consumer (`ui_gamma::GAMMA_RANGE`), which is this crate's standing + // posture and leaves the store's truth alone. + lua.globals().set( + "GetGamma", + // No argument is read (the getter never calls `lua_gettop`), and one number comes back. + lua.create_function(|lua, _: mlua::MultiValue| { + let model = lua.app_data_ref::().expect("model app_data"); + let gamma = model + .cvars + .get(&CVAR_GAMMA.to_ascii_lowercase()) + .and_then(|slot| slot.value.parse::().ok()) + // A VM whose host registered nothing (a bare test kit) answers as if the + // registered default were in place, which is 0.0 — the slider's centre. The + // reference has no such arm: its `0x63de30` lookup CREATES the record on a miss, + // so `[eax+0x24]` is always readable. + .unwrap_or(1.0); + Ok(1.0 - gamma) + })?, + )?; + lua.globals().set( + "SetGamma", + lua.create_function(|lua, value: Value| { + // `lua_isnumber` (`0x4891fe`): a number, or a string Lua can convert to one. Anything + // else raises through `0x6f4940`, which does not return — the trailing `xor eax,eax` + // at `0x489215` is dead code. + let v = match &value { + Value::Integer(i) => *i as f64, + Value::Number(n) => *n, + Value::String(s) => { + match s.to_str().ok().and_then(|s| s.trim().parse::().ok()) { + Some(n) => n, + None => return Err(mlua::Error::runtime(USAGE_SET_GAMMA)), + } + } + _ => return Err(mlua::Error::runtime(USAGE_SET_GAMMA)), + }; + // `SStrPrintf(buf, 0x10, "%f", 1.0 - v)` — six decimals into a SIXTEEN-byte buffer, so + // a long value is truncated to 15 characters rather than rejected. Reproduced because + // the truncated string is what the CVar then holds and what `GetCVar("gamma")` answers. + let mut written = format!("{:.6}", 1.0 - v); + written.truncate(15); + let mut model = lua.app_data_mut::().expect("model app_data"); + write_cvar(&mut model, CVAR_GAMMA, written, None); + // Zero return values, not nil (`eax = 0` at every `ret`). + Ok(mlua::MultiValue::new()) + })?, + )?; Ok(()) } +/// The reference's own display-gamma CVar — `0x402d70`, name string `0x82e924`, registered `"1.0"`, +/// flags 0. Defined in the crate that publishes the two verbs that read and write it, so the +/// binding and the name cannot drift apart; the app welds it to its own table. +pub const CVAR_GAMMA: &str = "gamma"; + +/// `0x8424cc`, verbatim. +const USAGE_SET_GAMMA: &str = "Usage: SetGamma(value)"; + /// The `WorldDetail` slider's **stop**, benilla's spelling — the panel position `0`/`1`/`2`, held as /// a CVar because 1.12 has no such CVar and our own Graphics page needs somewhere to keep it (the /// `autoLootDefault` posture; the app's table carries the reasoning and the `Same` row). diff --git a/crates/benilla-ui/src/script/event.rs b/crates/benilla-ui/src/script/event.rs index 6c8af878..00d1dc6d 100644 --- a/crates/benilla-ui/src/script/event.rs +++ b/crates/benilla-ui/src/script/event.rs @@ -208,6 +208,12 @@ pub(super) fn fire_visibility_changes(lua: &Lua, changed: Vec) { let hidden_hover = model .mouseover .filter(|&m| items.iter().any(|&(h, _, vis)| h == m && !vis)); + // The removal tail is a VIRTUAL dispatch — `0x764cce mov edx,[edi]; push 1; + // call [edx+0x50]` — so a Button reaches its own `0x7794e0`, whose DISABLED guard skips + // the base leave and with it the `` script. Hiding a disabled hovered button is + // therefore as silent as walking off one ([`super::button::hover_notify_runs`]); the + // hover cache and the drag-arm still clear, because those are the *caller's* half. + let notified = super::button::hover_notify_runs(&model, hidden_hover); if let Some(m) = hidden_hover { model.mouseover = None; if model.drag.as_ref().is_some_and(|d| d.source == m) { @@ -215,7 +221,9 @@ pub(super) fn fire_visibility_changes(lua: &Lua, changed: Vec) { } } model.hover_repick |= hidden_hover.is_some() || items.iter().any(|&(_, _, vis)| vis); - hidden_hover.map(|m| (m, model.frame_id(m))) + hidden_hover + .filter(|_| notified) + .map(|m| (m, model.frame_id(m))) }; if let Some((_, oid)) = left { if let Err(e) = fire_widget_handler(lua, oid, "OnLeave", vec![Value::Boolean(true)]) { diff --git a/crates/benilla-ui/src/script/mod.rs b/crates/benilla-ui/src/script/mod.rs index 003f9c2e..cd3aacfb 100644 --- a/crates/benilla-ui/src/script/mod.rs +++ b/crates/benilla-ui/src/script/mod.rs @@ -201,8 +201,9 @@ pub use cursor::{ CursorPetAction, CursorSpell, CursorStablePet, EnchantConfirm, WorldPick, EQUIPMENT_BAG, }; pub use cvars::{ - MultisampleFormat, ScreenResolution, VideoCaps, CVAR_FRILL_DENSITY, CVAR_NAMEPLATE_ENEMIES, - CVAR_NAMEPLATE_FRIENDS, CVAR_WORLD_DETAIL, VIDEO_DEFAULT_CVARS, WORLD_DETAIL_STOPS, + MultisampleFormat, ScreenResolution, VideoCaps, CVAR_FRILL_DENSITY, CVAR_GAMMA, + CVAR_NAMEPLATE_ENEMIES, CVAR_NAMEPLATE_FRIENDS, CVAR_WORLD_DETAIL, VIDEO_DEFAULT_CVARS, + WORLD_DETAIL_STOPS, }; pub use death::{DeathAction, DeathUiState}; pub use dressup::DressUpIntent; diff --git a/crates/benilla-ui/src/script/pointer.rs b/crates/benilla-ui/src/script/pointer.rs index 8d2ff41a..1179dccd 100644 --- a/crates/benilla-ui/src/script/pointer.rs +++ b/crates/benilla-ui/src/script/pointer.rs @@ -9,7 +9,7 @@ //! `OnLeave`/`OnEnter` pair (RF-0025's `motion=true` hover boundary) — and, before that, //! advances an armed drag gesture, an in-flight **Slider thumb drag**, and an in-flight //! **frame move** (`StartMoving`, [`super::object`]'s `movable` cluster). All three move within -//! one frame, crossing no hover boundary, so they must run before the boundary early-return. +//! one frame, crossing no hover boundary, so they must run before the boundary test. //! - [`UiScript::mouse_button`] resolves `OnMouseDown`/`OnMouseUp`, the `OnClick` //! press/release-registration rules ([`button::wants_click`]), the **drag trio** + **world //! drop** below, the **Slider thumb-drag** capture (a left press on a thumb, decision 0250 §5), @@ -269,8 +269,9 @@ impl UiScript { /// `OnEnter(self, motion=true)` on the newly-captured frame (if any). Tracks the current /// mouseover in the model. Also advances an armed [`super::Model::drag`] gesture (decision 0216 §3) /// and an in-flight [`super::Model::moving`] frame ([`super::object`]'s `movable` cluster): - /// both run BEFORE the no-boundary-crossed early return below, since dragging within one - /// frame crosses no hover boundary at all. Returns the captured frame id (so the app can drive + /// both run BEFORE the boundary test below, since dragging within one frame crosses no hover + /// boundary at all. A **DISABLED Button** takes the hover — `GetMouseFocus()` answers it — but + /// fires neither script ([`button::hover_notify_runs`]). Returns the captured frame id (so the app can drive /// `PointerOverUi`). Handler errors are collected into [`UiScript::errors`], never panicking. pub fn mouse_move(&mut self, x: f32, y: f32) -> Option { // A drag-active EditBox tracks every move (`0x77a860`): extend the selection to the @@ -278,10 +279,10 @@ impl UiScript { editbox::drag_update(&self.lua, x, y); let new_id = self.hit_test(x, y); #[allow(clippy::type_complexity)] - let (old_id, drag_start, boundary_crossed, slider_change, color_change): ( + let (leave_id, drag_start, enter_id, slider_change, color_change): ( Option, Option<(u32, String)>, - bool, + Option, Option<(u32, f32)>, Option<(u32, f64, f64, f64)>, ) = { @@ -303,7 +304,7 @@ impl UiScript { let new_handle = new_id.and_then(|id| model.id_to_frame.get(&id).copied()); if new_handle == model.mouseover { // no boundary crossed - (None, drag_start, false, slider_change, color_change) + (None, drag_start, None, slider_change, color_change) } else { // Fire OnLeave only if the frame we're leaving is still live. let old_id = model.mouseover.and_then(|h| { @@ -326,8 +327,18 @@ impl UiScript { // So a press held over a button and walked off it stays PUSHED in the reference — // it is the *drag threshold* (below) that un-presses one, and only for a frame // that registered for drag. Decision 2134. - let _ = old_handle; - (old_id, drag_start, true, slider_change, color_change) + // + // **A DISABLED Button swallows its own hover notify** and so runs neither script: + // both `0x779490` and `0x7794e0` open `mov eax,[esi+0x328]; test eax,eax; je`, + // branching past the base notify that owns the ``/`` slots + // ([`button::hover_notify_runs`]). The gate is per-side — leaving a button that + // was disabled *while* hovered is as silent as entering one — and it is applied + // AFTER `model.mouseover` moves, because the reference reassigns the hover target + // `[root+0x7c]` before it fires either notify: a disabled button is still the + // mouse focus, it just says nothing. + let old_id = old_id.filter(|_| button::hover_notify_runs(&model, old_handle)); + let enter_id = new_id.filter(|_| button::hover_notify_runs(&model, new_handle)); + (old_id, drag_start, enter_id, slider_change, color_change) } }; // The thumb-drag value change fires OnValueChanged (outside the borrow) — the scrollbar's @@ -359,17 +370,14 @@ impl UiScript { self.push_error(e); } } - if !boundary_crossed { - return new_id; - } - if let Some(oid) = old_id { + if let Some(oid) = leave_id { if let Err(e) = event::fire_widget_handler(&self.lua, oid, "OnLeave", vec![Value::Boolean(true)]) { self.push_error(e); } } - if let Some(nid) = new_id { + if let Some(nid) = enter_id { if let Err(e) = event::fire_widget_handler(&self.lua, nid, "OnEnter", vec![Value::Boolean(true)]) { diff --git a/crates/benilla-ui/src/script/tests/input.rs b/crates/benilla-ui/src/script/tests/input.rs index 78eac0d2..eda3e22d 100644 --- a/crates/benilla-ui/src/script/tests/input.rs +++ b/crates/benilla-ui/src/script/tests/input.rs @@ -672,6 +672,35 @@ fn hiding_the_hovered_frame_fires_its_onleave_before_onhide() { assert!(s.errors().is_empty(), "{:?}", s.errors()); } +/// The removal tail's own DISABLED case: `0x764cce` dispatches `[edx+0x50]` **virtually**, so a +/// Button lands in `0x7794e0` and the guard swallows the leave there too. The `OnHide` half is the +/// control — that one is not the hover notify and fires either way. +#[test] +fn hiding_a_hovered_disabled_button_fires_onhide_but_no_onleave() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + log = {} + local slot = CreateFrame("Button", "Slot") + slot:SetPoint("BOTTOMLEFT", 100, 100); slot:SetWidth(100); slot:SetHeight(100); slot:EnableMouse(true) + slot:SetScript("OnLeave", function() table.insert(log, "leave") end) + slot:SetScript("OnHide", function() table.insert(log, "hide") end) + slot:Disable() + "#, + ) + .unwrap(); + s.resolve(); + s.mouse_move(150.0, 150.0); + s.run("Slot:Hide()").unwrap(); + assert_eq!( + s.eval::("return table.concat(log, ',')").unwrap(), + "hide", + "the removal tail reaches the button's own leave notify, which is DISABLED-gated" + ); + assert!(s.errors().is_empty(), "{:?}", s.errors()); +} + /// The negative: hiding a frame the cursor is NOT over fires no OnLeave (the reference's removal /// tail acts only when the removed frame IS the cached hover). #[test] @@ -835,3 +864,70 @@ fn the_capture_is_per_button() { "the right button captured nothing, so its release dispatches nothing" ); } + +/// **A DISABLED Button runs neither `` nor ``** — `CSimpleButton::OnEnter +/// 0x779490` / `OnLeave 0x7794e0` open `mov eax,[esi+0x328]; test eax,eax; je`, branching past the +/// base notify that owns both script slots (wow-re `scratch/button-state-edge-set.md` §3.1). It +/// still TAKES the hover, because the walk reassigns `[root+0x7c]` before it fires the notify. +/// +/// The customer is every 1.12 addon that wrote `if this:IsEnabled() then` at the top of an +/// `` and expected it to be false: `IsEnabled()` is the NUMBER `1`/`0`, always truthy, so +/// the body runs unless the ENGINE holds it back. AtlasLoot's four QuickLook buttons are the live +/// case — unset, they disable themselves in `` and their `` indexes a nil +/// `AtlasLootCharDB.QuickLooks[n]`. +#[test] +fn a_disabled_button_takes_the_hover_but_fires_no_enter_or_leave() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + log = {} + local function mk(name, x) + local b = CreateFrame("Button", name) + b:SetPoint("BOTTOMLEFT", x, 0); b:SetWidth(100); b:SetHeight(100); b:EnableMouse(true) + b:SetScript("OnEnter", function(self) table.insert(log, self:GetName()..":enter") end) + b:SetScript("OnLeave", function(self) table.insert(log, self:GetName()..":leave") end) + return b + end + Off = mk("Off", 0) + On = mk("On", 200) + Off:Disable() + -- The guard the addons actually wrote, and why it cannot be theirs: a NUMBER, never nil. + assert(Off:IsEnabled() == 0 and On:IsEnabled() == 1) + "#, + ) + .unwrap(); + s.resolve(); + + let hit = s.mouse_move(50.0, 50.0); + assert!(hit.is_some(), "a disabled button still captures the point"); + assert_eq!( + s.eval::("return GetMouseFocus():GetName()") + .unwrap(), + "Off", + "the hover target moves before the notify — a disabled button IS the mouse focus" + ); + assert_eq!( + s.eval::("return table.concat(log, ',')").unwrap(), + "", + "entering a disabled button fires nothing" + ); + + // Leaving it is as silent as entering it, and the enabled neighbour is the control. + s.mouse_move(250.0, 50.0); + assert_eq!( + s.eval::("return table.concat(log, ',')").unwrap(), + "On:enter", + "no leave from the disabled button; the enabled one notifies normally" + ); + + // Disabled WHILE hovered: the leave is gated on the state at leave time, not at enter time. + s.run("On:Disable()").unwrap(); + s.mouse_move(400.0, 50.0); + assert_eq!( + s.eval::("return table.concat(log, ',')").unwrap(), + "On:enter", + "a button disabled under the cursor swallows its own leave" + ); + assert!(s.errors().is_empty(), "{:?}", s.errors()); +} diff --git a/crates/benilla-world/src/collision.rs b/crates/benilla-world/src/collision.rs index 5c694bf4..1081e646 100644 --- a/crates/benilla-world/src/collision.rs +++ b/crates/benilla-world/src/collision.rs @@ -53,6 +53,21 @@ pub(crate) enum CollisionLayer { Liquid, } +/// **The camera-collision probe radius (yd)** — the sphere [`WorldCollision::cast_camera`] sweeps +/// along the boom against the SOLID world. +/// +/// It is the margin kept between the camera and the surface it stops at, so the near plane does +/// not poke through the wall; smaller than the player capsule, so the camera threads gaps the body +/// cannot. **benilla's own construction** — the reference's camera trace is a bare ray +/// (`0x672170`) with no radius anywhere on it. +/// +/// It lives *here*, in the crate that owns the trace, rather than beside the camera that spawns +/// the probe, because a fixture that sweeps a different radius than the client ships is an +/// instrument reading itself: decision 2179's gate swept `0.15` against a client shipping `0.3` +/// and certified a camera that was being slammed onto the character on every level surface swim +/// (2185). Nothing may hardcode this number. +pub const CAMERA_PROBE_RADIUS: f32 = 0.3; + /// `CollisionLayers` for a WMO's **walking** collider — member of [`CollisionLayer::Walk`] only, so the /// camera query (which omits `Walk`) never hits it. pub(crate) fn walk_layers() -> CollisionLayers { @@ -200,16 +215,53 @@ impl WorldCollision<'_, '_> { /// …and what the third-person **camera** collides with: the other way round on those two WMO /// face layers (it takes NOCAMCOLLIDE faces the body walks through, and skips the DETAIL faces /// the body walks on). - /// `liquid` adds the waterline — `cameraWaterCollision`'s primary consumer. The reference does - /// exactly this and no more: `0x50e5ec` ORs `0xf0000` into the trace mask word the solver hands - /// its three collision queries, so the CVar changes *what this one query is allowed to see* and - /// re-lanes nothing. `false` and it is byte-for-byte the filter it always was. - pub(crate) fn camera_filter(liquid: bool) -> SpatialQueryFilter { - let mut mask = CollisionLayer::Default.to_bits() | CollisionLayer::Camera.to_bits(); - if liquid { - mask |= CollisionLayer::Liquid.to_bits(); - } - SpatialQueryFilter::from_mask(LayerMask(mask)) + /// + /// **The waterline is deliberately not in here** — it rides its own ray + /// ([`waterline_ray`](Self::waterline_ray)), for the reason spelled out there. This mask is the + /// SOLID world only, and is byte-for-byte the filter it always was. + pub(crate) fn camera_filter() -> SpatialQueryFilter { + SpatialQueryFilter::from_mask(LayerMask( + CollisionLayer::Default.to_bits() | CollisionLayer::Camera.to_bits(), + )) + } + + /// **The waterline leg of the camera trace — a RAY, not the probe sphere** (decision 2185). + /// + /// `cameraWaterCollision` is a trace mask (`0x50e5ec` ORs `0xf0000` into the word the solver + /// hands its three collision queries), and the query that word rides is a **line segment**: + /// `0x672170` takes `(start, end, out, frac, flags)` and carries no radius at all, bottoming + /// out in the Möller–Trumbore ray/triangle test at `0x7c2c40` (wow-re + /// `camera-water-atomicity.md` §145, `camera-cvar-kernels.md` §276). The reference's whole + /// camera trace is that ray; benilla's is a [`CAMERA_PROBE_RADIUS`] sphere, which is **our own + /// construction** — the margin that keeps the near plane out of a wall — and it stays, because + /// walls are what it is for. + /// + /// It cannot stay for water, because the corridor's clearance is a reference constant sized + /// for a ray. Arm A lifts the sweep origin to `surface + 2/9 = 0.2222` yd, and the probe's + /// radius is `0.3`: centred on that floor the sphere hangs **78 mm through the water plane + /// before the sweep has moved at all**, so a level camera behind a surface swimmer was returned + /// a hit at distance zero — the camera slammed onto the character, every frame the boom ran + /// flat. Tip it a couple of degrees down and the sweep escapes the plane and the hit vanishes; + /// that flip, against a snap-in/ease-out arm, is the "snaps in for a second every couple of + /// seconds" a surface swim was reported with. + /// + /// Two-sided, like the shape sweep beside it and for the same reason: a camera crossing the + /// surface from underneath has no facing contract to honour, it just must not end up through + /// the plane. + fn waterline_ray(&self, from: Vec3, movement: Vec3) -> Option { + let Ok(dir) = Dir3::new(movement) else { + return None; + }; + self.ms + .spatial_query + .cast_ray( + from, + dir, + movement.length(), + true, + &SpatialQueryFilter::from_mask(LayerMask(CollisionLayer::Liquid.to_bits())), + ) + .map(|h| h.distance) } /// Sweep `shape` along `movement` against the body's world. @@ -246,32 +298,46 @@ impl WorldCollision<'_, '_> { one_sided::cast_move(&self.ms, shape, from, movement, skin_width, filter) } - /// Sweep the **camera boom** against the camera's world. + /// Sweep the **camera boom** against the camera's world — the distance along `movement` the + /// arm is free to reach, or `None` for an unobstructed boom. /// /// Deliberately avian's own two-sided cast rather than the one-sided law above: the facing /// gate exists because a *body* must not stand on a face wound away from it, and a camera has /// no such contract — it just must not end up inside geometry, from either side. /// - /// `liquid` is `cameraWaterCollision`, straight through: with it set the boom stops at a - /// waterline the way it stops at ground, which is the whole of the option's primary effect and - /// what its own tooltip promises the player. - pub fn cast_camera( - &self, - shape: &Collider, - from: Vec3, - rotation: Quat, - movement: Vec3, - skin_width: f32, - liquid: bool, - ) -> Option { - self.ms.cast_move( - shape, - from, - rotation, - movement, - skin_width, - &Self::camera_filter(liquid), - ) + /// **Two legs, two shapes, and that is the point** (decision 2185). The solid world is swept + /// with the caller's probe sphere, which is benilla's own margin against a wall's near plane; + /// the waterline is a bare ray, which is the reference's own geometry and the only shape that + /// fits through the clearance the corridor budgets for it — see + /// [`waterline_ray`](Self::waterline_ray). `liquid` is `cameraWaterCollision`, straight + /// through: with it clear the water leg does not run and this is the query it always was. + /// + /// The nearer of the two wins, which is what one trace carrying both classes would have done. + /// + /// **The probe is this function's, not the caller's.** It used to be threaded in from a + /// `CameraProbe` resource in `benilla-app`, along with a rotation that was always the identity + /// (a sphere has no orientation) and a skin width that was always zero — three arguments of + /// noise around one number that belongs to the trace. That number is [`CAMERA_PROBE_RADIUS`], + /// and the whole of decision 2185 is that it must be read in the same place the clearance it + /// has to fit through is (`crates/benilla-app/tests/world_api_wall.rs` is what made the point: + /// exporting the constant widened the doorway, and owning the probe closes it instead). + pub fn cast_camera(&self, from: Vec3, movement: Vec3, liquid: bool) -> Option { + let solid = self + .ms + .cast_move( + &Collider::sphere(CAMERA_PROBE_RADIUS), + from, + Quat::IDENTITY, + movement, + 0.0, + &Self::camera_filter(), + ) + .map(|h| h.distance); + let water = liquid.then(|| self.waterline_ray(from, movement)).flatten(); + match (solid, water) { + (Some(a), Some(b)) => Some(a.min(b)), + (a, b) => a.or(b), + } } /// A one-sided ray against the body's world. @@ -410,6 +476,7 @@ mod liquid_trace_mask { PhysicsPlugins::new(bevy::app::PostUpdate), )); app.init_asset::(); + app.init_resource::(); app.world_mut().spawn(( RigidBody::Static, Collider::trimesh( @@ -430,17 +497,24 @@ mod liquid_trace_mask { app } - /// Drop a small probe from above the surface onto it, through `filter`. - fn descend(app: &mut App, filter: SpatialQueryFilter) -> Option { + /// Drop the camera boom from above the surface straight onto it. + fn descend_camera(app: &mut App, liquid: bool) -> Option { app.world_mut() - .run_system_once(move |ms: MoveAndSlide| { - ms.cast_move( - &Collider::sphere(0.1), + .run_system_once(move |c: WorldCollision| { + c.cast_camera(Vec3::new(0.0, 3.0, 0.0), Vec3::new(0.0, -6.0, 0.0), liquid) + }) + .expect("system runs") + } + + /// …and the walking body, which must pass through either way. + fn descend_body(app: &mut App) -> Option { + app.world_mut() + .run_system_once(move |c: WorldCollision| { + c.cast_body( + &Collider::sphere(CAMERA_PROBE_RADIUS), Vec3::new(0.0, 3.0, 0.0), - Quat::IDENTITY, Vec3::new(0.0, -6.0, 0.0), 0.0, - &filter, ) .map(|h| h.distance) }) @@ -514,25 +588,48 @@ mod liquid_trace_mask { /// The arm length the solver settles on for a swimmer whose feet are at `feet_y`, framing at /// `pivot_over_feet`, sweeping from `origin_over_feet`, looking at `pitch` radians (+up). fn open_arm(app: &mut App, feet_y: f32, geo: (f32, f32), pitch: f32) -> f32 { + let (head, boom, len) = boom_from(feet_y, geo, pitch); + app.world_mut() + .run_system_once(move |c: WorldCollision| { + c.cast_camera(head, boom, true).unwrap_or(len) + }) + .expect("system runs") + } + + /// The same arm through the query benilla shipped **before** decision 2185: one sphere sweep + /// carrying the liquid layer on its own mask, exactly as `camera_filter(true)` built it. + /// + /// It stays as the positive control for both of the tests below, because it is the query in + /// which the defect lives — and a gate whose control cannot fail is not a gate. + fn open_arm_swept(app: &mut App, feet_y: f32, geo: (f32, f32), pitch: f32) -> f32 { + let (head, boom, len) = boom_from(feet_y, geo, pitch); + app.world_mut() + .run_system_once(move |ms: MoveAndSlide| { + ms.cast_move( + &Collider::sphere(CAMERA_PROBE_RADIUS), + head, + Quat::IDENTITY, + boom, + 0.0, + &SpatialQueryFilter::from_mask(LayerMask( + CollisionLayer::Default.to_bits() + | CollisionLayer::Camera.to_bits() + | CollisionLayer::Liquid.to_bits(), + )), + ) + .map_or(len, |h| h.distance) + }) + .expect("system runs") + } + + /// `(sweep origin, boom vector, its length)` for a swimmer at `feet_y` framing at `geo`. + fn boom_from(feet_y: f32, geo: (f32, f32), pitch: f32) -> (Vec3, Vec3, f32) { let feet = Vec3::new(0.0, feet_y, 0.0); let head = feet + Vec3::Y * geo.1; let pivot = feet + Vec3::Y * geo.0; let fwd = Quat::from_euler(EulerRot::YXZ, 0.0, pitch, 0.0) * Vec3::NEG_Z; let boom = (pivot - fwd * ZOOM) - head; - let len = boom.length(); - app.world_mut() - .run_system_once(move |c: WorldCollision| { - c.cast_camera( - &Collider::sphere(0.15), - head, - Quat::IDENTITY, - boom, - 0.0, - true, - ) - .map_or(len, |h| h.distance) - }) - .expect("system runs") + (head, boom, boom.length()) } /// 2170's geometry at a given depth: the framing pivot is the bare swim preset, so it rides the @@ -553,6 +650,7 @@ mod liquid_trace_mask { /// given pitch. fn worst_over_depth( app: &mut App, + arm: impl Fn(&mut App, f32, (f32, f32), f32) -> f32, geo: impl Fn(f32) -> (f32, f32), pitch: f32, span: f32, @@ -562,7 +660,7 @@ mod liquid_trace_mask { let mut prev: Option = None; for i in 0..=n { let feet_y = -SURFACE_DEPTH - span * 0.5 + span * i as f32 / n as f32; - let d = open_arm(app, feet_y, geo(feet_y), pitch); + let d = arm(app, feet_y, geo(feet_y), pitch); if let Some(q) = prev { worst = worst.max((d - q).abs()); } @@ -595,7 +693,14 @@ mod liquid_trace_mask { let mut app = flooded_world(); // The control, at the pitch where the old geometry is worst: looking slightly down, which // swings the seat up across the plane. - let control = worst_over_depth(&mut app, uncorrected, (-2.0f32).to_radians(), 0.30, 600); + let control = worst_over_depth( + &mut app, + open_arm_swept, + uncorrected, + (-2.0f32).to_radians(), + 0.30, + 600, + ); assert!( control > 5.0, "the positive control must reproduce the regression — half a millimetre of settle \ @@ -603,7 +708,14 @@ mod liquid_trace_mask { ); // …and the corridor, across the whole range a swimmer looks through. for pitch_deg in [-25.0f32, -10.0, -2.0, -0.5, 0.0, 0.5, 2.0, 10.0, 25.0] { - let step = worst_over_depth(&mut app, corrected, pitch_deg.to_radians(), 0.30, 600); + let step = worst_over_depth( + &mut app, + open_arm, + corrected, + pitch_deg.to_radians(), + 0.30, + 600, + ); assert!( step < 0.1, "at {pitch_deg} deg the settle moved the arm {step} yd — the corridor's whole \ @@ -612,21 +724,77 @@ mod liquid_trace_mask { } } + /// **The probe that did not fit through its own corridor** (decision 2185). + /// + /// The corridor lifts the camera's sweep origin to `surface + 2/9 = 0.2222` yd. That is a + /// **reference** constant, and the reference's trace is a bare ray: `0x672170` takes a start, + /// an end, an out-point and a `frac` and carries no radius anywhere, bottoming out in the + /// Möller–Trumbore ray/triangle test at `0x7c2c40`. benilla's camera probe is a + /// [`CAMERA_PROBE_RADIUS`] = `0.3` yd sphere — **our own** construction, the margin that keeps + /// the near plane out of a wall. `0.3 > 0.2222`, so centred on the corridor floor it hangs 78 + /// mm through the water plane *before the sweep has moved at all*. + /// + /// A surface swimmer's boom runs from that origin to a seat at the same height — the corridor + /// pins both to `surface + 2/9` — so a **level** camera sweeps exactly parallel to the water, + /// and came back pinned at zero: the camera slammed onto the character. Tip it two degrees + /// down and the sphere escapes the plane and the hit vanishes. That flip, against an arm that + /// snaps in instantly and eases back out over about a second, is the "snaps in for a second + /// every couple of seconds" a surface swim was reported with — and it is the same mechanism + /// 2170 was reported for and 2179 could not reach, because the corridor it built is measured + /// in a clearance this probe was always too fat for. + #[test] + fn a_level_boom_behind_a_surface_swimmer_is_not_pinned_to_the_water() { + let mut app = flooded_world(); + let feet_y = -SURFACE_DEPTH; + let geo = corrected(feet_y); + + // The control, which must reproduce the defect: the pre-2185 query, level. + let control = open_arm_swept(&mut app, feet_y, geo, 0.0); + assert!( + control < 0.01, + "the positive control must reproduce the regression — a level boom swept as a sphere \ + starts inside the plane and comes back pinned at zero, got {control}" + ); + + // …and the shipping query, across the band a surface swimmer actually looks through. + for deg in [-25.0f32, -10.0, -2.0, -0.5, 0.0, 0.25, 0.5] { + let open = open_arm(&mut app, feet_y, geo, deg.to_radians()); + assert!( + open > ZOOM - 0.01, + "at {deg} deg the arm came back clipped at {open} — the water is 2/9 yd below a \ + boom that never descends to it" + ); + } + + // …and the option still does its job when the camera really is aimed under the water: + // 2/9 yd of clearance shed at sin(20 deg) crosses the plane 0.65 yd along the boom. + let aimed = open_arm(&mut app, feet_y, geo, 20.0f32.to_radians()); + assert!( + (aimed - 0.65).abs() < 0.05, + "aimed 20 deg up the boom must still stop ON the surface, got {aimed}" + ); + } + #[test] fn the_waterline_stops_the_camera_only_when_the_cvar_asks_for_it() { let mut app = world_with_waterline(); - let on = descend(&mut app, WorldCollision::camera_filter(true)); + let on = descend_camera(&mut app, true); assert!( - on.is_some_and(|d| (d - 2.9).abs() < 0.05), + on.is_some_and(|d| (d - 3.0).abs() < 0.01), "with cameraWaterCollision the boom stops at the surface, got {on:?}" ); + assert!( + on.is_some_and(|d| d > 2.95), + "and it stops ON the plane, not a probe radius short of it — the water leg is a ray \ + (decision 2185), which is what makes the corridor's 2/9 yd of clearance a clearance" + ); assert_eq!( - descend(&mut app, WorldCollision::camera_filter(false)), + descend_camera(&mut app, false), None, "with the CVar off the camera passes through, exactly as it did before this existed" ); assert_eq!( - descend(&mut app, WorldCollision::body_filter()), + descend_body(&mut app), None, "and the BODY passes through either way — a swimmer is not stopped by their own water" );