From 5821bdc9eae4f1068dd3788f50ec8c8ef8fa0754 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 13 May 2026 17:52:45 +0200 Subject: [PATCH] =?UTF-8?q?fix(B.4b):=20WorldPicker.Pick=20=E2=80=94=20han?= =?UTF-8?q?dle=20inside-sphere=20origin=20+=20document=20normalize=20contr?= =?UTF-8?q?act?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review flagged two latent correctness bugs in Pick: 1. The single t = -b - sqrt(d) intersection skipped entities whose 5m bounding sphere contained the ray origin. Realistic at point-blank range — if the player stands within ~5m of a door, the near-plane sits inside the door's bounding sphere and the door becomes unpickable. Standard fix: when t_near < 0 fall through to t_far = -b + sqrt(d) (the sphere exit point). 2. The discriminant formula assumes |direction| = 1. BuildRay currently normalizes so the assumption holds at the wire, but the contract wasn't documented. Added an explicit note. New test Pick_RayOriginInsideEntitySphere_StillReturnsServerGuid covers the inside-sphere case. Suite: 9/9 WorldPicker tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/AcDream.Core/Selection/WorldPicker.cs | 16 ++++++++++++++-- .../Selection/WorldPickerTests.cs | 17 +++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/AcDream.Core/Selection/WorldPicker.cs b/src/AcDream.Core/Selection/WorldPicker.cs index a0da64c..1c49b70 100644 --- a/src/AcDream.Core/Selection/WorldPicker.cs +++ b/src/AcDream.Core/Selection/WorldPicker.cs @@ -60,6 +60,13 @@ public static class WorldPicker /// using a fixed 5m sphere radius. Returns the /// of the closest hit within , or null on miss. /// + /// + /// World-space ray direction. Must be normalized — the geometric + /// ray-sphere formula simplifies a = dot(direction, direction) to + /// 1; non-unit input produces an undocumented t-scale that + /// makes maxDistance compare against ray-parameter units instead + /// of world meters. + /// /// /// Entities with ServerGuid == 0 (atlas-tier scenery, dat-hydrated /// statics) are skipped — they have no server-side identity and can't be @@ -94,8 +101,13 @@ public static class WorldPicker float d = b * b - c; if (d < 0f) continue; - float t = -b - MathF.Sqrt(d); - if (t < 0f) continue; // ray points away or origin inside + // Two intersection roots: t_near = -b - sqrt(d), t_far = -b + sqrt(d). + // If t_near < 0 the ray origin is INSIDE the sphere; fall through + // to t_far so the entity is still pickable at point-blank range. + float sqrtD = MathF.Sqrt(d); + float t = -b - sqrtD; + if (t < 0f) t = -b + sqrtD; // origin inside sphere -> use far exit + if (t < 0f) continue; // both roots negative -> sphere entirely behind ray if (t >= maxDistance) continue; if (t < bestT) { diff --git a/tests/AcDream.Core.Tests/Selection/WorldPickerTests.cs b/tests/AcDream.Core.Tests/Selection/WorldPickerTests.cs index 20550db..ac0bc5a 100644 --- a/tests/AcDream.Core.Tests/Selection/WorldPickerTests.cs +++ b/tests/AcDream.Core.Tests/Selection/WorldPickerTests.cs @@ -149,4 +149,21 @@ public class WorldPickerTests Assert.Null(result); } + + [Fact] + public void Pick_RayOriginInsideEntitySphere_StillReturnsServerGuid() + { + // Player ~3m from a door -> camera near-plane sits INSIDE the door's + // 5m bounding sphere. Naive t_near < 0 guard would skip; correct + // behavior is to fall through to t_far (the sphere exit point). + var entity = MakeEntity(0xABCDu, new Vector3(0, 0, -3)); + + var result = WorldPicker.Pick( + origin: Vector3.Zero, + direction: -Vector3.UnitZ, + candidates: new[] { entity }, + skipServerGuid: 0u); + + Assert.Equal(0xABCDu, result); + } }