From 3b3ae850fa2350a381eeb189a09fc4c4e857459f Mon Sep 17 00:00:00 2001 From: Sam Sung <51686767+samwhosung@users.noreply.github.com> Date: Wed, 30 Sep 2026 03:09:50 +0700 Subject: [PATCH] spell range: min_max_range takes the unit target and the auto-attack target apart, so a tooltip no longer pads a ranged spell by the swing target's reach (#654) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `min_max_range` took one `target_reach` that served two roles: the melee arm's fallback to the auto-attack target and the ranged arm's explicit target. The ranged arm padded whenever it was `Some`. The tooltip passed the auto-attack target's reach, so while you swung, every ranged spell's tooltip read its row plus both reaches: Fireball read "38 yd range" where 1.12.1 reads 35. `GetMinMaxRange 0x6e3480` counts its target (`[ebx+0x10]`) only when it is a unit (`0x6e34e1`-`0x6e34f9`). - **Melee arm** (`0x6e3552`-`0x6e3594`): with no unit target, it falls back to the caster's auto-attack target (`0x6e356a`), then to the caster's own reach. - **Ranged arm:** pads only for a non-null target (`0x6e35d8`, `0x6e35ec`). The max always takes the pad, and the min only when nonzero. What each caller passes: | Caller | Call site | Target | |---|---|---| | Tooltip | `0x52e9c2` | none | | `ActionHasRange` | `0x4e5852` | none | | The cast check and `IsActionInRange` | through `CanTargetUnit 0x6e4440` → `0x6e47b0` | the bound or selected target | Changes: - `min_max_range` takes `RangeTargets { target, attack_target }`. - The melee arm reads the target, else the attack target, else the caster. The ranged arm pads for the target alone. - Each caller passes what its reference call site passes. The action button's two verbs are now two calls: `has_range` with no target and `in_range` with the selected one. - `min_max_range_reads_reach` narrows to melee rows. The tooltip safety net then counts 813 reach-dependent views instead of 10,298, so starting to swing no longer rebuilds every ranged tooltip. Tests: - `ranges.rs`: `the_ranged_arm_pads_only_for_the_explicit_target`, `the_melee_arm_falls_back_from_the_target_to_the_attack_target_to_the_caster`, and the narrowed exactness test. - `range_cell_on_real_data` reads Fireball as "35 yd range" and Charge as "8-25 yd range" with an auto-attack target present. - `cast_range_refusal_follows_the_two_compares` covers a bound target's pad. - `a_ranged_button_pads_its_range_by_the_selected_target_alone`. Fixes #643 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/benilla-app/src/spell/mods.rs | 16 +- crates/benilla-app/src/spell/validator.rs | 19 +- crates/benilla-app/src/ui_action/state.rs | 141 ++++++++++++- .../benilla-app/src/ui_tooltip/spell_deps.rs | 3 +- .../benilla-app/src/ui_tooltip/spell_feed.rs | 19 +- crates/benilla-app/src/ui_tooltip/tests.rs | 17 ++ crates/benilla-formats/src/lib.rs | 18 +- crates/benilla-formats/src/spells/mod.rs | 4 +- crates/benilla-formats/src/spells/ranges.rs | 187 +++++++++++++----- 9 files changed, 336 insertions(+), 88 deletions(-) diff --git a/crates/benilla-app/src/spell/mods.rs b/crates/benilla-app/src/spell/mods.rs index 7208ee5f..80a2d8b4 100644 --- a/crates/benilla-app/src/spell/mods.rs +++ b/crates/benilla-app/src/spell/mods.rs @@ -234,9 +234,9 @@ impl SpellModifiers { d: &SpellDisplay, row: Option<&SpellRange>, caster_reach: f32, - target_reach: Option, + targets: benilla_formats::RangeTargets, ) -> Option<(f32, f32)> { - let (min, max) = benilla_formats::min_max_range(d, row, caster_reach, target_reach)?; + let (min, max) = benilla_formats::min_max_range(d, row, caster_reach, targets)?; Some(( min, if d.on_next_swing() { @@ -521,18 +521,22 @@ mod tests { flags: 0, }; mods.set(false, 5, OP_RANGE, 20); - let (min, max) = mods.min_max_range(&d, Some(&row), 1.5, Some(1.5)).unwrap(); + let target = benilla_formats::RangeTargets { + target: Some(1.5), + attack_target: None, + }; + let (min, max) = mods.min_max_range(&d, Some(&row), 1.5, target).unwrap(); assert_eq!(min, 11.0); assert!((max - 45.6).abs() < 0.001, "38 yards with +20% range"); - assert_eq!(mods.min_max_range(&d, None, 1.5, Some(1.5)), None); + assert_eq!(mods.min_max_range(&d, None, 1.5, target), None); let on_next_swing = SpellDisplay { attributes: 0x404, ..d }; assert_eq!( - mods.min_max_range(&on_next_swing, Some(&row), 1.5, Some(1.5)), - benilla_formats::min_max_range(&on_next_swing, Some(&row), 1.5, Some(1.5)), + mods.min_max_range(&on_next_swing, Some(&row), 1.5, target), + benilla_formats::min_max_range(&on_next_swing, Some(&row), 1.5, target), "the reference's early return bypasses op 5" ); } diff --git a/crates/benilla-app/src/spell/validator.rs b/crates/benilla-app/src/spell/validator.rs index 426160c2..7eba0f9f 100644 --- a/crates/benilla-app/src/spell/validator.rs +++ b/crates/benilla-app/src/spell/validator.rs @@ -14,7 +14,10 @@ pub(super) const ERR_TOO_CLOSE: u8 = 0x76; /// The range refusal, before `ArmCast`/`SendCast`, so an out-of-range press never runs the commit /// tail (the ranged sheath snap `0x6e5930` included): squared 3D distance against /// [`benilla_formats::min_max_range`], beyond max² out of range, inside a nonzero min² too close. -/// No range row or no distance passes; the server judges. +/// `target_reach` is the bound unit's, the target `IsTargetInRange 0x6e47b0` hands +/// `GetMinMaxRange` (`0x6e47ca`). The function looks the auto-attack target up only when that +/// target is no unit, which a bound unit never is, so none is passed. No range row or no +/// distance passes; the server judges. pub(super) fn cast_range_refusal( spell: &SpellDisplay, row: Option<&SpellRange>, @@ -23,7 +26,11 @@ pub(super) fn cast_range_refusal( dist_sq: Option, mods: &SpellModifiers, ) -> Option { - let (min, max) = mods.min_max_range(spell, row, self_reach, target_reach)?; + let targets = benilla_formats::RangeTargets { + target: target_reach, + attack_target: None, + }; + let (min, max) = mods.min_max_range(spell, row, self_reach, targets)?; let d2 = dist_sq?; if d2 > max * max { return Some(ERR_OUT_OF_RANGE); @@ -261,6 +268,14 @@ mod tests { let refuse = |d2: f32| cast_range_refusal(&d, Some(&fireball), 1.5, reach, Some(d2), &mods); assert_eq!(refuse(0.1), None); assert_eq!(refuse(60.0 * 60.0), Some(ERR_OUT_OF_RANGE)); + // The bound target is the pad's second reach (`0x6e47ca`): 35 + 1.5 + 1.5 = 38. + assert_eq!(refuse(37.0 * 37.0), None); + assert_eq!(refuse(39.0 * 39.0), Some(ERR_OUT_OF_RANGE)); + // With no unit bound the row stands as it is: 35. + assert_eq!( + cast_range_refusal(&d, Some(&fireball), 1.5, None, Some(37.0 * 37.0), &mods), + Some(ERR_OUT_OF_RANGE) + ); // A melee row has min 0: never too close. let melee = SpellRange { diff --git a/crates/benilla-app/src/ui_action/state.rs b/crates/benilla-app/src/ui_action/state.rs index f99ee43b..e5f49d2c 100644 --- a/crates/benilla-app/src/ui_action/state.rs +++ b/crates/benilla-app/src/ui_action/state.rs @@ -12,6 +12,7 @@ use std::time::Instant; use bevy::prelude::*; +use benilla_formats::RangeTargets; use benilla_protocol::messages::{ACTION_KIND_ITEM, ACTION_KIND_MACRO, ACTION_KIND_SPELL}; use benilla_ui::script::{ActionState, UiScript}; @@ -213,14 +214,25 @@ pub(super) fn feed_action_state( } else { st.usable = true; } - // The range verdict against the target (`0x4e56f0`); nil without one. let row = spells.as_ref().and_then(|s| s.ranges.get(d.range_index)); - let resolved = spell_mods.min_max_range(d, row, self_reach, target_reach); - st.has_range = resolved + // `ActionHasRange 0x4e5810` asks `GetMinMaxRange` with a null target (`0x4e5852`) + // and tests `|min|` and `|max|` against `FLT_EPSILON`. The auto-attack target the + // melee arm falls back to cannot move that: its max is at least the 5.0 floor. + st.has_range = spell_mods + .min_max_range(d, row, self_reach, RangeTargets::default()) .is_some_and(|(min, max)| min.abs() > f32::EPSILON || max.abs() > f32::EPSILON); - st.in_range = match (resolved, dist_sq) { - (Some((min, max)), Some(d2)) if st.has_range => { - Some(d2 >= min * min && d2 <= max * max) + // `IsActionInRange 0x4e56f0` runs `CanTargetUnit 0x6e4440`, which binds the selected + // target, then `IsTargetInRange 0x6e47b0`, which hands `GetMinMaxRange` that target + // (`0x6e47ca`): its reach pads a ranged row. Nil without a target. + st.in_range = match dist_sq { + Some(d2) if st.has_range => { + let targets = RangeTargets { + target: target_reach, + attack_target: None, + }; + spell_mods + .min_max_range(d, row, self_reach, targets) + .map(|(min, max)| d2 >= min * min && d2 <= max * max) } _ => None, }; @@ -477,6 +489,123 @@ mod tests { ); } + /// `IsActionInRange` (`0x4e56f0`) hands `GetMinMaxRange` the selected unit, so a ranged row + /// pads by that unit's reach and the player's alone: a Fireball (0-35) button reads in range + /// out to 38 yards from a 1.5-reach target, whoever the player swings at, and `ActionHasRange` + /// (`0x4e5810`, a null target) says the button has a range. + #[test] + fn a_ranged_button_pads_its_range_by_the_selected_target_alone() { + use benilla_protocol::messages::ActionButton; + use benilla_protocol::ObjectFields; + + const FIREBALL: u32 = 133; + const SWUNG_AT: u64 = 0xA1; + const SELECTED: u64 = 0xB2; + /// `UNIT_FIELD_COMBATREACH`. + const REACH: u16 = 130; + + let verdict = |distance: f32| { + let (tx, _rx) = crossbeam_channel::unbounded(); + let mut app = App::new(); + let mut actions = PlayerActions::default(); + actions.buttons.insert( + 0, + ActionButton { + slot: 0, + action: FIREBALL, + kind: ACTION_KIND_SPELL, + }, + ); + let fireball = SpellDisplay { + range_index: 4, + ..Default::default() + }; + let row = benilla_formats::SpellRange { + min: 0.0, + max: 35.0, + flags: 0, + }; + app.insert_resource(actions) + .insert_resource(crate::ui_macro::MacroBoundSpells::default()) + .insert_resource(Spells { + catalog: benilla_formats::SpellCatalog::from_displays( + [(FIREBALL, fireball)].into_iter().collect(), + ), + forms: Default::default(), + ranges: benilla_formats::SpellRangeCatalog::from_rows( + [(4, row)].into_iter().collect(), + ), + cast_times: Default::default(), + durations: Default::default(), + radii: Default::default(), + }) + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .insert_resource(NetCommands(tx)); + // The player swings at a 4.0-reach unit, close by. + let swung_at = app + .world_mut() + .spawn(( + Transform::from_xyz(2.0, 0.0, 0.0), + ObjectStore(ObjectFields::from_pairs(&[ + (22, 100), + (REACH, 4.0f32.to_bits()), + ])), + )) + .id(); + app.world_mut().spawn(( + SelfPlayer, + Engaged(SWUNG_AT), + Transform::default(), + ObjectStore(ObjectFields::from_pairs(&[(22, 100), (23, 500)])), + )); + // The selected unit keeps the descriptor's default 1.5 reach. + let selected = app + .world_mut() + .spawn(( + Transform::from_xyz(distance, 0.0, 0.0), + ObjectStore(ObjectFields::from_pairs(&[(22, 100)])), + )) + .id(); + let mut index = app.world_mut().resource_mut::(); + index.0.insert(SWUNG_AT, swung_at); + index.0.insert(SELECTED, selected); + app.insert_resource(Selection { + target: Some(selected), + guid: Some(SELECTED), + last: None, + }); + app.insert_non_send_resource(UiScript::new().unwrap()); + app.add_systems(Update, feed_action_state); + app.update(); + let script = app.world().non_send_resource::(); + assert!(script + .eval::("return ActionHasRange(1) == 1") + .unwrap()); + script + .eval::("return IsActionInRange(1) or -1") + .unwrap() + }; + + // 35 + the caster's 1.5 + the selected unit's 1.5 = 38. + assert_eq!(verdict(37.5), 1, "inside the padded max"); + assert_eq!( + verdict(38.5), + 0, + "past it: the swing's 4.0 reach pads nothing" + ); + } + /// An item slot's on-use spell takes the `0x6e3d60` walk (`0x4e5050`), and every shipped /// Food/Drink spell has `Attributes` bit 28, `ATTR_NOT_IN_COMBAT` (the walk's leg 8). #[test] diff --git a/crates/benilla-app/src/ui_tooltip/spell_deps.rs b/crates/benilla-app/src/ui_tooltip/spell_deps.rs index b933931b..9dba4386 100644 --- a/crates/benilla-app/src/ui_tooltip/spell_deps.rs +++ b/crates/benilla-app/src/ui_tooltip/spell_deps.rs @@ -124,7 +124,8 @@ pub(super) struct Deps { pub(super) home: bool, /// The required-form test read the shapeshift form. pub(super) form: bool, - /// The range cell read the caster's or the auto-attack target's combat reach. + /// The range cell's melee arm read the caster's or the auto-attack target's combat reach; + /// the ranged arm, given no target, reads neither. pub(super) reach: bool, /// The chance line read these percentages, a [`Chance`] bit each. pub(super) avoidance: u8, diff --git a/crates/benilla-app/src/ui_tooltip/spell_feed.rs b/crates/benilla-app/src/ui_tooltip/spell_feed.rs index 928529b1..dd0701ce 100644 --- a/crates/benilla-app/src/ui_tooltip/spell_feed.rs +++ b/crates/benilla-app/src/ui_tooltip/spell_feed.rs @@ -55,8 +55,8 @@ pub(super) struct ViewCtx<'a, 'w, 's> { pub(super) caster: ViewCaster<'a>, /// The caster's `UNIT_FIELD_COMBATREACH`; 1.5 is the descriptor default. pub(super) combat_reach: f32, - /// The caster's auto-attack target's reach, which `0x6e3480` reads itself (`[caster+0xc48]`); - /// with none, the caster's reach counts twice. + /// The caster's auto-attack target's reach, which `0x6e3480` looks up itself + /// (`[caster+0xc48]`) for its melee arm alone; with none, the caster's reach counts twice. pub(super) attack_target_reach: Option, /// The object index the worn-item search and each reagent's carried count resolve through. pub(super) objects: &'a Objects<'w, 's>, @@ -190,12 +190,15 @@ pub(super) fn build_view( { deps.borrow_mut().reach = true; } - let (min, max) = vctx.spell_mods.min_max_range( - d, - row, - vctx.combat_reach, - vctx.attack_target_reach, - )?; + // The call passes a null target (`0x52e9c2`): the auto-attack target is the melee arm's + // own lookup, and the ranged arm, with no target, pads nothing. + let targets = benilla_formats::RangeTargets { + target: None, + attack_target: vctx.attack_target_reach, + }; + let (min, max) = vctx + .spell_mods + .min_max_range(d, row, vctx.combat_reach, targets)?; if max <= 0.0 { return None; } diff --git a/crates/benilla-app/src/ui_tooltip/tests.rs b/crates/benilla-app/src/ui_tooltip/tests.rs index 8f8084d4..dcf9f941 100644 --- a/crates/benilla-app/src/ui_tooltip/tests.rs +++ b/crates/benilla-app/src/ui_tooltip/tests.rs @@ -621,6 +621,23 @@ fn range_cell_on_real_data() { .expect("Fireball view"); assert_eq!(v.range.as_deref(), Some("35 yd range")); + // The tooltip passes a null target (`0x52e9c2`), so the auto-attack target pads no ranged row: + // Fireball still reads its 35 while you swing at a 1.5-reach unit, and Charge its 8-25. + let v = spell_tooltip_view( + 133, + &spells, + &mut t.ctx_engaged(&objects, Some(&store), 1.5), + ) + .expect("Fireball view"); + assert_eq!(v.range.as_deref(), Some("35 yd range")); + let v = spell_tooltip_view( + 100, + &spells, + &mut t.ctx_engaged(&objects, Some(&store), 1.5), + ) + .expect("Charge view"); + assert_eq!(v.range.as_deref(), Some("8-25 yd range")); + // An authored pair, `"%d-%d"`: Charge's 8-25, which the reach does not change. let v = spell_tooltip_view(100, &spells, &mut t.ctx_for(&objects, 0, None, Some(&big))) .expect("Charge view"); diff --git a/crates/benilla-formats/src/lib.rs b/crates/benilla-formats/src/lib.rs index b04d3893..f10634c1 100644 --- a/crates/benilla-formats/src/lib.rs +++ b/crates/benilla-formats/src/lib.rs @@ -198,15 +198,15 @@ pub use spells::{ cc_exemption, grants_immunity, load_shapeshift_forms, load_spell_cast_times, load_spell_catalog, load_spell_dispel_types, load_spell_durations, load_spell_radii, load_spell_ranges, min_max_range, min_max_range_reads_reach, soft_modify, substitute, - CcExemption, FormRefusal, LearnAnnouncement, LearnEffect, OpenLock, ShapeshiftForm, - SpellCastTime, SpellCastTimeCatalog, SpellCatalog, SpellDispelTypes, SpellDisplay, - SpellDuration, SpellDurationCatalog, SpellMods, SpellRadius, SpellRadiusCatalog, SpellRange, - SpellRangeCatalog, TokenContext, TokenNumber, ATTR_CASTABLE_WHILE_DEAD, ATTR_NOT_IN_COMBAT, - ATTR_ONLY_STEALTHED, COMBAT_REACH_ADD, MELEE_RANGE_FLOOR, ON_NEXT_SWING_RANGE, - SPELL_ATTR_IS_TRADESKILL, SPELL_EFFECT_CREATE_ITEM, SPELL_EFFECT_ENCHANT_ITEM, - SPELL_EFFECT_ENCHANT_ITEM_TEMPORARY, SPELL_EFFECT_LEARN_PET_SPELL, SPELL_EFFECT_LEARN_SPELL, - SPELL_EFFECT_PROSPECTING, SPELL_EFFECT_SKILL_STEP, SPELL_EFFECT_SKINNING, - SPELL_EFFECT_TRADE_SKILL, + CcExemption, FormRefusal, LearnAnnouncement, LearnEffect, OpenLock, RangeTargets, + ShapeshiftForm, SpellCastTime, SpellCastTimeCatalog, SpellCatalog, SpellDispelTypes, + SpellDisplay, SpellDuration, SpellDurationCatalog, SpellMods, SpellRadius, SpellRadiusCatalog, + SpellRange, SpellRangeCatalog, TokenContext, TokenNumber, ATTR_CASTABLE_WHILE_DEAD, + ATTR_NOT_IN_COMBAT, ATTR_ONLY_STEALTHED, COMBAT_REACH_ADD, MELEE_RANGE_FLOOR, + ON_NEXT_SWING_RANGE, SPELL_ATTR_IS_TRADESKILL, SPELL_EFFECT_CREATE_ITEM, + SPELL_EFFECT_ENCHANT_ITEM, SPELL_EFFECT_ENCHANT_ITEM_TEMPORARY, SPELL_EFFECT_LEARN_PET_SPELL, + SPELL_EFFECT_LEARN_SPELL, SPELL_EFFECT_PROSPECTING, SPELL_EFFECT_SKILL_STEP, + SPELL_EFFECT_SKINNING, SPELL_EFFECT_TRADE_SKILL, }; mod skill_lines; pub use skill_lines::{ diff --git a/crates/benilla-formats/src/spells/mod.rs b/crates/benilla-formats/src/spells/mod.rs index 2bf479bf..2cfad737 100644 --- a/crates/benilla-formats/src/spells/mod.rs +++ b/crates/benilla-formats/src/spells/mod.rs @@ -32,8 +32,8 @@ mod immunity; pub use immunity::{cc_exemption, grants_immunity, CcExemption}; pub use radius::{load_spell_radii, SpellRadius, SpellRadiusCatalog}; pub use ranges::{ - load_spell_ranges, min_max_range, min_max_range_reads_reach, SpellRange, SpellRangeCatalog, - COMBAT_REACH_ADD, MELEE_RANGE_FLOOR, ON_NEXT_SWING_RANGE, + load_spell_ranges, min_max_range, min_max_range_reads_reach, RangeTargets, SpellRange, + SpellRangeCatalog, COMBAT_REACH_ADD, MELEE_RANGE_FLOOR, ON_NEXT_SWING_RANGE, }; pub use soft_float::modify as soft_modify; pub use tokens::{substitute, SpellMods, TokenContext, TokenNumber}; diff --git a/crates/benilla-formats/src/spells/ranges.rs b/crates/benilla-formats/src/spells/ranges.rs index 7eca08ae..21b055e7 100644 --- a/crates/benilla-formats/src/spells/ranges.rs +++ b/crates/benilla-formats/src/spells/ranges.rs @@ -33,14 +33,27 @@ pub const MELEE_RANGE_FLOOR: f32 = 5.0; /// The on-next-swing short-circuit's max, `[0x8118d4]` (`0x6e3504`). pub const ON_NEXT_SWING_RANGE: f32 = 100.0; +/// The combat reaches `GetMinMaxRange` (`0x6e3480`) reads besides the caster's own, `None` where +/// that unit is absent. The two are different inputs: only the first is an argument of the call. +#[derive(Clone, Copy, Debug, Default, PartialEq)] +pub struct RangeTargets { + /// The call's `target` argument (`[ebx+0x10]`), when it is a unit (typemask bit 3, + /// `0x6e34e1`-`0x6e34f9`): a null target, and one that is no unit, read `None`. Both arms + /// read its reach, and the ranged arm pads only for a target passed in (`0x6e35d8`). + pub target: Option, + /// The caster's auto-attack target, `[caster+0xc48]`, which the function looks up itself + /// (`0x47bf60`, `0x6e356a`). Only the melee arm reads it, and only with no unit target passed + /// (`0x6e3552`-`0x6e3584`). A caller that has no such unit to hand passes `None`. + pub attack_target: Option, +} + /// `GetMinMaxRange` (`0x6e3480`): a spell's `{min, max}` cast range, summed in `f64` because the /// client keeps the reach sums on the x87 stack and stores `f32` only at the end. On-next-swing /// spells (`Attributes & 0x404`, `0x6e34fb`) short-circuit to `(0, 100)`. The melee arm sums the -/// target's reach, else the caster's own a second time, and reads the auto-attack target itself -/// (`attack_target_guid`, `0x6e356a`), so a caller with no explicit target passes that unit's -/// reach. Given a target, the ranged arm pads the max, and the min only when nonzero (the -/// `fcomp`-vs-0.0 guard), so a min-0 spell never refuses `TOO_CLOSE`. `None`: no row, or the self -/// row (id 1, `{0, 0}`). +/// reach of the unit target, else the auto-attack target, else the caster's own, with the +/// caster's. The ranged arm pads only for a unit target passed in, the auto-attack target never +/// pads it: the max always, and the min only when nonzero (the `fcomp`-vs-0.0 guard), so a min-0 +/// spell never refuses `TOO_CLOSE`. `None`: no row, or the self row (id 1, `{0, 0}`). /// /// Not applied: the PvP `max += 2.6667` (`0x6e3648`, gated by `0x5fc350`), and for a player the /// `Attributes & 2` scale (`0x6e36aa`), `max *= RangedModRange · 0.01` of the ranged-slot item, @@ -50,14 +63,17 @@ pub fn min_max_range( spell: &crate::spells::SpellDisplay, row: Option<&SpellRange>, caster_reach: f32, - target_reach: Option, + targets: RangeTargets, ) -> Option<(f32, f32)> { if spell.on_next_swing() { return Some((0.0, ON_NEXT_SWING_RANGE)); } let row = row?; - let reach = target_reach.unwrap_or(caster_reach); if row.is_melee() { + let reach = targets + .target + .or(targets.attack_target) + .unwrap_or(caster_reach); let sum = f64::from(reach) + f64::from(caster_reach) + f64::from(COMBAT_REACH_ADD); let max = if sum > f64::from(MELEE_RANGE_FLOOR) { sum as f32 @@ -69,7 +85,7 @@ pub fn min_max_range( if row.min == 0.0 && row.max == 0.0 { return None; } - let Some(target_reach) = target_reach else { + let Some(target_reach) = targets.target else { return Some((row.min, row.max)); }; let pad = f64::from(caster_reach) + f64::from(target_reach); @@ -81,18 +97,17 @@ pub fn min_max_range( Some((min, (f64::from(row.max) + pad) as f32)) } -/// Whether [`min_max_range`] reads either reach for this spell and row: every arm but the -/// on-next-swing short-circuit, the missing row and the self row `{0, 0}`. The melee arm sums both; -/// the ranged arm pads by both, or by neither when there is no target, so a change of either reach -/// can move the result. +/// Whether [`min_max_range`] reads a reach for this spell and row when it is passed no unit +/// target, as the spell tooltip calls it (`push 0`, `0x52e9c2`): only the melee arm, which sums +/// the auto-attack target's reach, else the caster's own, with the caster's. The ranged arm pads +/// only for a target passed in, so a tooltip prints its row as it is; the on-next-swing +/// short-circuit, the missing row and the self row read none. With a unit target the ranged arm +/// reads both reaches too. pub fn min_max_range_reads_reach( spell: &crate::spells::SpellDisplay, row: Option<&SpellRange>, ) -> bool { - if spell.on_next_swing() { - return false; - } - row.is_some_and(|row| row.is_melee() || row.min != 0.0 || row.max != 0.0) + !spell.on_next_swing() && row.is_some_and(SpellRange::is_melee) } /// `SpellRange.dbc`, by row id ([`crate::spells::SpellDisplay::range_index`]). @@ -156,60 +171,73 @@ pub fn load_spell_ranges(chain: &mut Chain) -> Result { mod tests { use super::*; - #[test] - fn min_max_range_follows_the_byte_law() { - let spell = |range_index: u32, attributes: u32| crate::spells::SpellDisplay { + /// The two inputs, as a call passes them: `target` the unit argument, `attack` the caster's + /// auto-attack target that the melee arm looks up itself. + fn reaches(target: Option, attack: Option) -> RangeTargets { + RangeTargets { + target, + attack_target: attack, + } + } + + fn spell(range_index: u32, attributes: u32) -> crate::spells::SpellDisplay { + crate::spells::SpellDisplay { range_index, attributes, ..Default::default() - }; - let melee = SpellRange { - min: 0.0, - max: 5.0, - flags: 1, - }; + } + } + + const MELEE: SpellRange = SpellRange { + min: 0.0, + max: 5.0, + flags: 1, + }; + const CHARGE: SpellRange = SpellRange { + min: 8.0, + max: 25.0, + flags: 0, + }; + const FIREBALL: SpellRange = SpellRange { + min: 0.0, + max: 35.0, + flags: 0, + }; + + #[test] + fn min_max_range_follows_the_byte_law() { // Two 1.5-reach units (4.333) floor at 5.0, let d = spell(2, 0); assert_eq!( - min_max_range(&d, Some(&melee), 1.5, Some(1.5)), + min_max_range(&d, Some(&MELEE), 1.5, reaches(Some(1.5), None)), Some((0.0, MELEE_RANGE_FLOOR)) ); // a 4.0 pair (9.333) clears it. - let (_, max) = min_max_range(&d, Some(&melee), 4.0, Some(4.0)).unwrap(); + let (_, max) = min_max_range(&d, Some(&MELEE), 4.0, reaches(Some(4.0), None)).unwrap(); assert!((max - 9.3333).abs() < 1e-3); // With no target the caster's reach counts twice, as in the client: 9.333 alone too. - let (_, max) = min_max_range(&d, Some(&melee), 4.0, None).unwrap(); + let (_, max) = min_max_range(&d, Some(&MELEE), 4.0, RangeTargets::default()).unwrap(); assert!((max - 9.3333).abs() < 1e-3); // Charge's 8-25 row pads both bounds by the bare reach sum; the 1.3333 is melee-only. - let charge_row = SpellRange { - min: 8.0, - max: 25.0, - flags: 0, - }; - let (min, max) = min_max_range(&d, Some(&charge_row), 1.5, Some(1.5)).unwrap(); + let (min, max) = min_max_range(&d, Some(&CHARGE), 1.5, reaches(Some(1.5), None)).unwrap(); assert!((min - (8.0 + 3.0)).abs() < 1e-3); assert!((max - (25.0 + 3.0)).abs() < 1e-3); // Fireball's 0-35 row pads the max only: the fcomp-vs-0.0 guard keeps the min at zero. - let fireball_row = SpellRange { - min: 0.0, - max: 35.0, - flags: 0, - }; - let (min, max) = min_max_range(&d, Some(&fireball_row), 1.5, Some(1.5)).unwrap(); + let (min, max) = min_max_range(&d, Some(&FIREBALL), 1.5, reaches(Some(1.5), None)).unwrap(); assert_eq!(min, 0.0); assert!((max - 38.0).abs() < 1e-3); // No target, as the tooltip calls it (`target = NULL`, `0x52e9c2`): the raw bounds. assert_eq!( - min_max_range(&d, Some(&charge_row), 1.5, None), + min_max_range(&d, Some(&CHARGE), 1.5, RangeTargets::default()), Some((8.0, 25.0)) ); // The on-next-swing attribute short-circuits to 100 without reading the row. assert_eq!( - min_max_range(&spell(1, 0x400), None, 1.5, None), + min_max_range(&spell(1, 0x400), None, 1.5, RangeTargets::default()), Some((0.0, ON_NEXT_SWING_RANGE)) ); @@ -218,13 +246,63 @@ mod tests { max: 0.0, flags: 0, }; - assert_eq!(min_max_range(&d, Some(&self_row), 1.5, None), None); + assert_eq!( + min_max_range(&d, Some(&self_row), 1.5, RangeTargets::default()), + None + ); } - /// The predicate is exact over every arm: it is true for a spell and row exactly when some - /// change of either reach, with or without a target, moves [`min_max_range`]'s answer. + /// The ranged arm pads only for the target passed in (`0x6e35d8 test ecx,ecx`, `0x6e35ec je`), + /// never for the caster's auto-attack target, which the ranged arm does not read. #[test] - fn the_reach_predicate_matches_what_min_max_range_reads() { + fn the_ranged_arm_pads_only_for_the_explicit_target() { + let d = spell(4, 0); + // Fireball's 35 with no target passed but an auto-attack target of reach 1.5: 35, not 38. + assert_eq!( + min_max_range(&d, Some(&FIREBALL), 1.5, reaches(None, Some(1.5))), + Some((0.0, 35.0)) + ); + // The same row with the target passed, and the caster's reach 1.5: 35 + 1.5 + 1.5. + assert_eq!( + min_max_range(&d, Some(&FIREBALL), 1.5, reaches(Some(1.5), None)), + Some((0.0, 38.0)) + ); + // An auto-attack target beside it pads nothing: the target passed is the pad's second reach. + assert_eq!( + min_max_range(&d, Some(&FIREBALL), 1.5, reaches(Some(1.5), Some(4.0))), + Some((0.0, 38.0)) + ); + // A bounded row's min stays unpadded without a target too: Charge's 8-25. + assert_eq!( + min_max_range(&d, Some(&CHARGE), 1.5, reaches(None, Some(1.5))), + Some((8.0, 25.0)) + ); + } + + /// The melee arm takes the unit target's reach, else the auto-attack target's, else the + /// caster's own (`0x6e3552`-`0x6e3584`, `0x6e3594`), and adds the caster's. + #[test] + fn the_melee_arm_falls_back_from_the_target_to_the_attack_target_to_the_caster() { + let d = spell(2, 0); + let max = |target, attack| { + min_max_range(&d, Some(&MELEE), 4.0, reaches(target, attack)) + .unwrap() + .1 + }; + let sum = |reach: f32| reach + 4.0 + COMBAT_REACH_ADD; + // With no target passed the auto-attack target's 6.0 counts: 6 + 4 + 1.3333. + assert!((max(None, Some(6.0)) - sum(6.0)).abs() < 1e-3); + // A target passed wins over it. + assert!((max(Some(2.0), Some(6.0)) - sum(2.0)).abs() < 1e-3); + // With neither, the caster's reach counts twice. + assert!((max(None, None) - sum(4.0)).abs() < 1e-3); + } + + /// The predicate is exact for the tooltip's call (`0x52e9c2`, no unit target passed): it is + /// true for a spell and row exactly when some change of the caster's reach or of the + /// auto-attack target's, present or not, moves [`min_max_range`]'s answer. + #[test] + fn the_reach_predicate_matches_what_min_max_range_reads_with_no_target() { let row = |min, max, flags| Some(SpellRange { min, max, flags }); let rows = [ None, @@ -240,17 +318,18 @@ mod tests { ..Default::default() }; for row in &rows { + // (caster reach, auto-attack target's reach) before and after. let moves = [ - (1.5, None, 4.0, None), - (1.5, Some(1.5), 4.0, Some(1.5)), - (1.5, Some(1.5), 1.5, Some(4.0)), - (1.5, None, 1.5, Some(4.0)), - (1.5, Some(1.5), 1.5, None), + ((1.5, None), (4.0, None)), + ((1.5, Some(1.5)), (1.5, Some(4.0))), + ((1.5, Some(1.5)), (4.0, Some(1.5))), + ((1.5, None), (1.5, Some(4.0))), + ((1.5, Some(1.5)), (1.5, None)), ] .iter() - .any(|&(a, at, b, bt)| { - min_max_range(&spell, row.as_ref(), a, at) - != min_max_range(&spell, row.as_ref(), b, bt) + .any(|&((a, a_attack), (b, b_attack))| { + min_max_range(&spell, row.as_ref(), a, reaches(None, a_attack)) + != min_max_range(&spell, row.as_ref(), b, reaches(None, b_attack)) }); assert_eq!( min_max_range_reads_reach(&spell, row.as_ref()),