General physics fixes#562
Merged
Merged
Conversation
The hit normal was transformed back to world coordinates using the matrix directly instead of its transpose. Since the forward transform into local space uses the transpose, the inverse rotation must use the non-transposed matrix — use math.transpose() to correct this. Add a test verifying the returned hit normal points into world space.
Add PhysicsRegressionTests covering known transform and collision bugs: point/lineZ/circle collider transforms, matrix scale extraction, ball radius scaling, collision event round-trips, Line3D hit event firing, and boxed Equals delegation. Includes an IL-walking helper to assert Equals(object) forwards to the typed overload.
PointCollider transformed its position and then rebuilt its AABB by applying the same matrix to that already-transformed point. Translations and other transforms therefore affected broad-phase bounds twice, leaving the collider geometry and bounds in different places. Build the zero-size bounds directly from the final transformed point. Also expose runtime internals to the Unity test assembly so the collider-level regressions compile and can verify the broad-phase position alongside the narrow-phase point.
LineZCollider reconstructed transforms from a translation plus a separately extracted Z scale. That skipped legal rotation around Z, ignored XY scale, and expanded the height around a translated midpoint instead of transforming the original endpoints. Transform the low and high endpoints with the complete matrix, take their shared XY position, and order the resulting Z values. The collider and its bounds now follow translation, Z rotation, axis scale, and negative Z scale exactly.
GetScale measured matrix rows even though Unity.Mathematics stores each transformed basis axis in a column. Once a non-uniform scale was rotated, row lengths blended the axes and could report a false uniform scale. Measure the XYZ length of each basis column instead. Scale extraction is now rotation-invariant, and collider transformability checks correctly reject rotated non-uniform XY scale instead of baking an ellipse as a circle.
The non-transformable-collider fallback moved a ball into local space but left its radius in world units. Uniformly scaled items therefore tested a correctly transformed center against a sphere of the wrong size, shifting collision times and contact depth. Apply the matrix scale to the radius when all three axes are uniform, where a sphere remains exactly representable by one scalar and the inverse transform restores the original value. Non-uniform transforms remain unchanged because representing those faithfully requires ellipsoid collision support.
CollisionEventData exposes the legacy XY hit velocity used by flippers and plungers, but a tilted-space transform can rotate part of that vector into Z. The old transform discarded that component immediately, so applying the inverse matrix returned a smaller XY velocity. Keep the transformed Z component privately alongside the public XY value and include it in subsequent transforms. Existing collider code retains its two-dimensional API while world-to-local round trips preserve the complete velocity needed by tilted plungers.
The line-3D event gate compared normal dot velocity directly with a positive threshold. An approaching ball produces a negative dot product, so ordinary impacts resolved physically but never crossed the event threshold; receding motion had the sign the event code expected. Convert the pre-collision dot product to a positive impact speed before comparing it with the threshold. This matches the convention used by the other static colliders and restores hit events for primitive, ramp, rubber, and wire-guide line segments.
Aabb.Equals(object) and ColliderHeader.Equals(object) checked the boxed type but then passed the original object back to Equals. Overload resolution selected the same object overload, causing unbounded recursion and a process-ending stack overflow. Capture the typed value in each pattern match and call the corresponding IEquatable overload explicitly. The boxed paths now share the established field comparisons, and the regression suite description no longer labels the completed cases as intentionally red.
Greptile SummaryThis PR fixes a cluster of physics correctness bugs discovered while investigating anomalous plunger-ramp behavior: a ball gaining more momentum could counterintuitively rebound earlier than a slower one. The root cause was an incorrectly applied normal transform in
Confidence Score: 5/5All changes are targeted bug fixes with dedicated regression tests; no new APIs or data-model changes that could break callers. Every fix is mathematically sound (transpose = inverse for orthogonal rotation, column magnitudes for TRS scale, relative tolerance for large-scale uniformity checks), the Aabb/ColliderHeader Equals recursion crash is definitively resolved, and the new test suite guards each fix independently. No behavioral regressions were identified. No files require special attention; the most impactful changes in Line3DCollider.cs and MathExtensions.cs are well-covered by the accompanying tests. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant BallMover
participant Line3DCollider
participant LineZCollider
participant CollisionEvent
BallMover->>Line3DCollider: HitTest(ball, dTime)
Line3DCollider->>Line3DCollider: "hitTestBall.Position = _matrix * ball.Position"
Line3DCollider->>Line3DCollider: "hitTestBall.Velocity = _matrix * ball.Velocity"
Line3DCollider->>LineZCollider: HitTest(collEvent, hitTestBall)
LineZCollider-->>Line3DCollider: hitTime, collEvent.HitNormal (local space)
Note over Line3DCollider: FIX: n_world = transpose(_matrix) * n_local
Line3DCollider-->>BallMover: hitTime, HitNormal (world space)
BallMover->>Line3DCollider: Collide(ball, collEvent)
Note over Line3DCollider: FIX: impactSpeed = -dot(HitNormal, Velocity)
Line3DCollider->>Line3DCollider: BallCollider.Collide3DWall(...)
alt "impactSpeed >= Threshold"
Line3DCollider->>CollisionEvent: FireHitEvent(ball, hitEvents, Header)
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant BallMover
participant Line3DCollider
participant LineZCollider
participant CollisionEvent
BallMover->>Line3DCollider: HitTest(ball, dTime)
Line3DCollider->>Line3DCollider: "hitTestBall.Position = _matrix * ball.Position"
Line3DCollider->>Line3DCollider: "hitTestBall.Velocity = _matrix * ball.Velocity"
Line3DCollider->>LineZCollider: HitTest(collEvent, hitTestBall)
LineZCollider-->>Line3DCollider: hitTime, collEvent.HitNormal (local space)
Note over Line3DCollider: FIX: n_world = transpose(_matrix) * n_local
Line3DCollider-->>BallMover: hitTime, HitNormal (world space)
BallMover->>Line3DCollider: Collide(ball, collEvent)
Note over Line3DCollider: FIX: impactSpeed = -dot(HitNormal, Velocity)
Line3DCollider->>Line3DCollider: BallCollider.Collide3DWall(...)
alt "impactSpeed >= Threshold"
Line3DCollider->>CollisionEvent: FireHitEvent(ball, hitEvents, Header)
end
Reviews (3): Last reviewed commit: "physics(balls): compare transform scale ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Extract ClearHitVelocity() so clearing the collision event's hit velocity also zeroes the transformed Z component, not just the 2D vector. ClearCollider() now clears it too, and KickerApi uses the shared helper. Add a regression test covering transform after clear.
Ball transforms decide whether a sphere's radius can follow a collider matrix by comparing the three extracted axis scales. The previous absolute tolerance grew stricter as the matrix scale increased, allowing a matrix and its inverse to disagree and permanently shrink the persistent ball. Compare scale values relative to their magnitude and share the predicate with circle collider validation. Cover a rotated uniform scale of ten with an inverse/forward regression that previously changed radius 25 to 2.5.
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
1 participant
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.
Fixes several transform and collision bugs uncovered while investigating the MG plunger ramp, where increasing plunger momentum could make the ball rebound earlier instead of traveling farther up the ramp.
The original issue was caused by an incorrectly transformed Line3D hit normal. A broader physics audit found related transform, event, and equality problems, which are also addressed here with regression coverage.
Changes
AabbandColliderHeaderequality to their typed overloadsinstead of recursing through
Equals(object).Ball radius scaling is deliberately limited to uniform transforms. A non-uniformly transformed sphere becomes an ellipsoid and cannot be represented faithfully by the engine's scalar radius.