fix(#1119): make melee auto-engage reachability 3-D, not XY-only - #1124
Merged
Merged
Conversation
target_in_melee_range, reconcile_engage_nav_state, and drive_auto_engage_melee all judged whether a target was reachable using only XY distance. A target on an elevated ledge could sit well inside the XY ring while being tens of units above the player — physically out of reach of this driver's XY-only steering (wish_vspeed is always 0.0) — yet get reported as in melee range/engaging, even silently overwriting a prior honest no_path from the walker's own real pathfinding. target_in_melee_range now uses Entity::dist_to (genuine 3-D distance). A new MELEE_ENGAGE_MAX_Z_GAP constant (anchored to the real A* planner's own STEP_H walkable-rise cap) gates whether a target is worth chasing at all, via a new shared ActionLoop::melee_chase_plausible helper used by both reconcile_engage_nav_state's want_engage gate and drive_auto_engage_melee's outer gate — replacing the previous hand-duplicated (and matching-but-wrong) XY-only checks at each site. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NxfLFobwMobnF1jKjozRKp
Addresses findings from an independent review of PR #1124: - drive_auto_engage_melee could pin a character at a stuck-forever `engaging` with zero progress: a target within MELEE_ENGAGE_MAX_Z_GAP but directly overhead/underfoot (dx=dy~=0) made wish_dir fall back to [0.0, 0.0] every tick, since there is no XY direction left to close a purely vertical gap. melee_chase_plausible now declines that case outright instead of reporting it plausible. - The Z-gap cap was symmetric (+-20) despite the real A* planner's own climb/drop limits being asymmetric (STEP_H=20 climbing, MAX_STEP_DOWN=60 descending, since a drop is gravity-assisted and a climb is not) -- added MELEE_ENGAGE_MAX_Z_DROP=60 and made the gate asymmetric to match, so a legitimately gravity-reachable target 20-60u below is no longer wrongly declined. - target_in_melee_range and melee_chase_plausible could disagree in pet mode: PET_STANDOFF_RANGE (25) is wider than MELEE_ENGAGE_MAX_Z_GAP (20), so a target at dz=22 read "in range" by raw 3-D distance while the driver silently declined to chase it. Both now share GameState::melee_z_reachable_by_chase. - Corrected MELEE_ENGAGE_MAX_Z_GAP's doc comment: collision.rs's own comment on STEP_H says a smooth ramp's aggregate rise is governed by MAX_WALK_GRADE, not the per-cell STEP_H, so the constant is a heuristic approximation of what the real planner can climb, not an exact match. - docs/http-api.md's `engaging`/`melee_engaged`/`target_in_melee_range` entries now describe the Z-gap gate. - melee_chase_plausible now returns the already-computed (dist2d, dist3d) instead of callers recomputing dist3d's sqrt a second time. Verified: full workspace test suite (0 failed) and clippy -D warnings both clean after the change; new regression + boundary tests cover the stuck-forever case, the asymmetric cap boundaries, and the pet-mode Z-gap/standoff-range mismatch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CWrs7cDd1NAJmaLscBjgcj
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
target_in_melee_range/the auto-engage driver reported targets on elevated, unreachable ledges as "in melee range"/"engaging" because reachability was computed from XY distance only, ignoring Z — even silently overwriting a prior honestno_pathfrom the walker's own real pathfinding.target_in_melee_range(eqoxide-core/src/game_state.rs) now usesEntity::dist_to, a genuine 3-D distance, instead of a hand-rolled XY-only calc.MELEE_ENGAGE_MAX_Z_GAP(anchored to the real A* planner's ownSTEP_Hwalkable-rise cap ineqoxide-nav) and a new sharedActionLoop::melee_chase_plausible(dx, dy, dz, engage)helper, used by bothreconcile_engage_nav_state'swant_engagegate anddrive_auto_engage_melee's outer gate, so a target beyond that Z gap is never promoted toengagingin the first place — this both fixes the false "in range" report and, as a side effect, stopsno_pathfrom ever being clobbered for that target, without weakeningno_path's terminal-state guarantee for unrelated, reachable targets.Follow-up commit: closing review findings
An independent review of the first commit surfaced five further issues, all fixed here:
wish_dirfall back to[0.0, 0.0]every tick — there's no XY direction left to close a purely vertical gap, so the driver pinned itself atengagingforever with zero progress.melee_chase_plausiblenow declines that case outright.STEP_H=20 climbing,MAX_STEP_DOWN=60 descending — a drop is gravity-assisted, a climb is not). AddedMELEE_ENGAGE_MAX_Z_DROP=60 and made the gate asymmetric to match, so a legitimately gravity-reachable target 20–60u below is no longer wrongly declined.target_in_melee_rangeandmelee_chase_plausiblecould disagree in pet mode —PET_STANDOFF_RANGE(25) is wider thanMELEE_ENGAGE_MAX_Z_GAP(20), so a target at dz=22 read "in range" by raw 3-D distance while the driver silently declined to chase it. Both now share one helper,GameState::melee_z_reachable_by_chase.MELEE_ENGAGE_MAX_Z_GAP's doc comment —collision.rs's own comment onSTEP_Hsays a smooth ramp's aggregate rise is governed byMAX_WALK_GRADE, not the per-cellSTEP_H, so the constant is a heuristic approximation of what the real planner can climb, not an exact match.docs/http-api.md'sengaging/melee_engaged/target_in_melee_rangeentries now describe the Z-gap gate.melee_chase_plausiblenow returns the already-computed(dist2d, dist3d)instead of callers recomputingdist3d's sqrt a second time.Test plan
cargo build --workspace --all-targets— cleancargo clippy --workspace -- -D warnings— clean, 0 warnings (run locally; clippy isn't installed on the remote builder)cargo test --workspace --locked(via remote builder) — all passing, 0 failed, including new regression/boundary tests added across both commits:game_state::tests::target_in_melee_range_is_false_across_an_unreachable_z_gap_1119game_state::tests::target_in_melee_range_is_true_at_the_same_xy_gap_with_no_z_gap(control)game_state::tests::target_in_melee_range_is_false_for_a_pet_mode_target_within_standoff_but_past_the_z_gapgame_state::tests::target_in_melee_range_is_true_for_a_pet_mode_target_within_both_standoff_and_z_gap(control)game_state::tests::melee_z_reachable_by_chase_is_asymmetric_at_its_boundariesaction_loop::tests::drive_auto_engage_melee_declines_an_unreachable_ledge_target_1119action_loop::tests::reconcile_engage_does_not_clobber_no_path_for_an_unreachable_ledge_target_1119action_loop::tests::drive_auto_engage_melee_declines_a_directly_overhead_target_with_no_xy_left_to_closeaction_loop::tests::drive_auto_engage_melee_treats_max_z_gap_boundary_inclusivelyaction_loop::tests::drive_auto_engage_melee_allows_a_much_larger_downward_gap_than_upwardaction_loop::tests::drive_auto_engage_melee_declines_a_pet_mode_target_within_standoff_but_past_the_z_gapdebug_discloses_whether_the_target_is_actually_in_melee_range_1007integration test (z=0 fixtures — identical result under 2-D or 3-D distance)nav_statestayedidle, never got stuck atengaging, confirming the fix declines an unreachable target promptly instead of looping.target_in_melee_range: true), and killed it (target_hp_pct→ 0.0) on the very first poll, then disengaged cleanly. Confirms normal engage/kill behavior is unaffected.🤖 Generated with Claude Code
https://claude.ai/code/session_01CWrs7cDd1NAJmaLscBjgcj