Support sweep and overlap #2657
Conversation
WalkthroughThe changes introduce a unified geometry abstraction for physics shapes, adding new geometry interfaces and classes, and extend the physics API with shape casting and overlap query methods. Both the design and implementation layers are updated to support these features, including new geometry creation methods, sweep and overlap queries, and improved resource management. Extensive tests are added to validate the new functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PhysicsScene
participant Engine
participant NativePhysics
participant Geometry
User->>PhysicsScene: boxCast(center, orientation, halfExtents, direction, ...)
PhysicsScene->>Engine: get native physics instance
PhysicsScene->>NativePhysics: createBoxGeometry(halfExtents)
NativePhysics-->>PhysicsScene: IBoxGeometry
PhysicsScene->>PhysicsScene: setup pose and filter
PhysicsScene->>NativePhysics: sweep(geometry, pose, direction, ...)
NativePhysics-->>PhysicsScene: hit result (if any)
PhysicsScene-->>User: boolean (hit) and/or HitResult
User->>PhysicsScene: overlapSphere(center, radius, ...)
PhysicsScene->>Engine: get native physics instance
PhysicsScene->>NativePhysics: createSphereGeometry(radius)
NativePhysics-->>PhysicsScene: ISphereGeometry
PhysicsScene->>PhysicsScene: setup pose and filter
PhysicsScene->>NativePhysics: overlapAny(geometry, pose, ...)
NativePhysics-->>PhysicsScene: overlap result (if any)
PhysicsScene-->>User: boolean (overlap) and/or OverlapHitResult
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📥 CommitsReviewing files that changed from the ba 8000 se of the PR and between 2bc49d4 and f7a0de3. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 12
🔭 Outside diff range comments (4)
packages/core/src/physics/PhysicsMaterial.ts (1)
20-26: 🛠️ Refactor suggestionAdd initialization guard and default destroy flag.
- The
_destroyedfield is declared but not initialized; consider defaulting it tofalsefor clarity.- Prevent calls to an uninitialized physics backend by guarding in the constructor:
-export class PhysicsMaterial { - private _destroyed: boolean; +export class PhysicsMaterial { + private _destroyed = false; /** @internal */ _nativeMaterial: IPhysicsMaterial; constructor() { + if (!Engine._nativePhysics) { + throw new Error("Native physics engine not initialized. Call Engine._initialize first."); + } this._nativeMaterial = Engine._nativePhysics.createPhysicsMaterial( this._staticFriction, this._dynamicFriction, this._bounciness, this._bounceCombine, this._frictionCombine );tests/src/core/physics/PhysicsScene.test.ts (2)
142-153: 🛠️ Refactor suggestion
++undefined→NaN– initialise counters before increment
enterEvent[box1.id]is initialised as an empty array, so
++enterEvent[box1.id][other.id]operates onundefined, yieldingNaN
and breaking the expected numeric semantics.-++enterEvent[box1.id][other.id]; +enterEvent[box1.id][other.id] = (enterEvent[box1.id][other.id] ?? 0) + 1;Apply the same pattern to every similar counter mutation in this file.
75-79: 💡 Verification agent🧩 Analysis chain
Avoid invoking private APIs from tests
physics._update(8)reaches into a private/underscore-prefixed member that is not guaranteed to stay stable.
Please switch to the public engine tick (engine.update,engine.run, orphysics.update) or expose a dedicated test helper instead.-physics._update(8); +// Prefer a public update helper. E.g.: +physics.update(8);
🏁 Script executed:
#!/bin/bash set -e echo "=== PhysicsScene class definitions ===" rg "class PhysicsScene" -n echo "=== Private _update method definitions ===" rg "_update\(" -n src echo "=== Public update method in PhysicsScene (AST search) ===" ast-grep --pattern $'class PhysicsScene { $$$ update($_) { $$$ } $$$ }' echo "=== physics.update usage ===" rg "physics\.update" -n . echo "=== engine.update usage ===" rg "engine\.update" -n src echo "=== engine.run usage ===" rg "engine\.run" -n srcLength of output: 360
🏁 Script executed:
#!/bin/bash echo "=== private _update occurrences ===" rg -n "_update(" . || true echo "=== public update method in PhysicsScene.ts ===" rg -n "^\s*update(" packages/core/src/physics/PhysicsScene.ts || true echo "=== physics.update usage in tests ===" rg -n "physics\.update" tests || true echo "=== engine.update usage ===" rg -n "engine\.update" . || true echo "=== engine.run usage ===" rg -n "engine\.run" . || true echo "=== Excerpt of PhysicsScene (first 200 lines) ===" sed -n '1,200p' packages/core/src/physics/PhysicsScene.ts || trueLength of output: 22203
🏁 Script executed:
#!/bin/bash set -e echo "=== Searching for public update method in PhysicsScene ===" rg -n '^\s*update\(' packages/core/src/physics/PhysicsScene.ts || true echo "=== Context around potential update method ===" sed -n '200,400p' packages/core/src/physics/PhysicsScene.ts || trueLength of outp 8000 ut: 7396
🏁 Script executed:
#!/bin/bash set -e echo "=== Locate private _update in PhysicsScene.ts ===" rg -n 'private _update' packages/core/src/physics/PhysicsScene.ts || true echo "=== Show context around private _update (lines 350-450) ===" sed -n '350,450p' packages/core/src/physics/PhysicsScene.ts || true echo "=== Find engine.update signature in Engine.ts ===" rg -n '^\s*update\(' packages/core/src/Engine.ts || true echo "=== Show context around engine.update (lines 300-360) ===" sed -n '300,360p' packages/core/src/Engine.ts || true echo "=== Find engine.run signature in Engine.ts ===" rg -n '^\s*run\(' packages/core/src/Engine.ts || trueLength of output: 6866
Use Public Engine Tick Instead of PhysicsScene._update
Tests should not reach into private APIs (_update). Replace direct calls tophysics._update(…)with the engine’s public update loop, which invokes physics under the hood:-function updatePhysics(physics) { - for (let i = 0; i < 5; ++i) { - physics._update(8); - } -} +function updatePhysics(engine) { + // Public engine.update drives physics, rendering, input, etc. + for (let i = 0; i < 5; ++i) { + engine.update(); + } +}If you need a per‐scene physics step without touching other subsystems, please expose a dedicated public API on
PhysicsScene—for example,scene.physics.update(deltaTime: number)—or provide a test‐only helper rather than using_update.packages/physics-physx/src/PhysXPhysicsScene.ts (1)
65-73:⚠️ Potential issueGuard against undefined event callbacks
_onContactEnter/_Exit/_Stayare optional (?) but are invoked unconditionally.
Ifundefinedis passed from the caller this will raise a runtime exception the first time a contact occurs.- onContactBegin: (collision) => { - this._onContactEnter(collision); - }, + onContactBegin: (collision) => { + this._onContactEnter?.(collision); + },(Apply the same
?.guard foronContactEndandonContactPersist.)
♻️ Duplicate comments (3)
packages/physics-physx/src/PhysXPhysics.ts (2)
228-230: Ditto – narrow return type for sphere geometry(See comment above.)
242-244: Ditto – narrow return type for plane geometry(See comment above.)
packages/design/src/physics/IPhysics.ts (1)
107-113: Return type documented but missing in implementationEnsure
IPhysics.createSphereGeometry()is implemented withISphereGeometryreturn type (see PhysXPhysics comment).
🧹 Nitpick comments (21)
packages/core/src/Polyfill.ts (2)
38-71: Extend the check to cover browsers that exposeAudioContextbut lack the Promise-based APIRight now we patch only when
window.AudioContextis absent.
Safari 13/14 expose an un-prefixedAudioContext, but itsdecodeAudioDatais still callback-only.
Those users would miss the Promise shim.A small guard keeps the current behaviour while covering that gap:
- if (!window.AudioContext && (window as any).webkitAudioContext) { + if ( + // No standard impl – fall back to prefixed + (!window.AudioContext && (window as any).webkitAudioContext) || + // Standard impl exists but still callback-style + (window.AudioContext && + !("decodeAudioData" in AudioContext.prototype) /* unlikely */ || + AudioContext.prototype.decodeAudioData.length !== 1) // expects callbacks + ) {This gives older Safari users the same ergonomic Promise interface without changing behaviour on modern browsers.
44-57: Type-safety & forward-compatibility
originalDecodeAudioDatais captured asany-heavy, losing type information and future safety.
Since the Web Audio spec already describes the Promise overload, you can keep types intact:- const originalDecodeAudioData = AudioContext.prototype - .decodeAudioData as ( - audioData: ArrayBuffer, - successCallback?: DecodeSuccessCallback | null, - errorCallback?: DecodeErrorCallback | null - ) => void; + const originalDecodeAudioData = AudioContext.prototype + .decodeAudioData.bind(AudioContext.prototype) as AudioContext["prototype"]["decodeAudioData"];This keeps the compiler aware of parameter lists and return type.
tests/src/core/Polyfill.test.ts (3)
40-41: Prefer assigningundefinedoverdeletein hot paths
delete window.AudioContext;incurs a de-opt in V8 and is flagged by Biome.
In test code the perf hit is negligible, but replacing withwindow.AudioContext = undefined as any;avoids the lint error while keeping semantics.🧰 Tools
🪛 Biome (1.9.4)
[error] 40-40: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🪛 ESLint
[error] 41-41: Delete
······(prettier/prettier)
47-50: Narrow the callback type
successCallback: Functionobscures intent and forfeits type checking.
Use the precise signature:- decodeAudioData(arrayBuffer: ArrayBuffer, successCallback: Function) { + decodeAudioData( + arrayBuffer: ArrayBuffer, + successCallback: (buffer: AudioBuffer) => void + ) {This prevents accidental misuse inside the mock.
🧰 Tools
🪛 Biome (1.9.4)
[error] 47-47: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
62-66:self = thisis unnecessary inside an arrow functionArrow functions capture
thislexically:- const self = this; - return new Promise(resolve => { - originalDecodeAudioData.apply(self, [arrayBuffer, resolve]); + return new Promise(resolve => { + originalDecodeAudioData.call(this, arrayBuffer, resolve);Simplifies the code and satisfies the
noUselessThisAliasrule.🧰 Tools
🪛 Biome (1.9.4)
[error] 63-63: This aliasing of this is unnecessary.
Arrow functions inherits
thisfrom their enclosing scope.
Safe fix: Use this instead of an alias.(lint/complexity/noUselessThisAlias)
🪛 ESLint
[error] 62-62: Insert
·(prettier/prettier)
[error] 64-64: Replace
resolvewith(resolve)(prettier/prettier)
packages/core/src/physics/shape/BoxColliderShape.ts (1)
29-33: Guard against uninitialized native physics.
Constructing a collider shape before the engine’s physics backend is initialized can lead to a null‐reference error. Consider adding a runtime check in the constructor:constructor() { super(); + if (!Engine._nativePhysics) { + throw new Error("Native physics engine not initialized. Call Engine._initialize first."); + } this._nativeShape = Engine._nativePhysics.createBoxColliderShape( this._id, this._size, this._material._nativeMaterial );packages/core/src/physics/index.ts (1)
4-4: Export new overlap query result type.
AddingOverlapHitResultalongsideHitResultextends the public API to cover overlap queries. Ensure the package version and documentation/changelog reflect this new export.packages/core/src/physics/shape/CapsuleColliderShape.ts (1)
58-63: Guard against uninitialized native physics.
As with other shapes, protect against constructing a capsule before the physics system is ready:constructor() { super(); + if (!Engine._nativePhysics) { + throw new Error("Native physics engine not initialized. Call Engine._initialize first."); + } this._nativeShape = Engine._nativePhysics.createCapsuleColliderShape( this._id, this._radius, this._height, this._material._nativeMaterial );packages/physics-physx/src/shape/PhysXGeometry.ts (1)
8-9: Replaceanywith generics for stronger typingUsing
anyfor_physXand_geometryhides most type errors.
A light generic parameter (e.g.TGeom) keeps flexibility while still giving IntelliSense and compile-time checks.packages/physics-physx/src/PhysXPhysics.ts (3)
54-57: Fields are declared but still typedanyNow that you store the instances for later cleanup, giving them an explicit PhysX type (e.g.
physX.PxDefaultErrorCallback) would prevent accidental mis-assignment.
100-103: Hard-coded CDN URLs risk 404 or downgrade confusionThe new JS URL contains
downgrade.js, and the WASM URL a fixed hash.
Expose these as configurable constants (or accept a base URL) so downstream apps can pin versions or host their own bundles.
Also add a basic fetch/timeout error message to aid troubleshooting.
286-288: Shadowingversionimport can confuse readersThe local constant
versionmasks the earlier importversionfrom@galacean/engine.
Rename the local variable tophysxVersion(or similar) to avoid ambiguity.packages/design/src/physics/IPhysics.ts (1)
166-170:destroy()JSDoc refers to scene, not engineThe method destroys the physics engine instance, not a physics scene. Update the description to avoid confusion.
tests/src/core/physics/PhysicsScene.test.ts (1)
168-193: Duplicate private-update calls & magic indicesThe test accesses
engineLite.sceneManager.scenes[0]twice and again calls the
private_update. Consider:
- Using
activeScenewhich already gives you the first/active scene.- Consolidating the physics step through the
updatePhysicshelper you
declared above.This reduces duplication and hides internal details behind one helper.
packages/design/src/physics/IPhysicsScene.ts (4)
50-59: Doc-parameter mismatch inraycastsignatureThe JSDoc still describes
distance& a “raycast result callback” but no longer
mentions the layer mask that used to exist, while the method signature now
exposes anonRaycastfilter. Please update the comment so that parameter
order & names match the actual signature.
61-80: Inconsistent terminology forsweepposeThe doc block talks about
center&orientation, yet the API receives a
singleposeobject containingtranslation&rotation.To avoid confusion for implementers:
- * @param center - The center of the geometry - * @param orientation - The orientation of the geometry + * @param pose - World pose of the geometry (translation & rotation)Also adjust the param list in the doc to reflect
posereplacing two separate
arguments.
81-94:overlapAnyout-parameter undocumented
outHitResult?: (shapeUniqueID: number) => voidis not mentioned in the JSDoc.
Either remove the parameter or document its behaviour (e.g. “invoked once with
the first overlapping shape ID if an overlap is found”).Lacking documentation makes the contract ambiguous for back-end authors.
95-99: Missing return type annotation ondestroyMost engine interfaces explicitly denote whether
destroyreturnsvoidor a
Promise. Stating it in the JSDoc keeps the contract clear and prevents
future accidental API drift.packages/physics-lite/src/LitePhysics.ts (1)
166-182: Consistent, actionable error messages for unsupported jointsSame concern as above applies to the joint-creation stubs.
While touching these lines, consider inserting the concrete alternative:-throw "Physics-lite don't support HingeJoint. Use Physics-PhysX instead!"; +throw new Error("HingeJoint is unavailable in Physics-lite; switch to the PhysX backend.");Clear, grammatically correct messages help developers quickly understand the
limitation and the migration path.packages/physics-lite/src/LitePhysicsScene.ts (2)
155-168: ThrowError, not raw stringsThrowing raw strings loses stack information and tooling support. Replace with proper
Errorobjects and keep a consistent message style.-throw "Physics-lite don't support sweep. Use Physics-PhysX instead!"; +throw new Error("Physics-lite does not support sweep. Please use Physics-PhysX.");Do the same for
overlapAny,addCharacterController,removeCharacterController,setGravity, etc.
186-188:destroy()is empty – consider at least clearing internal arraysEven if Lite physics has no native resources, cleaning up internal collections (
_staticColliders,_dynamicColliders,_eventMap) prevents memory leaks when switching scenes at runtime.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
packages/core/src/Engine.ts(2 hunks)packages/core/src/Polyfill.ts(2 hunks)packages/core/src/Scene.ts(1 hunks)packages/core/src/Utils.ts(1 hunks)packages/core/src/physics/CharacterController.ts(2 hunks)packages/core/src/physics/DynamicCollider.ts(2 hunks)packages/core/src/physics/HitResult.ts(1 hunks)packages/core/src/physics/PhysicsMaterial.ts(2 hunks)packages/core/src/physics/PhysicsScene.ts(7 hunks)packages/core/src/physics/StaticCollider.ts(2 hunks)packages/core/src/physics/index.ts(1 hunks)packages/core/src/physics/joint/FixedJoint.ts(2 hunks)packages/core/src/physics/joint/HingeJoint.ts(2 hunks)packages/core/src/physics/joint/SpringJoint.ts(2 hunks)packages/core/src/physics/shape/BoxColliderShape.ts(2 hunks)packages/core/src/physics/shape/CapsuleColliderShape.ts(2 hunks)packages/core/src/physics/shape/PlaneColliderShape.ts(2 hunks)packages/core/src/physics/shape/SphereColliderShape.ts(2 hunks)packages/design/src/physics/IPhysics.ts(5 hunks)packages/design/src/physics/IPhysicsScene.ts(3 hunks)packages/design/src/physics/geometry/IBoxGeometry.ts(1 hunks)packages/design/src/physics/geometry/ICapsuleGeometry.ts(1 hunks)packages/design/src/physics/geometry/IGeometry.ts(1 hunks)packages/design/src/physics/geometry/IPlaneGeometry.ts(1 hunks)packages/design/src/physics/geometry/ISphereGeometry.ts(1 hunks)packages/design/src/physics/geometry/index.ts(1 hunks)packages/design/src/physics/index.ts(1 hunks)packages/physics-lite/src/LitePhysics.ts(3 hunks)packages/physics-l 8000 ite/src/LitePhysicsScene.ts(6 hunks)packages/physics-physx/src/PhysXCharacterController.ts(1 hunks)packages/physics-physx/src/PhysXPhysics.ts(7 hunks)packages/physics-physx/src/PhysXPhysicsScene.ts(12 hunks)packages/physics-physx/src/joint/PhysXJoint.ts(0 hunks)packages/physics-physx/src/shape/PhysXBoxColliderShape.ts(4 hunks)packages/physics-physx/src/shape/PhysXBoxGeometry.ts(1 hunks)packages/physics-physx/src/shape/PhysXCapsuleColliderShape.ts(7 hunks)packages/physics-physx/src/shape/PhysXCapsuleGeometry.ts(1 hunks)packages/physics-physx/src/shape/PhysXColliderShape.ts(6 hunks)packages/physics-physx/src/shape/PhysXGeometry.ts(1 hunks)packages/physics-physx/src/shape/PhysXPlaneColliderShape.ts(2 hunks)packages/physics-physx/src/shape/PhysXPlaneGeometry.ts(1 hunks)packages/physics-physx/src/shape/PhysXSphereColliderShape.ts(3 hunks)packages/physics-physx/src/shape/PhysXSphereGeometry.ts(1 hunks)tests/src/core/Polyfill.test.ts(1 hunks)tests/src/core/physics/PhysicsScene.test.ts(33 hunks)
💤 Files with no reviewable changes (1)
- packages/physics-physx/src/joint/PhysXJoint.ts
🧰 Additional context used
🧬 Code Graph Analysis (24)
packages/core/src/physics/StaticCollider.ts (1)
packages/core/src/Engine.ts (1)
Engine(52-696)
packages/core/src/physics/DynamicCollider.ts (1)
packages/core/src/Engine.ts (1)
Engine(52-696)
packages/core/src/physics/shape/CapsuleColliderShape.ts (1)
packages/core/src/Engine.ts (1)
Engine(52-696)
packages/design/src/physics/geometry/IPlaneGeometry.ts (2)
packages/design/src/physics/geometry/index.ts (2)
IPlaneGeometry(5-5)IGeometry(1-1)packages/design/src/physics/geometry/IGeometry.ts (1)
IGeometry(6-19)
packages/design/src/physics/geometry/ISphereGeometry.ts (2)
packages/design/src/physics/geometry/index.ts (2)
ISphereGeometry(3-3)IGeometry(1-1)packages/design/src/physics/geometry/IGeometry.ts (1)
IGeometry(6-19)
packages/core/src/physics/PhysicsMaterial.ts (1)
packages/core/src/Engine.ts (1)
Engine(52-696)
packages/core/src/physics/joint/HingeJoint.ts (1)
packages/core/src/Engine.ts (1)
Engine(52-696)
packages/core/src/physics/shape/SphereColliderShape.ts (1)
packages/core/src/Engine.ts (1)
Engine(52-696)
packages/design/src/physics/geometry/ICapsuleGeometry.ts (2)
packages/design/src/physics/geometry/index.ts (2)
ICapsuleGeometry(4-4)IGeometry(1-1)packages/design/src/physics/geometry/IGeometry.ts (1)
IGeometry(6-19)
packages/physics-physx/src/shape/PhysXPlaneColliderShape.ts (1)
packages/physics-physx/src/shape/PhysXPlaneGeometry.ts (1)
PhysXPlaneGeometry(7-12)
packages/physics-physx/src/shape/PhysXSphereGeometry.ts (2)
packages/physics-physx/src/shape/PhysXGeometry.ts (1)
PhysXGeometry(7-38)packages/design/src/physics/geometry/ISphereGeometry.ts (1)
ISphereGeometry(6-11)
packages/design/src/physics/geometry/IBoxGeometry.ts (2)
packages/design/src/physics/geometry/index.ts (2)
IBoxGeometry(2-2)IGeometry(1-1)packages/design/src/physics/geometry/IGeometry.ts (1)
IGeometry(6-19)
packages/design/src/physics/IPhysicsScene.ts (1)
packages/design/src/physics/geometry/IGeometry.ts (1)
IGeometry(6-19)
packages/design/src/physics/IPhysics.ts (5)
packages/design/src/physics/geometry/IBoxGeometry.ts (1)
IBoxGeometry(7-12)packages/design/src/physics/geometry/index.ts (4)
IBoxGeometry(2-2)ISphereGeometry(3-3)IPlaneGeometry(5-5)ICapsuleGeometry(4-4)packages/design/src/physics/geometry/ISphereGeometry.ts (1)
ISphereGeometry(6-11)packages/design/src/physics/geometry/IPlaneGeometry.ts (1)
IPlaneGeometry(6-6)packages/design/src/physics/geometry/ICapsuleGeometry.ts (1)
ICapsuleGeometry(6-16)
packages/physics-physx/src/shape/PhysXGeometry.ts (1)
packages/design/src/physics/geometry/IGeometry.ts (1)
IGeometry(6-19)
packages/physics-physx/src/shape/PhysXPlaneGeometry.ts (2)
packages/physics-physx/src/shape/PhysXGeometry.ts (1)
PhysXGeometry(7-38)packages/design/src/physics/geometry/IPlaneGeometry.ts (1)
IPlaneGeometry(6-6)
packages/physics-physx/src/shape/PhysXCapsuleGeometry.ts (3)
packages/physics-physx/src/shape/PhysXGeometry.ts (1)
PhysXGeometry(7-38)packages/design/src/physics/geometry/ICapsuleGeometry.ts (1)
ICapsuleGeometry(6-16)packages/physics-physx/src/shape/PhysXSphereGeometry.ts (2)
radius(16-18)radius(20-22)
packages/physics-physx/src/shape/PhysXBoxColliderShape.ts (1)
packages/physics-physx/src/shape/PhysXBoxGeometry.ts (1)
PhysXBoxGeometry(10-26)
packages/physics-physx/src/shape/PhysXColliderShape.ts (1)
packages/physics-physx/src/shape/PhysXGeometry.ts (1)
PhysXGeometry(7-38)
packages/core/src/Engine.ts (1)
packages/design/src/physics/IPhysics.ts (1)
IPhysics(17-170)
packages/physics-physx/src/PhysXPhysics.ts (7)
packages/physics-physx/src/index.ts (1)
PhysXRuntimeMode(2-2)packages/physics-physx/src/shape/PhysXBoxGeometry.ts (3)
halfExtents(19-21)halfExtents(23-25)PhysXBoxGeometry(10-26)packages/physics-physx/src/shape/PhysXCapsuleGeometry.ts (5)
radius(16-18)radius(20-22)halfHeight(27-29)halfHeight(31-33)PhysXCapsuleGeometry(7-34)packages/physics-physx/src/shape/PhysXSphereGeometry.ts (3)
radius(16-18)radius(20-22)PhysXSphereGeometry(7-23)packages/physics-physx/src/shape/PhysXSphereColliderShape.ts (1)
PhysXSphereColliderShape(11-46)packages/physics-physx/src/shape/PhysXPlaneColliderShape.ts (1)
PhysXPlaneColliderShape(11-22)packages/physics-physx/src/shape/PhysXPlaneGeometry.ts (1)
PhysXPlaneGeometry(7-12)
packages/physics-lite/src/LitePhysics.ts (7)
packages/design/src/physics/geometry/IBoxGeometry.ts (1)
IBoxGeometry(7-12)packages/design/src/physics/geometry/index.ts (4)
IBoxGeometry(2-2)ISphereGeometry(3-3)IPlaneGeometry(5-5)ICapsuleGeometry(4-4)packages/design/src/physics/geometry/ISphereGeometry.ts (1)
ISphereGeometry(6-11)packages/design/src/physics/geometry/IPlaneGeometry.ts (1)
IPlaneGeometry(6-6)packages/design/src/physics/geometry/ICapsuleGeometry.ts (1)
ICapsuleGeometry(6-16)packages/design/src/physics/joints/IHingeJoint.ts (1)
IHingeJoint(7-64)packages/design/src/physics/joints/ISpringJoint.ts (1)
ISpringJoint(7-37)
packages/core/src/physics/PhysicsScene.ts (3)
packages/core/src/Engine.ts (1)
Engine(52-696)packages/core/src/physics/HitResult.ts (2)
HitResult(8-19)OverlapHitResult(24-29)packages/design/src/physics/geometry/IGeometry.ts (1)
IGeometry(6-19)
packages/physics-lite/src/LitePhysicsScene.ts (2)
packages/design/src/physics/geometry/IGeometry.ts (1)
IGeometry(6-19)packages/design/src/physics/index.ts (1)
ICharacterController(1-1)
🪛 Biome (1.9.4)
tests/src/core/Polyfill.test.ts
[error] 40-40: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 45-45: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
[error] 47-47: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
[error] 63-63: This aliasing of this is unnecessary.
Arrow functions inherits this from their enclosing scope.
Safe fix: Use this instead of an alias.
(lint/complexity/noUselessThisAlias)
🪛 ESLint
tests/src/core/Polyfill.test.ts
[error] 41-41: Delete ······
(prettier/prettier)
[error] 44-44: Delete ······
(prettier/prettier)
[error] 45-45: Delete ·
(prettier/prettier)
[error] 46-46: Delete ······
(prettier/prettier)
[error] 53-53: Delete ····
(prettier/prettier)
[error] 55-55: Delete ····
(prettier/prettier)
[error] 57-57: Delete ······
(prettier/prettier)
[error] 59-59: Delete ····
(prettier/prettier)
[error] 61-61: Delete ····
(prettier/prettier)
[error] 62-62: Insert ·
(prettier/prettier)
[error] 64-64: Replace resolve with (resolve)
(prettier/prettier)
[error] 68-68: Delete ····
(prettier/prettier)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (22.x, windows-latest)
🔇 Additional comments (66)
packages/core/src/Polyfill.ts (1)
7-10: Nice incremental polyfill coverageIncluding
_registerAudioContext()in the publicregisterPolyfill()entry point is a clean way to keep all polyfills centrally managed and opt-in.packages/core/src/Utils.ts (1)
82-82: URL scheme fix: Changed "files://" to "file://"This change fixes the URL scheme prefix for local files from what appears to have been "files://" to the correct standard "file://". The RFC standard specifies "file://" as the proper URI scheme for local files, which will ensure proper URL resolution when working with file paths.
packages/core/src/physics/StaticCollider.ts (1)
1-1: Architecture improvement: Centralized physics engine referenceThe change from
PhysicsScene._nativePhysicstoEngine._nativePhysicsaligns with the broader architecture refactoring that centralizes the native physics engine reference in the Engine class. This improves the architecture by providing a single source of truth for the physics engine instance.Also applies to: 16-16
packages/core/src/physics/joint/SpringJoint.ts (1)
2-2: Architecture improvement: Centralized physics engine referenceSimilar to the changes in other physics-related files, this modification centralizes the native physics reference by using
Engine._nativePhysicsinstead of the previous scattered approach. This enhances maintainability and makes physics engine lifecycle management more straightforward.Also applies to: 89-89
packages/physics-physx/src/PhysXCharacterController.ts (1)
139-139: Memory management improvement: Added explicit descriptor cleanupAdding
desc.delete()explicitly frees the native PhysX controller descriptor after the controller is created and configured. This prevents potential memory leaks when working with native code bindings, which is especially important for long-running applications.packages/core/src/physics/shape/PlaneColliderShape.ts (1)
2-2: Architectural refactor: Centralized physics access through EngineThe change from
PhysicsScene._nativePhysicstoEngine._nativePhysicsis part of a broader architectural refactor to centralize access to the native physics engine. This improves consistency across the codebase and supports the new sweep and overlap functionality being added.Also applies to: 11-11
packages/core/src/physics/DynamicCollider.ts (1)
3-3: Refactored to use centralized physics access patternThis change aligns with the architectural decision to centralize native physics access through the
Engineclass rather than throughPhysicsScene. This is consistent with other physics component changes and enables the new sweep and overlap functionality.Also applies to: 309-309
packages/core/src/physics/shape/SphereColliderShape.ts (1)
3-3: Consistent physics engine access refactorThe change from using
PhysicsScene._nativePhysicstoEngine._nativePhysicsmaintains consistency with the architectural changes made throughout the physics system. This centralization likely simplifies maintenance and resource management.Also applies to: 27-27
packages/core/src/physics/joint/FixedJoint.ts (1)
1-1: Standardized physics engine access patternThis change follows the same pattern implemented across other physics components, centralizing access to the native physics engine through the
Engineclass static property. This consistency makes the codebase more maintainable.Also applies to: 12-12
packages/design/src/physics/index.ts (1)
12-12: Expose new geometry abstractions.
Re-exporting./geometry/indexmakes the new geometry interfaces (e.g.,IGeometry,IBoxGeometry, etc.) available to consumers of the design package.
Remember to update the design package documentation or changelog to reflect the addition.packages/core/src/physics/shape/BoxColliderShape.ts (1)
4-4: Centralize native physics access via Engine.
Switching the import fromPhysicsScenetoEnginecentralizes all native-physics calls. This aligns with other collider shapes and ensures a single entry point for the physics backend.packages/core/src/physics/PhysicsMaterial.ts (1)
2-2: Centralize native material creation.
ImportingEngineand usingEngine._nativePhysics.createPhysicsMaterialconsolidates material creation through theEngineentry point, matching other physics objects.packages/core/src/physics/shape/CapsuleColliderShape.ts (1)
3-3: Centralize native physics reference.
Updating the import to useEngineensures a consistent access point for the native physics backend, matching other collider shapes.packages/core/src/physics/joint/HingeJoint.ts (2)
3-3: Import refactor aligns with centralized physics engine management.This change from importing
PhysicsScenetoEnginereflects the architectural shift to use a central physics engine instance from Engine class.
163-163: Properly leveraging centralized physics engine instance.The code now correctly uses
Engine._nativePhysicsinstead ofPhysicsScene._nativePhysics, consistent with the architecture refactor that centralizes the physics engine instance within the Engine class.packages/design/src/physics/geometry/ISphereGeometry.ts (1)
1-11: Well-defined interface for sphere geometry.This new interface properly extends the base
IGeometryinterface with a radius property specific to spheres. The JSDoc comments are clear and descriptive.packages/design/src/physics/geometry/index.ts (1)
1-5: Good use of barrel file pattern for geometry interfaces.The index file properly re-exports all geometry interfaces using TypeScript's
export typesyntax, making it easier to import these interfaces throughout the codebase.packages/core/src/physics/CharacterController.ts (2)
3-3: Import change aligns with centralized physics engine approach.Changing from
PhysicsScenetoEngineimport supports the architectural refactor to centralize physics engine access.
83-83: Properly updated to use centralized physics engine.This change correctly uses
Engine._nativePhysicsinstead of the previous approach, consistent with the architectural refactor for physics engine centralization.packages/core/src/Scene.ts (1)
517-518: Added physics cleanup on scene destructionThe addition of
this.physics._destroy()ensures proper cleanup of physics resources when a scene is destroyed. This is a necessary improvement to prevent memory leaks and resource leaks.packages/design/src/physics/geometry/IPlaneGeometry.ts (1)
1-6: LGTM: Clean interface definition for plane geometryThe IPlaneGeometry interface properly extends the base IGeometry interface, following the pattern of other geometry interfaces in the system. This maintains a consistent design approach across different geometry types.
packages/core/src/physics/HitResult.ts (1)
21-29: LGTM: Good implementation of overlap result structureThe new
OverlapHitResultclass provides a clear data structure for representing overlap query results, containing the essential entity and shape information. This complements the existingHitResultclass and maintains a consistent pattern.packages/physics-physx/src/shape/PhysXPlaneColliderShape.ts (2)
2-5: LGTM: Updated importsThe imports have been updated to align with the new geometry abstraction system. Removed the direct Vector3 import and added the PhysXPlaneGeometry import.
17-18: LGTM: Improved geometry encapsulationThis change encapsulates the PhysX geometry creation within the PhysXPlaneGeometry wrapper class instead of directly instantiating PhysX geometries. This is a good architectural improvement that:
- Provides better abstraction of the PhysX API
- Makes the code more maintainable
- Aligns with the pattern used for other geometry types
packages/design/src/physics/geometry/IBoxGeometry.ts (1)
4-12: Well-structured box geometry interface with clear type definitions.The interface correctly extends IGeometry and defines half extents with appropriate types, making it suitable for box-shaped collision objects in the physics system.
packages/physics-physx/src/shape/PhysXSphereGeometry.ts (1)
4-23: Clean implementation of the sphere geometry wrapper.The class correctly implements the ISphereGeometry interface while encapsulating the PhysX-specific implementation details. The getter and setter for radius work directly with the underlying native object, providing a clean abstraction.
packages/design/src/physics/geometry/ICapsuleGeometry.ts (1)
3-16: Well-defined capsule geometry interface.This interface properly extends IGeometry with the two essential properties for a capsule shape: radius and halfHeight. The JSDoc comments are clear and descriptive.
packages/core/src/Engine.ts (2)
61-62: Good centralization of physics engine reference.Adding a static reference to the physics engine at the Engine level makes architectural sense and will help with consistent access patterns.
626-631: Proper error handling when replacing an existing physics engine.The warning message effectively alerts developers when a physics engine is being replaced, while still allowing the operation to proceed, which maintains flexibility.
packages/physics-physx/src/shape/PhysXPlaneGeometry.ts (1)
1-12: Implementation follows the geometry abstraction pattern correctlyThe implementation provides a clean wrapper for PhysX plane geometry that properly extends the base geometry class and implements the plane geometry interface. It follows the same pattern as other geometry implementations in the codebase.
packages/physics-physx/src/shape/PhysXCapsuleGeometry.ts (3)
1-11: Implementation follows the geometry abstraction pattern correctlyThe class properly extends PhysXGeometry and implements ICapsuleGeometry interface, initializing the underlying PhysX capsule geometry with the specified radius and halfHeight parameters.
8000
13-22: Radius getters and setters are correctly implementedThe implementation provides direct access to the underlying PhysX geometry's radius property, which aligns with the pattern used in other geometry classes like PhysXSphereGeometry.
24-33: Half-height getters and setters are correctly implementedThe implementation provides direct access to the underlying PhysX geometry's halfHeight property, maintaining consistency with the approach used for other properties.
packages/physics-physx/src/shape/PhysXSphereColliderShape.ts (6)
5-5: Correctly imports the new PhysXSphereGeometry classThe import statement is properly added for the new geometry abstraction.
12-13: Property declaration for _physXGeometry is correctly typedThe protected property declaration uses the correct type and the
declarekeyword to ensure TypeScript recognizes it.
21-22: Properly initializes _physXGeometry and assigns _pxGeometryThe constructor now correctly creates a PhysXSphereGeometry instance with the scaled radius and assigns the underlying PhysX geometry object to _pxGeometry.
32-33: Radius updates now go through _physXGeometryThe setRadius method now updates the radius through the _physXGeometry instance, which centralizes geometry property management.
43-44: World scale updates now go through _physXGeometryThe setWorldScale method now properly updates the radius through the _physXGeometry instance after recalculating the max scale.
1-46:✅ Verification successful
Ensure proper resource cleanup for _physXGeometry
While the class correctly initializes and uses the new PhysXGeometry abstraction, it's not clear if _physXGeometry is properly released when the collider shape is destroyed. Make sure to call _physXGeometry.release() in the destructor or override method to prevent memory leaks.
Run this script to check if there's a proper cleanup mechanism in the parent class or in other methods:
🏁 Script executed:
#!/bin/bash # Look for destructor or cleanup methods in PhysXColliderShape rg -A 10 "destroy|cleanup|release|dispose" "packages/physics-physx/src/shape/PhysXColliderShape.ts" # Check if there's an override method in PhysXSphereColliderShape that might handle cleanup rg -A 5 "override" "packages/physics-physx/src/shape/PhysXSphereColliderShape.ts"Length of output: 845
No additional cleanup needed:
destroy()inPhysXColliderShapealready releases geometryThe base
PhysXColliderShape.destroy()method calls boththis._pxShape.release()andthis._physXGeometry.release(). SincePhysXSphereColliderShapeinherits this implementation and does not overridedestroy(), no extra cleanup is required in the subclass.packages/physics-physx/src/shape/PhysXBoxGeometry.ts (5)
1-6: Import and type definitions look goodThe imports and Vector3Like type definition are appropriate for the PhysXBoxGeometry implementation.
7-10: Documentation comment is concise and clearThe class documentation clearly indicates the purpose of this class.
10-14: Constructor implementation is correctThe constructor properly extends PhysXGeometry and initializes the native PxBoxGeometry with the provided half-extents.
16-21: Getter for halfExtents is implemented correctlyThe getter properly returns the half-extents from the underlying native geometry.
23-25: Setter for halfExtents is implemented correctlyThe setter properly updates the half-extents on the underlying native geometry.
packages/physics-physx/src/shape/PhysXBoxColliderShape.ts (5)
5-5: Import of PhysXBoxGeometry is correctly addedThe import is appropriately added to support the new geometry abstraction.
12-13: Protected geometry property declaration is appropriateThe _physXGeometry property is correctly declared with the appropriate type, using declare to ensure TypeScript correctly handles the property declaration in a derived class.
22-23: Constructor correctly initializes PhysXBoxGeometryThe implementation properly creates a PhysXBoxGeometry instance and gets the native geometry using getGeometry().
36-36: setSize method updated to use the geometry abstractionThe implementation correctly updates the halfExtents property on the PhysXBoxGeometry instance.
49-49: setWorldScale method updated to use the geometry abstractionThe implementation correctly updates the halfExtents property on the PhysXBoxGeometry instance when the world scale changes.
packages/physics-physx/src/shape/PhysXCapsuleColliderShape.ts (8)
6-6: Import of PhysXCapsuleGeometry is correctly addedThe import is appropriately added to support the new geometry abstraction.
15-16: Protected geometry property declaration is appropriateThe _physXGeometry property is correctly declared with the appropriate type, using declare to ensure TypeScript correctly handles the property declaration in a derived class.
34-35: Constructor correctly initializes PhysXCapsuleGeometryThe implementation properly creates a PhysXCapsuleGeometry instance and gets the native geometry using getGeometry().
48-54: setRadius method updated to use the geometry abstractionThe implementation correctly updates the radius property on the PhysXCapsuleGeometry instance for different up axis scenarios.
59-59: Using geometry properties for controller updatesThe code now uses _physXGeometry.radius for controller updates, which is consistent with the new abstraction pattern.
74-80: setHeight method updated to use the geometry abstractionThe implementation correctly updates the halfHeight property on the PhysXCapsuleGeometry instance for different up axis scenarios.
85-85: Using geometry properties for controller updatesThe code now uses _physXGeometry.halfHeight for controller updates, which is consistent with the new abstraction pattern.
125-140: setWorldScale method implementation is correctThe code correctly updates both the radius and halfHeight on the geometry based on the up axis and then updates the shape with the new geometry.
packages/physics-physx/src/shape/PhysXColliderShape.ts (7)
6-6: Import of PhysXGeometry is correctly addedThe import is appropriately added to support the new geometry abstraction.
30-30: Static vector for temporary calculations is well-definedThe static _tempVector4 property is appropriately defined for reuse in calculations.
43-44: Protected geometry properties are properly definedThe _physXGeometry and _pxGeometry properties are correctly declared to support the new abstraction pattern.
140-145: pointDistance method correctly uses the geometry abstractionThe method now delegates to _physXGeometry.pointDistance instead of directly calling the native API, which is consistent with the abstraction pattern.
151-152: Proper resource cleanup in destroy methodThe destroy method now correctly releases both the native shape and the geometry wrapper, preventing memory leaks.
160-162: Improved native resource management in _setShapeFlagsThe method now properly creates and deletes the PxShapeFlags instance, ensuring proper resource cleanup.
175-177: Improved native resource management in _initializeThe _initialize method now properly creates and deletes the PxShapeFlags instance, ensuring proper resource cleanup.
packages/design/src/physics/IPhysics.ts (2)
92-98: Generic parameter type looks goodThe addition of
createBoxGeometry()aligns well with the new geometry abstraction. ✔️
120-124: Plane geometry API fits abstractionNo concerns here. 👍
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/design/src/physics/geometry/IGeometry.ts (3)
10-10: Consider using a more specific return type forgetGeometry()The method returns
any, which reduces type safety. While this might be necessary if the underlying geometry objects vary widely, consider using a generic type parameter or a union type if the possible return types are known.- getGeometry(): any; + getGeometry<T = any>(): T;This would allow consumers to specify the expected return type when needed, while maintaining flexibility.
8-10: The interface documentation refers specifically to PhysXSince this is a general geometry interface that could potentially work with multiple physics engines, consider using more generic terminology in the documentation rather than specifically mentioning PhysX.
- * Get the underlying PhysX geometry object. + * Get the underlying physics engine geometry object.
23-26: Ensure proper resource management documentationThe
release()method implies manual resource management. Consider adding more detailed documentation about when this method should be called and the consequences of not calling it, to prevent memory leaks./** * Release the geometry resources. + * This must be called when the geometry is no longer needed to prevent memory leaks. + * After calling this method, the geometry object should not be used anymore. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/design/src/physics/geometry/IGeometry.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: e2e (22.x)
- GitHub Check: codecov
- GitHub Check: build (22.x, windows-latest)
🔇 Additional comments (1)
packages/design/src/physics/geometry/IGeometry.ts (1)
18-21: ThepointDistancemethod implementation matches the interfaceThe parameter type for
poseis now correctly defined as an object with translation and rotation properties, addressing the previous review comment.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev/1.5 #2657 +/- ##
===========================================
+ Coverage 69.06% 69.20% +0.14%
===========================================
Files 974 985 +11
Lines 102716 103274 +558
Branches 8904 8963 +59
===========================================
+ Hits 70936 71468 +532
- Misses 31512 31538 +26
Partials 268 268
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/src/core/physics/PhysicsScene.test.ts (1)
535-538: Suggest adding validation for multiple intersections.While the test coverage is comprehensive, consider adding tests for scenarios where the sweeps might hit multiple colliders and need to report all hits, not just the closest one.
This could be implemented by adding a test that casts through multiple colliders and verifies all hits are reported when using a multi-hit collection method (if available in the API).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (10)
packages/core/src/physics/CharacterController.ts(2 hunks)packages/core/src/physics/DynamicCollider.ts(2 hunks)packages/core/src/physics/PhysicsScene.ts(7 hunks)packages/design/src/physics/IPhysics.ts(5 hunks)packages/physics-lite/src/LitePhysics.ts(5 hunks)packages/physics-lite/src/LitePhysicsScene.ts(6 hunks)packages/physics-physx/src/PhysXCharacterController.ts(1 hunks)packages/physics-physx/src/PhysXPhysics.ts(7 hunks)packages/physics-physx/src/shape/PhysXColliderShape.ts(5 hunks)tests/src/core/physics/PhysicsScene.test.ts(34 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/core/src/physics/DynamicCollider.ts
- packages/physics-physx/src/PhysXCharacterController.ts
- packages/core/src/physics/CharacterController.ts
- packages/physics-physx/src/PhysXPhysics.ts
- packages/design/src/physics/IPhysics.ts
- packages/physics-physx/src/shape/PhysXColliderShape.ts
- packages/core/src/physics/PhysicsScene.ts
- packages/physics-lite/src/LitePhysicsScene.ts
- packages/physics-lite/src/LitePhysics.ts
🔇 Additional comments (11)
tests/src/core/physics/PhysicsScene.test.ts (11)
14-17: New imports support the sweep and overlap functionality.The added imports for
StaticCollider,OverlapHitResult,Quaternion, and the updated Vector3 import properly support the new physics functionality being tested in this file.
172-172: Consistent use of scene physics property replaces direct physics manager references.Code has been updated to access physics functionality through
sceneManager.scenes[0].physicsrather than a direct physics manager reference. This change improves consistency with the architectural shift where native physics engine instance was centralized fromPhysicsScene._nativePhysicstoEngine._nativePhysics.Also applies to: 196-196, 210-211, 214-217, 222-230, 233-236, 244-251, 260-262, 268-269, 274-276, 285-286, 295-295, 304-305, 311-317, 321-323, 345-346
539-654: Comprehensive boxCast tests cover all key scenarios.The new boxCast tests thoroughly validate:
- Basic hit/miss detection
- Layer masking
- Distance limits
- Behavior when box is inside collider
- Rotated box casting
- Multiple collider interactions
The tests cover all expected usage patterns for the boxCast functionality.
656-720: Complete test coverage for sphereCast functionality.The sphereCast tests methodically verify:
- Basic hit/miss scenarios
- Layer masks
- Distance limits
- Behavior when sphere is inside a collider
- Multiple collider interactions
This ensures the sphereCast functionality works correctly in all expected scenarios.
722-853: Thorough capsuleCast test implementation.The capsuleCast tests properly validate:
- Direction-based hit/miss scenarios
- Layer masking
- Distance limits
- Orientation/rotation handling
- Multiple collider scenarios
- Edge cases (capsule inside collider)
The tests provide comprehensive coverage of the capsuleCast functionality.
855-926: Well-structured overlapBox tests.The overlapBox tests properly check:
- Basic overlap/non-overlap scenarios
- Layer masking
- Box orientation and rotation
- Multiple collider detection
- Edge contact cases
The test structure follows a consistent pattern with the other spatial query tests.
928-999: Thorough overlapSphere tests match the testing pattern.The overlapSphere tests validate:
- Basic overlap detection
- Layer mask filtering
- Behavior when sphere contains collider
- Multiple collider interactions
- Edge cases
The tests maintain consistency with the other spatial query test patterns.
1001-1111: Complete overlapCapsule tests provide good coverage.The overlapCapsule tests verify:
- Basic overlap detection with various parameters
- Layer mask filtering
- Behavior with collider inside capsule
- Rotation and orientation handling
- Multiple collider scenarios
- Edge contact cases
These tests complete the comprehensive coverage of the new spatial query functionality.
1514-1525: Updated collision expectations for kinematic colliders.The test expectations have been correctly updated to reflect changes in how dynamic kinematic colliders interact. The test now correctly expects no trigger events when both colliders are dynamic and kinematic, which is consistent with the physics engine behavior.
1145-1146: Consistent physics access in collision tests.All collision tests have been updated to access physics through
enginePhysX.sceneManager.scenes[0].physicsinstead of directly using a physics manager reference. This change is consistent with the architectural updates in the engine.Also applies to: 1169-1170, 1193-1194, 1217-1218, 1241-1242, 1265-1266, 1289-1290, 1313-1314, 1337-1338, 1361-1362, 1385-1386, 1409-1410, 1433-1434, 1457-1458, 1481-1482, 1505-1506, 1529-1530, 1553-1554
1-1583:❌ Incorrect review comment
Missing unit tests for the new physics API.
While the tests thoroughly validate the PhysX implementation of sweep and overlap queries, there are no equivalent tests for the LitePhysics implementation. This could lead to inconsistent behavior between physics backends.
Consider adding the same sweep and overlap tests to the LitePhysics test section, or verify if LitePhysics supports these operations with the following script:
🏁 Script executed:
#!/bin/bash # Check if LitePhysics implements the sweep and overlap methods # Look for sweep and overlap method implementations in LitePhysics echo "Searching for sweep and overlap implementations in LitePhysics..." rg -A 1 "class (LitePhysicsScene|LitePhysics)" --type ts | grep -B 3 -A 20 "boxCast|sphereCast|capsuleCast|overlapBox|overlapSphere|overlapCapsule" # Check if there are any test skipping mechanisms for these features echo "Checking for feature detection or test skipping..." rg "boxCast|sphereCast|capsuleCast|overlapBox|overlapSphere|overlapCapsule" --type ts | grep -B 3 -A 3 "if|switch|skip|todo"Length of output: 528
🏁 Script executed:
#!/bin/bash # Search for LitePhysicsScene class definition echo "Searching for LitePhysicsScene class definition..." rg -n "class LitePhysicsScene" . # Search for sweep and overlap method implementations across the repo echo "Searching for sweep and overlap method implementations..." rg -nE "boxCast|sphereCast|capsuleCast|overlapBox|overlapSphere|overlapCapsule" .Length of output: 570
🏁 Script executed:
#!/bin/bash # Search across physics-lite for sweep and overlap features echo "Searching for boxCast..." grep -RIn "boxCast" packages/physics-lite || echo "not found" echo "Searching for sphereCast..." grep -RIn "sphereCast" packages/physics-lite || echo "not found" echo "Searching for capsuleCast..." grep -RIn "capsuleCast" packages/physics-lite || echo "not found" echo "Searching for overlapBox..." grep -RIn "overlapBox" packages/physics-lite || echo "not found" echo "Searching for overlapSphere..." grep -RIn "overlapSphere" packages/physics-lite || echo "not found" echo "Searching for overlapCapsule..." grep -RIn "overlapCapsule" packages/physics-lite || echo "not found"Length of output: 848
LitePhysics does not implement sweep/overlap APIs—tests not needed
A search of packages/physics-lite/src/LitePhysicsScene.ts shows no boxCast, sphereCast, capsuleCast, overlapBox, overlapSphere or overlapCapsule methods. Because LitePhysics doesn’t currently support these operations, adding equivalent sweep/overlap tests isn’t applicable. If support is added in the future, corresponding tests can be introduced then.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/core/src/physics/PhysicsScene.ts (1)
404-410: Leaking native geometry objects – remember torelease()Every call to the helper methods (
boxCast,sphereCast,capsuleCast,overlapBox,overlapSphere,overlapCapsule) creates a native geometry viaEngine._nativePhysics.create*Geometry(...)but never releases it, so each sweep/overlap leaves behind unmanaged PhysX allocations.Apply this fix to all six helper methods:
boxCast( center: Vector3, orientation: Quaternion, halfExtents: Vector3, direction: Vector3, distance?: number, layerMask?: Layer, outHitResult?: HitResult ): boolean { const boxGeometry = Engine._nativePhysics.createBoxGeometry(halfExtents); const pose = PhysicsScene._tempPose; pose.translation.copyFrom(center); pose.rotation.copyFrom(orientation); - return this._sweep(boxGeometry, pose, direction, distance, layerMask, outHitResult); + try { + return this._sweep(boxGeometry, pose, direction, distance, layerMask, outHitResult); + } finally { + boxGeometry.release(); + } }
🧹 Nitpick comments (2)
packages/core/src/physics/PhysicsScene.ts (2)
631-678: Consider handling geometry lifetime in the _sweep methodAs an alternative to fixing the memory leak in each individual cast method, you could modify the
_sweepmethod to take ownership of the geometry object and release it when done.private _sweep( geometry: IGeometry, pose: { translation: Vector3; rotation: Quaternion }, direction: Vector3, distance?: number, layerMask?: Layer, outHitResult?: HitResult ): boolean { const maxDistance = distance ?? Number.MAX_VALUE; const mask = layerMask ?? Layer.Everything; const preFilter = (obj: number) => { const shape = Engine._physicalObjectsMap[obj]; if (!shape) { return false; } return shape.collider.entity.layer & mask && shape.isSceneQuery; }; + try { if (outHitResult !== undefined) { const result = this._nativePhysicsScene.sweep( geometry, pose, direction, maxDistance, preFilter, (shapeUniqueID, distance, position, normal) => { outHitResult.entity = Engine._physicalObjectsMap[shapeUniqueID].collider.entity; outHitResult.shape = Engine._physicalObjectsMap[shapeUniqueID]; outHitResult.distance = distance; outHitResult.point.copyFrom(position); outHitResult.normal.copyFrom(normal); } ); if (!result) { outHitResult.entity = null; outHitResult.shape = null; outHitResult.distance = 0; outHitResult.point.set(0, 0, 0); outHitResult.normal.set(0, 0, 0); return false; } return true; } return this._nativePhysicsScene.sweep(geometry, pose, direction, maxDistance, preFilter); + } finally { + geometry.release(); + } }If you implement this change, you would then remove the try/finally blocks from the individual cast methods.
680-712: Consider handling geometry lifetime in the _overlap methodSimilar to the
_sweepmethod, the_overlapmethod should take ownership of the geometry object and release it when done.private _overlap( geometry: IGeometry, pose: { translation: Vector3; rotation: Quaternion }, layerMask?: Layer, outOverlapHitResult?: OverlapHitResult ): boolean { const mask = layerMask ?? Layer.Everything; const preFilter = (obj: number) => { const shape = Engine._physicalObjectsMap[obj]; if (!shape) { return false; } return shape.collider.entity.layer & mask && shape.isSceneQuery; }; + try { if (outOverlapHitResult !== undefined) { const result = this._nativePhysicsScene.overlapAny(geometry, pose, preFilter, (shapeUniqueID) => { const hitShape = Engine._physicalObjectsMap[shapeUniqueID]; outOverlapHitResult.entity = hitShape._collider.entity; outOverlapHitResult.shape = hitShape; }); if (!result) { outOverlapHitResult.entity = null; outOverlapHitResult.shape = null; return false; } return true; } return this._nativePhysicsScene.overlapAny(geometry, pose, preFilter); + } finally { + geometry.release(); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
packages/core/src/physics/PhysicsScene.ts(9 hunks)packages/physics-physx/src/shape/PhysXColliderShape.ts(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/physics-physx/src/shape/PhysXColliderShape.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/core/src/physics/PhysicsScene.ts (3)
packages/core/src/Engine.ts (1)
Engine(52-697)packages/core/src/physics/HitResult.ts (2)
HitResult(8-19)OverlapHitResult(24-29)packages/design/src/physics/geometry/IGeometry.ts (1)
IGeometry(6-27)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: codecov
- GitHub Check: build (22.x, windows-latest)
- GitHub Check: e2e (22.x)
🔇 Additional comments (4)
packages/core/src/physics/PhysicsScene.ts (4)
1-11: LGTM: Good import organizationThe additions of
IGeometryandOverlapHitResultimports are appropriate for the new functionality.
344-350: LGTM: Consistent filtering logicThe filtering logic is consistently implemented across raycast, sweep, and overlap methods, which is good for maintainability.
Also applies to: 642-648, 688-694
18-21:❌ Incorrect review comment
Consider thread safety with static _tempPose
The static
_tempPoseobject is reused across multiple methods. This could lead to race conditions if multiple sweep or overlap queries are performed concurrently from different contexts (e.g., worker threads or within the same frame).Consider either:
- Making the pose object local to each method call (more memory but thread-safe)
- Ensuring these methods are never called concurrently
- Using a pool pattern if performance is critical
🏁 Script executed:
#!/bin/bash # Check for potential concurrent usage of PhysicsScene methods rg ".*Cast|overlap.*" --type ts packages/core --context 5 | grep -v "packages/core/src/physics/PhysicsScene.ts"Length of output: 26262
Ignore thread-safety warning for static _tempPose
The
_tempPoseobject is only used within synchronous, single‐threaded query methods (and in separate worker contexts each with its own module instance), so there’s no risk of concurrent modification. No changes required here.Likely an incorrect or invalid review comment.
623-625:✅ Verification successful
Verify destroy behavior is properly hooked into the scene lifecycle
You've added a new
_destroy()method to clean up the native physics scene, but make sure it's actually called during scene destruction.
🏁 Script executed:
#!/bin/bash # Check if the _destroy method is called somewhere in the scene lifecycle rg "physicsScene\._destroy" --type ts packages/coreLength of output: 53
I’m not seeing any direct calls to
_destroy()—let’s pinpoint wherePhysicsSceneis constructed and whether its destroy hook is ever invoked:
🏁 Script executed:
#!/bin/bash # 1. List all occurrences of the _destroy method rg -n '_destroy\(' --type ts packages # 2. Find where PhysicsScene is instantiated rg -n 'new PhysicsScene' --type ts packages # 3. Check all references to any physicsScene property/variable rg -n 'physicsScene' --type ts packagesLength of output: 2380
PhysicsScene destruction is already wired up
The new
PhysicsScene._destroy()is invoked inScene._destroy()(packages/core/src/Scene.ts:508), which is part of the scene’s teardown. No additional hooks are needed.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
tests/src/core/Polyfill.test.ts (1)
39-72:⚠️ Potential issueTest re-implements the production polyfill instead of exercising it
This test implements its own version of the AudioContext polyfill logic rather than validating the actual implementation in the codebase. The current approach means that if the production polyfill code in
packages/core/src/Polyfill.tschanges or breaks, this test would still pass.Instead of reimplementing the polyfill, consider rewriting the test to validate the actual production code:
- delete window.AudioContext; - (window as any).webkitAudioContext = class MockWebkitAudioContext { - state = "suspended"; - - constructor() { } - - decodeAudioData(arrayBuffer: ArrayBuffer, successCallback: Function) { - setTimeout(() => { - successCallback({ duration: 10 } as AudioBuffer); - }, 10); - } - }; - - window.AudioContext = (window as any).webkitAudioContext; - - expect(window.AudioContext).to.equal((window as any).webkitAudioContext); - - const context = new window.AudioContext(); - - const originalDecodeAudioData = context.decodeAudioData; - - AudioContext.prototype.decodeAudioData = function(arrayBuffer: ArrayBuffer): Promise<AudioBuffer> { - const self = this; - return new Promise(resolve => { - originalDecodeAudioData.apply(self, [arrayBuffer, resolve]); - }); - }; + // Remove native AudioContext to force the polyfill path + delete (window as any).AudioContext; + + // Create minimal mock for webkitAudioContext + (window as any).webkitAudioContext = class { + state = "suspended"; + + decodeAudioData(arrayBuffer: ArrayBuffer, successCallback: (buffer: AudioBuffer) => void) { + setTimeout(() => { + successCallback({ duration: 10 } as AudioBuffer); + }, 10); + } + }; + + // Import and register the actual polyfill + const { Polyfill } = await import("@galacean/engine-core"); + Polyfill.registerPolyfill(); + + // Create context using the polyfilled AudioContext + const context = new AudioContext();This approach:
- Tests the actual polyfill implementation, not a copy
- Will fail if the polyfill is removed or regressed
- Properly addresses static analysis issues with more specific types
🧰 Tools
🪛 Biome (1.9.4)
[error] 40-40: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 45-45: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
[error] 47-47: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
[error] 63-63: This aliasing of this is unnecessary.
Arrow functions inherits
thisfrom their enclosing scope.
Safe fix: Use this instead of an alias.(lint/complexity/noUselessThisAlias)
🪛 ESLint
[error] 41-41: Delete
······(prettier/prettier)
[error] 44-44: Delete
······(prettier/prettier)
[error] 45-45: Delete
·(prettier/prettier)
[error] 46-46: Delete
······(prettier/prettier)
[error] 53-53: Delete
····(prettier/prettier)
[error] 55-55: Delete
····(prettier/prettier)
[error] 57-57: Delete
······(prettier/prettier)
[error] 59-59: Delete
····(prettier/prettier)
[error] 61-61: Delete
····(prettier/prettier)
[error] 62-62: Insert
·(prettier/prettier)
[error] 64-64: Replace
resolvewith(resolve)(prettier/prettier)
[error] 68-68: Delete
····(prettier/prettier)
packages/core/src/physics/PhysicsScene.ts (6)
494-498:⚠️ Potential issueAdd resource cleanup to prevent memory leaks
The
overlapSpheremethod creates a native geometry object without releasing it.Ensure proper cleanup of native geometry objects with a try/finally pattern:
overlapSphere(center: Vector3, radius: number, layerMask?: Layer, outOverlapHitResult?: OverlapHitResult): boolean { const sphereGeometry = Engine._nativePhysics.createSphereGeometry(radius); + try { const pose = PhysicsScene._tempPose; pose.translation.copyFrom(center); return this._overlap(sphereGeometry, pose, layerMask, outOverlapHitResult); + } finally { + sphereGeometry.release(); + } }
457-462:⚠️ Potential issueAdd resource cleanup to prevent memory leaks
The
capsuleCastmethod also creates a native geometry object without releasing it.Ensure proper cleanup of native geometry objects with a try/finally pattern:
capsuleCast( center: Vector3, radius: number, height: number, orientation: Quaternion, direction: Vector3, distance?: number, layerMask?: Layer, outHitResult?: HitResult ): boolean { const capsuleGeometry = Engine._nativePhysics.createCapsuleGeometry(radius, height); + try { const pose = PhysicsScene._tempPose; pose.translation.copyFrom(center); pose.rotation.copyFrom(orientation); return this._sweep(capsuleGeometry, pose, direction, distance, layerMask, outHitResult); + } finally { + capsuleGeometry.release(); + } }
517-522:⚠️ Potential issueAdd resource cleanup to prevent memory leaks
The
overlapCapsulemethod creates a native geometry object without releasing it.Ensure proper cleanup of native geometry objects with a try/finally pattern:
overlapCapsule( center: Vector3, radius: number, height: number, orientation: Quaternion, layerMask?: Layer, outOverlapHitResult?: OverlapHitResult ): boolean { const capsuleGeometry = Engine._nativePhysics.createCapsuleGeometry(radius, height); + try { const pose = PhysicsScene._tempPose; pose.translation.copyFrom(center); pose.rotation.copyFrom(orientation); return this._overlap(capsuleGeometry, pose, layerMask, outOverlapHitResult); + } finally { + capsuleGeometry.release(); + } }
404-410:⚠️ Potential issueAdd resource cleanup to prevent memory leaks
The
boxCastmethod creates a native geometry object but does not release it, potentially causing memory leaks. Each call will allocate unmanaged resources without releasing them.Ensure proper cleanup of native geometry objects with a try/finally pattern:
boxCast( center: Vector3, orientation: Quaternion, halfExtents: Vector3, direction: Vector3, distance?: number, layerMask?: Layer, outHitResult?: HitResult ): boolean { const boxGeometry = Engine._nativePhysics.createBoxGeometry(halfExtents); + try { const pose = PhysicsScene._tempPose; pose.translation.copyFrom(center); pose.rotation.copyFrom(orientation); return this._sweep(boxGeometry, pose, direction, distance, layerMask, outHitResult); + } finally { + boxGeometry.release(); + } }
479-484:⚠️ Potential issueAdd resource cleanup to prevent memory leaks
The
overlapBoxmethod creates a native geometry object without releasing it.Ensure proper cleanup of native geometry objects with a try/finally pattern:
overlapBox( center: Vector3, orientation: Quaternion, halfExtents: Vector3, layerMask?: Layer, outOverlapHitResult?: OverlapHitResult ): boolean { const boxGeometry = Engine._nativePhysics.createBoxGeometry(halfExtents); + try { const pose = PhysicsScene._tempPose; pose.translation.copyFrom(center); pose.rotation.copyFrom(orientation); return this._overlap(boxGeometry, pose, layerMask, outOverlapHitResult); + } finally { + boxGeometry.release(); + } }
430-434:⚠️ Potential issueAdd resource cleanup to prevent memory leaks
Similar to
boxCast, thesphereCastmethod creates a native geometry object but doesn't release it.Ensure proper cleanup of native geometry objects with a try/finally pattern:
sphereCast( center: Vector3, radius: number, direction: Vector3, distance?: number, layerMask?: Layer, outHitResult?: HitResult ): boolean { const sphereGeometry = Engine._nativePhysics.createSphereGeometry(radius); + try { const pose = PhysicsScene._tempPose; pose.translation.copyFrom(center); return this._sweep(sphereGeometry, pose, direction, distance, layerMask, outHitResult); + } finally { + sphereGeometry.release(); + } }
🧹 Nitpick comments (8)
tests/src/core/Polyfill.test.ts (2)
63-66: Remove unnecessarythisaliasingWhen using arrow functions,
thisis already inherited from the enclosing scope, making the alias unnecessary.- const self = this; - return new Promise(resolve => { - originalDecodeAudioData.apply(self, [arrayBuffer, resolve]); - }); + return new Promise(resolve => { + originalDecodeAudioData.apply(this, [arrayBuffer, resolve]); + });🧰 Tools
🪛 Biome (1.9.4)
[error] 63-63: This aliasing of this is unnecessary.
Arrow functions inherits
thisfrom their enclosing scope.
Safe fix: Use this instead of an alias.(lint/complexity/noUselessThisAlias)
🪛 ESLint
[error] 64-64: Replace
resolvewith(resolve)(prettier/prettier)
45-45: Remove unnecessary empty constructorThe empty constructor doesn't add any functionality and can be removed.
- constructor() { }🧰 Tools
🪛 Biome (1.9.4)
[error] 45-45: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
🪛 ESLint
[error] 45-45: Delete
·(prettier/prettier)
packages/core/src/physics/PhysicsScene.ts (6)
631-638: Update parameter types to match usage patternsThe
_sweepmethod declaresdistanceandlayerMaskas optional parameters, but they're used with optional chaining, which is the correct approach. However, when using strict null checks in TypeScript, the type definitions should explicitly mark these as optional.private _sweep( geometry: IGeometry, pose: { translation: Vector3; rotation: Quaternion }, direction: Vector3, distance?: number, layerMask?: Layer, outHitResult?: HitResult ): boolean {
680-685: Update parameter types to match usage patternsSimilarly, the
_overlapmethod should explicitly marklayerMaskas optional to match its usage pattern.private _overlap( geometry: IGeometry, pose: { translation: Vector3; rotation: Quaternion }, layerMask?: Layer, outOverlapHitResult?: OverlapHitResult ): boolean {
623-625: Add null check defensive programmingThe
_destroymethod callsdestroy()onthis._nativePhysicsScenewith optional chaining, which is good. However, consider adding a more complete cleanup pattern._destroy() { if (this._nativePhysicsScene) { this._nativePhysicsScene.destroy(); this._nativePhysicsScene = null; } }
385-410: Add parameter validation for halfExtentsThe
boxCastmethod acceptshalfExtentswithout validation. Consider adding validation to ensure all dimensions are positive.boxCast( center: Vector3, orientation: Quaternion, halfExtents: Vector3, direction: Vector3, distance?: number, layerMask?: Layer, outHitResult?: HitResult ): boolean { + if (halfExtents.x <= 0 || halfExtents.y <= 0 || halfExtents.z <= 0) { + console.warn("BoxCast: halfExtents must have positive values on all axes"); + return false; + } const boxGeometry = Engine._nativePhysics.createBoxGeometry(halfExtents); // ...
422-434: Add parameter validation for radiusThe
sphereCastmethod acceptsradiuswithout validation. Consider adding validation to ensure it's positive.sphereCast( center: Vector3, radius: number, direction: Vector3, distance?: number, layerMask?: Layer, outHitResult?: HitResult ): boolean { + if (radius <= 0) { + console.warn("SphereCast: radius must be positive"); + return false; + } const sphereGeometry = Engine._nativePhysics.createSphereGeometry(radius); // ...
447-462: Add parameter validation for radius and heightThe
capsuleCastmethod acceptsradiusandheightwithout validation. Consider adding validation to ensure they're positive.capsuleCast( center: Vector3, radius: number, height: number, orientation: Quaternion, direction: Vector3, distance?: number, layerMask?: Layer, outHitResult?: HitResult ): boolean { + if (radius <= 0 || height <= 0) { + console.warn("CapsuleCast: radius and height must be positive"); + return false; + } const capsuleGeometry = Engine._nativePhysics.createCapsuleGeometry(radius, height); // ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (45)
packages/core/src/Engine.ts(2 hunks)packages/core/src/Polyfill.ts(2 hunks)packages/core/src/Scene.ts(1 hunks)packages/core/src/Utils.ts(1 hunks)packages/core/src/physics/CharacterController.ts(2 hunks)packages/core/src/physics/DynamicCollider.ts(2 hunks)packages/core/src/physics/HitResult.ts(1 hunks)packages/core/src/physics/PhysicsMaterial.ts(2 hunks)packages/core/src/physics/PhysicsScene.ts(9 hunks)packages/core/src/physics/StaticCollider.ts(2 hunks)packages/core/src/physics/index.ts(1 hunks)packages/core/src/physics/joint/FixedJoint.ts(2 hunks)packages/core/src/physics/joint/HingeJoint.ts(2 hunks)packages/core/src/physics/joint/SpringJoint.ts(2 hunks)packages/core/src/physics/shape/BoxColliderShape.ts(2 hunks)packages/core/src/physics/shape/CapsuleColliderShape.ts(2 hunks)packages/core/src/physics/shape/PlaneColliderShape.ts(2 hunks)packages/core/src/physics/shape/SphereColliderShape.ts(2 hunks)packages/design/src/physics/IPhysics.ts(5 hunks)packages/design/src/physics/IPhysicsScene.ts(3 hunks)packages/design/src/physics/geometry/IBoxGeometry.ts(1 hunks)packages/design/src/physics/geometry/ICapsuleGeometry.ts(1 hunks)packages/design/src/physics/geometry/IGeometry.ts(1 hunks)packages/design/src/physics/geometry/IPlaneGeometry.ts(1 hunks)packages/design/src/physics/geometry/ISphereGeometry.ts(1 hunks)packages/design/src/physics/geometry/index.ts(1 hunks)packages/design/src/physics/index.ts(1 hunks)packages/physics-lite/src/LitePhysics.ts(5 hunks)packages/physics-lite/src/LitePhysicsScene.ts(6 hunks)packages/physics-physx/src/PhysXCharacterController.ts(1 hunks)packages/physics-physx/src/PhysXPhysics.ts(7 hunks)packages/physics-physx/src/PhysXPhysicsScene.ts(12 hunks)packages/physics-physx/src/joint/PhysXJoint.ts(0 hunks)packages/physics-physx/src/shape/PhysXBoxColliderShape.ts(4 hunks)packages/physics-physx/src/shape/PhysXBoxGeometry.ts(1 hunks)packages/physics-physx/src/shape/PhysXCapsuleColliderShape.ts(7 hunks)packages/physics-physx/src/shape/PhysXCapsuleGeometry.ts(1 hunks)packages/physics-physx/src/shape/PhysXColliderShape.ts(6 hunks)packages/physics-physx/src/shape/PhysXGeometry.ts(1 hunks)packages/physics-physx/src/shape/PhysXPlaneColliderShape.ts(2 hunks)packages/physics-physx/src/shape/PhysXPlaneGeometry.ts(1 hunks)packages/physics-physx/src/shape/PhysXSphereColliderShape.ts(3 hunks)packages/physics-physx/src/shape/PhysXSphereGeometry.ts(1 hunks)tests/src/core/Polyfill.test.ts(1 hunks)tests/src/core/physics/PhysicsScene.test.ts(34 hunks)
💤 Files with no reviewable changes (1)
- packages/physics-physx/src/joint/PhysXJoint.ts
🚧 Files skipped from review as they are similar to previous changes (41)
- packages/core/src/physics/shape/SphereColliderShape.ts
- packages/core/src/physics/StaticCollider.ts
- packages/core/src/physics/shape/BoxColliderShape.ts
- packages/core/src/physics/joint/SpringJoint.ts
- packages/design/src/physics/index.ts
- packages/physics-physx/src/PhysXCharacterController.ts
- packages/core/src/physics/PhysicsMaterial.ts
- packages/core/src/physics/shape/CapsuleColliderShape.ts
- packages/core/src/physics/joint/HingeJoint.ts
- packages/core/src/physics/DynamicCollider.ts
- packages/physics-physx/src/shape/PhysXCapsuleGeometry.ts
- packages/core/src/Engine.ts
- packages/design/src/physics/geometry/ISphereGeometry.ts
- packages/core/src/physics/joint/FixedJoint.ts
- packages/core/src/physics/CharacterController.ts
- packages/core/src/Scene.ts
- packages/core/src/physics/index.ts
- packages/core/src/physics/shape/PlaneColliderShape.ts
- packages/core/src/Utils.ts
- packages/design/src/physics/geometry/IPlaneGeometry.ts
- packages/design/src/physics/geometry/IBoxGeometry.ts
- packages/design/src/physics/geometry/index.ts
- packages/design/src/physics/geometry/ICapsuleGeometry.ts
- packages/physics-physx/src/shape/PhysXPlaneColliderShape.ts
- packages/physics-physx/src/shape/PhysXSphereColliderShape.ts
- packages/physics-physx/src/shape/PhysXSphereGeometry.ts
- packages/physics-physx/src/shape/PhysXPlaneGeometry.ts
- packages/physics-physx/src/shape/PhysXBoxColliderShape.ts
- packages/physics-physx/src/shape/PhysXBoxGeometry.ts
- packages/physics-physx/src/PhysXPhysics.ts
- packages/core/src/Polyfill.ts
- packages/physics-physx/src/shape/PhysXCapsuleColliderShape.ts
- packages/physics-physx/src/shape/PhysXGeometry.ts
- packages/design/src/physics/IPhysics.ts
- packages/physics-physx/src/shape/PhysXColliderShape.ts
- packages/design/src/physics/geometry/IGeometry.ts
- packages/core/src/physics/HitResult.ts
- packages/physics-lite/src/LitePhysicsScene.ts
- packages/physics-physx/src/PhysXPhysicsScene.ts
- packages/physics-lite/src/LitePhysics.ts
- packages/design/src/physics/IPhysicsScene.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/core/src/physics/PhysicsScene.ts (4)
packages/core/src/physics/Collision.ts (1)
Collision(8-47)packages/core/src/Engine.ts (1)
Engine(52-697)packages/core/src/physics/HitResult.ts (2)
HitResult(8-19)OverlapHitResult(24-29)packages/design/src/physics/geometry/IGeometry.ts (1)
IGeometry(6-27)
tests/src/core/physics/PhysicsScene.test.ts (8)
packages/core/src/physics/PhysicsScene.ts (2)
fixedTimeStep(206-208)fixedTimeStep(210-212)packages/math/src/index.ts (1)
Ray(12-12)packages/core/src/physics/StaticCollider.ts (1)
StaticCollider(9-21)packages/core/src/physics/index.ts (3)
StaticCollider(7-7)HitResult(4-4)OverlapHitResult(4-4)packages/core/src/physics/shape/BoxColliderShape.ts (1)
BoxColliderShape(10-48)packages/core/src/physics/HitResult.ts (2)
HitResult(8-19)OverlapHitResult(24-29)packages/core/src/physics/shape/CapsuleColliderShape.ts (4)
radius(17-19)radius(21-26)height(31-33)height(35-40)packages/core/src/physics/shape/SphereColliderShape.ts (2)
radius(14-16)radius(18-23)
🪛 Biome (1.9.4)
tests/src/core/Polyfill.test.ts
[error] 40-40: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 45-45: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
[error] 47-47: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
[error] 63-63: This aliasing of this is unnecessary.
Arrow functions inherits this from their enclosing scope.
Safe fix: Use this instead of an alias.
(lint/complexity/noUselessThisAlias)
🪛 ESLint
tests/src/core/Polyfill.test.ts
[error] 41-41: Delete ······
(prettier/prettier)
[error] 44-44: Delete ······
(prettier/prettier)
[error] 45-45: Delete ·
(prettier/prettier)
[error] 46-46: Delete ······
(prettier/prettier)
[error] 53-53: Delete ····
(prettier/prettier)
[error] 55-55: Delete ····
(prettier/prettier)
[error] 57-57: Delete ······
(prettier/prettier)
[error] 59-59: Delete ····
(prettier/prettier)
[error] 61-61: Delete ····
(prettier/prettier)
[error] 62-62: Insert ·
(prettier/prettier)
[error] 64-64: Replace resolve with (resolve)
(prettier/prettier)
[error] 68-68: Delete ····
(prettier/prettier)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: e2e (22.x)
- GitHub Check: e2e (22.x)
- GitHub Check: codecov
🔇 Additional comments (12)
tests/src/core/Polyfill.test.ts (1)
39-72: PR title and implementation mismatchThe PR is titled "Support sweep and overlap" but this test is implementing AudioContext polyfill testing. This mismatch raises concerns about the scope and purpose of the PR.
Please clarify how this AudioContext polyfill test relates to the sweep and overlap functionality mentioned in the PR title. If they're unrelated, consider creating a separate PR for these changes.
🧰 Tools
🪛 Biome (1.9.4)
[error] 40-40: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 45-45: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
[error] 47-47: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
[error] 63-63: This aliasing of this is unnecessary.
Arrow functions inherits
thisfrom their enclosing scope.
Safe fix: Use this instead of an alias.(lint/complexity/noUselessThisAlias)
🪛 ESLint
[error] 41-41: Delete
······(prettier/prettier)
[error] 44-44: Delete
······(prettier/prettier)
[error] 45-45: Delete
·(prettier/prettier)
[error] 46-46: Delete
······(prettier/prettier)
[error] 53-53: Delete
····(prettier/prettier)
[error] 55-55: Delete
····(prettier/prettier)
[error] 57-57: Delete
······(prettier/prettier)
[error] 59-59: Delete
····(prettier/prettier)
[error] 61-61: Delete
····(prettier/prettier)
[error] 62-62: Insert
·(prettier/prettier)
[error] 64-64: Replace
resolvewith(resolve)(prettier/prettier)
[error] 68-68: Delete
····(prettier/prettier)
packages/core/src/physics/PhysicsScene.ts (3)
18-21: Good use of a static shared object for pose dataThe addition of a static
_tempPoseobject is an effective optimization to avoid repeated object creation during geometric query operations.
1-2: Clean and necessary importsThe imports are well-organized and include only what's needed for the new functionality.
344-350: Good use of unified preFilter functionThe preFilter function for raycast has been renamed and unified with the same approach used in the new sweep and overlap methods, improving code consistency.
tests/src/core/physics/PhysicsScene.test.ts (8)
539-654: Well-structured and comprehensive tests for boxCast functionalityThe new
boxCasttests cover a variety of scenarios including:
- No hit detection
- Hit detection
- Layer mask filtering
- Distance limiting
- Box inside collider
- Rotation tests
- Multiple collider tests
- Direction handling
This is a thorough test suite that validates the functionality works as expected across different use cases.
656-720: Comprehensive tests for sphereCas DEE6 t functionalityThe
sphereCasttests follow a similar pattern to theboxCasttests, covering the essential scenarios for sphere casting. The structure is consistent, which makes the tests easy to follow and maintain.
722-853: Well-designed tests for capsuleCast functionalityThe
capsuleCasttests cover all the important scenarios, including rotation handling and multiple collider interactions. The test cases are thorough and establish confidence in the implementation.
855-926: Good coverage of overlapBox functionalityThe
overlapBoxtests verify basic overlap detection, layer masking, and various geometric configurations, including edge cases like containing the entire collider and edge contacts.
928-999: Thorough testing of overlapSphere functionalityThe
overlapSpheretests follow the established pattern and provide good coverage of the functionality.
1001-1111: Comprehensive tests for overlapCapsule functionalityThe
overlapCapsuletests cover all essential scenarios, including capsule orientation, which is particularly important for accurate collision detection.
172-172: Consistent use of scene physics accessThe test has been updated to use
engineLite.sceneManager.scenes[0].physicsinstead of directly accessing the physics manager, which aligns with the architecture changes.
1514-1526: Corrected test case for dynamic kinematic collidersThis test case now correctly reflects that collisions or triggers don't occur between two dynamic kinematic colliders, updating the expectations accordingly.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Other information:
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation
Refactor