Refactor capsule creation to support dynamic shape selection#597
Refactor capsule creation to support dynamic shape selection#597tracygardner wants to merge 3 commits intomainfrom
Conversation
Multi-object GLB models (e.g. airplane) have a vertex-less root mesh. PhysicsShapeMesh on empty geometry produces a degenerate Havok shape (a sphere), causing the model to float above the ground. Fall back to PhysicsShapeBox derived from the hierarchical bounding box instead. https://claude.ai/code/session_01VZjZFU3AZyeseyRxtLuEwW
In setupMesh, createCapsuleFromBoundingBox is called for all loaded models. For wide/flat models like the airplane the capsule radius exceeds half the height, producing a sphere (cylinderHeight=0) centred on the bounding box rather than the base, which floats the model above the ground. Fall back to PhysicsShapeBox when height < 2*capsuleRadius. https://claude.ai/code/session_01VZjZFU3AZyeseyRxtLuEwW
Rename createCapsuleFromBoundingBox to createShapeFromBoundingBox and make it return a PhysicsShapeBox when the bounding box is wider than it is tall (height < 2*radius), which would otherwise produce a sphere that floats the model above the ground. Tall models continue to get a capsule as before. https://claude.ai/code/session_01VZjZFU3AZyeseyRxtLuEwW
📝 WalkthroughWalkthroughThe PR refactors physics shape creation by renaming Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
Deploying flockxr with
|
| Latest commit: |
faf074d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://165d37fa.flockxr.pages.dev |
| Branch Preview URL: | https://claude-fix-airplane-physics.flockxr.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/animate.js (1)
18-29:⚠️ Potential issue | 🟠 MajorAllow animation collider swaps to start from the box fallback.
When the "vertical" animation path creates a
PhysicsShapeBoxviacreateShapeFromBoundingBox(api/animate.js:67), the next animation change exits early because the guard at lines 19-27 only acceptsPhysicsShapeCapsuleinstances. This prevents wide/flat models from ever transitioning tohorizontal-fly,horizontal-fall, orsittingcolliders. The guard must also accept box shapes as valid starting states.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/animate.js` around lines 18 - 29, The guard in updateCapsuleShapeForAnimation prevents transitions when the current collider is a PhysicsShapeBox created by createShapeFromBoundingBox; update the early-return condition to accept both flock.BABYLON.PhysicsShapeCapsule and flock.BABYLON.PhysicsShapeBox as valid starting types. Specifically, in updateCapsuleShapeForAnimation check physicsMesh.physics.shape instanceof flock.BABYLON.PhysicsShapeCapsule || physicsMesh.physics.shape instanceof flock.BABYLON.PhysicsShapeBox (or equivalent) so box-shaped colliders can be swapped into the horizontal/sitting capsule paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/mesh.js`:
- Around line 28-39: Compute the adjustedHeight (using shrinkAmount) before
deciding the fallback and use that adjustedHeight when checking against 2 *
radius so you don't produce a negative cylinderHeight; specifically, move or add
adjustedHeight = Math.max(0, height - shrinkAmount) above the conditional and
change the test from if (height < 2 * radius) to if (adjustedHeight < 2 *
radius) so the fallback box path and the cylinderHeight calculation
(cylinderHeight = adjustedHeight - 2 * radius) use the same threshold.
In `@api/physics.js`:
- Around line 58-62: The created collider's actual type isn't being saved: in
createPhysicsShape when you call flock.createShapeFromBoundingBox(mesh,
flock.scene) (and similarly where setupMesh seeds bodies), capture the returned
shape object, call getShapeTypeFromPhysics(returnedShape) to derive the real
type, and assign that result to physicsShapeType (instead of the requested
"CAPSULE" literal). Update any mesh/body seeding code that previously set
physicsShapeType implicitly to instead inspect the created shape and persist the
derived type so boxes created by createShapeFromBoundingBox are stored
correctly.
---
Outside diff comments:
In `@api/animate.js`:
- Around line 18-29: The guard in updateCapsuleShapeForAnimation prevents
transitions when the current collider is a PhysicsShapeBox created by
createShapeFromBoundingBox; update the early-return condition to accept both
flock.BABYLON.PhysicsShapeCapsule and flock.BABYLON.PhysicsShapeBox as valid
starting types. Specifically, in updateCapsuleShapeForAnimation check
physicsMesh.physics.shape instanceof flock.BABYLON.PhysicsShapeCapsule ||
physicsMesh.physics.shape instanceof flock.BABYLON.PhysicsShapeBox (or
equivalent) so box-shaped colliders can be swapped into the horizontal/sitting
capsule paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d1c06f2a-4042-48cb-a8fc-17cb8c08771e
📒 Files selected for processing (4)
api/animate.jsapi/mesh.jsapi/physics.jsui/blockmesh.js
| if (height < 2 * radius) { | ||
| return new flock.BABYLON.PhysicsShapeBox( | ||
| localCenter, | ||
| flock.BABYLON.Quaternion.Identity(), | ||
| new flock.BABYLON.Vector3(width, height, depth), | ||
| scene, | ||
| ); | ||
| } | ||
|
|
||
| const shrinkAmount = 0.01; | ||
| const adjustedHeight = Math.max(0, height - shrinkAmount); | ||
| const cylinderHeight = adjustedHeight - 2 * radius; |
There was a problem hiding this comment.
Use the same threshold for the fallback and the shrink step.
height < 2 * radius is checked before subtracting shrinkAmount, so meshes at height === 2 * radius — or within 0.01 above it — still go down the capsule path and end up with a negative cylinderHeight. That reintroduces the degenerate geometry this change is trying to avoid.
Suggested fix
- if (height < 2 * radius) {
+ const shrinkAmount = 0.01;
+ const adjustedHeight = Math.max(0, height - shrinkAmount);
+ if (adjustedHeight <= 2 * radius) {
return new flock.BABYLON.PhysicsShapeBox(
localCenter,
flock.BABYLON.Quaternion.Identity(),
new flock.BABYLON.Vector3(width, height, depth),
scene,
);
}
- const shrinkAmount = 0.01;
- const adjustedHeight = Math.max(0, height - shrinkAmount);
- const cylinderHeight = adjustedHeight - 2 * radius;
+ const cylinderHeight = Math.max(0, adjustedHeight - 2 * radius);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (height < 2 * radius) { | |
| return new flock.BABYLON.PhysicsShapeBox( | |
| localCenter, | |
| flock.BABYLON.Quaternion.Identity(), | |
| new flock.BABYLON.Vector3(width, height, depth), | |
| scene, | |
| ); | |
| } | |
| const shrinkAmount = 0.01; | |
| const adjustedHeight = Math.max(0, height - shrinkAmount); | |
| const cylinderHeight = adjustedHeight - 2 * radius; | |
| const shrinkAmount = 0.01; | |
| const adjustedHeight = Math.max(0, height - shrinkAmount); | |
| if (adjustedHeight <= 2 * radius) { | |
| return new flock.BABYLON.PhysicsShapeBox( | |
| localCenter, | |
| flock.BABYLON.Quaternion.Identity(), | |
| new flock.BABYLON.Vector3(width, height, depth), | |
| scene, | |
| ); | |
| } | |
| const cylinderHeight = Math.max(0, adjustedHeight - 2 * radius); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/mesh.js` around lines 28 - 39, Compute the adjustedHeight (using
shrinkAmount) before deciding the fallback and use that adjustedHeight when
checking against 2 * radius so you don't produce a negative cylinderHeight;
specifically, move or add adjustedHeight = Math.max(0, height - shrinkAmount)
above the conditional and change the test from if (height < 2 * radius) to if
(adjustedHeight < 2 * radius) so the fallback box path and the cylinderHeight
calculation (cylinderHeight = adjustedHeight - 2 * radius) use the same
threshold.
| const createPhysicsShape = (mesh, shapeType) => { | ||
| if (shapeType === "CAPSULE") { | ||
| mesh.computeWorldMatrix(true); | ||
| return flock.createCapsuleFromBoundingBox(mesh, flock.scene); | ||
| return flock.createShapeFromBoundingBox(mesh, flock.scene); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 'getShapeTypeFromPhysics|PhysicsShapeBox|createShapeFromBoundingBox|physicsShapeType|physicsCache|resolvedShapeType|setupMesh' api/physics.js api/mesh.jsRepository: flipcomputing/flock
Length of output: 6511
🏁 Script executed:
sed -n '3,17p' api/physics.jsRepository: flipcomputing/flock
Length of output: 448
Persist the actual collider type returned by createShapeFromBoundingBox().
getShapeTypeFromPhysics() only recognizes PhysicsShapeCapsule and PhysicsShapeMesh (returns null for any other type). However, createShapeFromBoundingBox() can return PhysicsShapeBox based on dimensions (api/mesh.js:29). When called at lines 61 and 328–331, the actual returned shape type is never captured—only the requested "CAPSULE" intent is stored in physicsShapeType. This causes boxes created here to be misidentified as "MESH" on restore (line 276 defaults to "MESH" when type is missing), losing the intended collider. Similarly, bodies seeded in api/mesh.js:805 via setupMesh don't persist their shape type at all.
Derive the real shape type from the returned object immediately after creation and cache it instead of the requested type.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/physics.js` around lines 58 - 62, The created collider's actual type
isn't being saved: in createPhysicsShape when you call
flock.createShapeFromBoundingBox(mesh, flock.scene) (and similarly where
setupMesh seeds bodies), capture the returned shape object, call
getShapeTypeFromPhysics(returnedShape) to derive the real type, and assign that
result to physicsShapeType (instead of the requested "CAPSULE" literal). Update
any mesh/body seeding code that previously set physicsShapeType implicitly to
instead inspect the created shape and persist the derived type so boxes created
by createShapeFromBoundingBox are stored correctly.
2 participants
Summary
Renamed
createCapsuleFromBoundingBoxtocreateShapeFromBoundingBoxand enhanced it to dynamically select between box and capsule shapes based on mesh dimensions. This improves physics shape accuracy for meshes where height is less than twice the radius.Key Changes
flockMesh.createCapsuleFromBoundingBox()tocreateShapeFromBoundingBox()across all call sites (mesh.js, physics.js, animate.js, blockmesh.js)PhysicsShapeBoxwhen mesh height is less than 2× radius, preventing degenerate capsule shapesImplementation Details
The function now performs a dimension check early in the process:
height < 2 * radius: creates a box shape with dimensions matching the bounding boxThis prevents invalid capsule geometries and ensures appropriate physics behavior for all mesh proportions.
https://claude.ai/code/session_01VZjZFU3AZyeseyRxtLuEwW
Summary by CodeRabbit