Add player-specific capsule physics shape creation#598
Open
tracygardner wants to merge 12 commits intomainfrom
Open
Add player-specific capsule physics shape creation#598tracygardner wants to merge 12 commits intomainfrom
tracygardner wants to merge 12 commits intomainfrom
Conversation
Wide/flat meshes (e.g. airplane) produce a degenerate sphere via the existing createCapsuleFromBoundingBox because radius >= height/2. A separate createPlayerCapsuleFromBoundingBox caps radius at height/3, ensuring the cylinder section is always present and the capsule is narrow enough for slope traversal. moveForward now detects a missing physicsCapsule and automatically switches the mesh to PLAYER_CAPSULE shape on first use, via a one-time async call guarded by _playerCapsulePending. https://claude.ai/code/session_019iH8iKivgCCsF3m8MpYcsc
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a player-specific capsule generator (clamped radius) and persists player-capsule metadata; defers/async-initializes player capsule from movement when physics/capsule data is missing or degenerate; and exposes an injectable capsule factory plus a Changes
Sequence Diagram(s)sequenceDiagram
participant Movement as Movement
participant Physics as Physics Engine
participant Mesh as Mesh Factory
Movement->>Movement: moveForward(model)
Movement->>Movement: check model.physics & mesh.metadata.physicsCapsule
alt capsule missing or degenerate
Movement->>Physics: setPhysicsShape(model, "PLAYER_CAPSULE")
Physics->>Mesh: createPlayerCapsuleFromBoundingBox(mesh, scene)
Mesh->>Mesh: compute radius,height,baseY,localCenter (radius ≤ height/3)
Mesh-->>Physics: return PhysicsShapeCapsule
Physics-->>Movement: shape applied (async)
Movement->>Movement: return early this frame (retry next update)
else capsule valid
Movement->>Movement: enforce vertical constraint
Movement->>Physics: ground/step shapeCast (ignoredBodies: [model.physics])
Movement->>Movement: apply movement/step logic
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 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 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 51 minutes and 10 seconds.Comment |
Deploying flockxr with
|
| Latest commit: |
2aa0970
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://766b0d1b.flockxr.pages.dev |
| Branch Preview URL: | https://claude-fix-physics-capsule-y.flockxr.pages.dev |
The CAPSULE case in setPhysicsShape now defaults to createPlayerCapsuleFromBoundingBox, so explicit user calls like setPhysicsShape(player, "CAPSULE") also get the height/3 radius cap. Previously the airplane produced a degenerate sphere (radius > height/2) even when the user set the physics shape explicitly. Removes the PLAYER_CAPSULE internal type — CAPSULE now always uses the correct formula since setPhysicsShape capsules are exclusively for player characters. https://claude.ai/code/session_019iH8iKivgCCsF3m8MpYcsc
The early return on !model.physics prevented the auto-capsule path from ever running when setPhysicsShape had not been called explicitly. GLB root nodes often fail MESH shape creation, leaving model.physics null. Include !model.physics in the capsule condition so the auto-switch fires regardless. ensureVerticalConstraint moved to after the capsule check since it requires physics to exist. https://claude.ai/code/session_019iH8iKivgCCsF3m8MpYcsc
setupMesh calls createCapsuleFromBoundingBox for every loaded model, which degenerates to a sphere for wide/flat meshes like the airplane. Switch to createPlayerCapsuleFromBoundingBox (radius capped at height/3) so the default capsule is always a proper tall shape suitable for movement. https://claude.ai/code/session_019iH8iKivgCCsF3m8MpYcsc
Revert setupMesh and applyCapsuleToRoot defaults to createCapsuleFromBoundingBox so non-player objects keep the standard capsule shape. moveForward now detects a degenerate capsule (height <= 2*radius, no cylinder section) as well as missing physics/capsule, and upgrades to PLAYER_CAPSULE on first call only. PLAYER_CAPSULE is an internal shape type in setPhysicsShape that uses createPlayerCapsuleFromBoundingBox (radius capped at height/3). https://claude.ai/code/session_019iH8iKivgCCsF3m8MpYcsc
restitution: 0.5 caused micro-bounces at slope crests — the capsule briefly went airborne, cutting forward control (airControlFactor=0), producing the stuck-at-top-of-slope behaviour. Player capsule now uses restitution: 0 on the body and friction: 0 on the shape material so it slides cleanly over terrain polygons without bouncing or gripping edges. https://claude.ai/code/session_019iH8iKivgCCsF3m8MpYcsc
…on change Ground check and step probe shape casts had ignoredBodies: [] so they were colliding with the player's own physics body. For wide/short player capsules (box, airplane) the self-hit normal comes back horizontal → grounded=false → forward control cut → bumping/stalling. Character capsules happened to produce a near-vertical self-hit normal so the bug was masked. Fix: pass [model.physics] as ignoredBodies in both the ground check and step probe casts so only terrain and other objects are detected. Also reverts the restitution/friction change from the previous commit — it was wrong; character meshes work fine with the existing restitution. https://claude.ai/code/session_019iH8iKivgCCsF3m8MpYcsc
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/movement.js (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAddress Prettier formatting issue flagged by CI.
The pipeline reports a Prettier check failure. Run the formatter to resolve before merge:
npx prettier --write api/movement.js🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/movement.js` at line 1, CI reports a Prettier formatting failure in api/movement.js (the file declaring the top-level variable let flock;); fix it by running the project formatter (e.g., npx prettier --write api/movement.js) or applying the same Prettier rules in your editor to reformat the file so it passes the Prettier check, then re-run the Prettier/CI check and commit the changed formatting.
🤖 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/movement.js`:
- Around line 18-23: The code sets model._playerCapsulePending=true and calls
flock.setPhysicsShape(modelName, "PLAYER_CAPSULE") but never clears the flag if
the promise rejects, which can permanently block re-initialization; change the
promise handling on flock.setPhysicsShape(...) in the block that sets
model._playerCapsulePending so the flag is cleared in a finally handler (or via
.then(...).catch(...).finally(...)) regardless of success or failure, ensuring
delete model._playerCapsulePending always runs and allowing retries on transient
errors.
---
Outside diff comments:
In `@api/movement.js`:
- Line 1: CI reports a Prettier formatting failure in api/movement.js (the file
declaring the top-level variable let flock;); fix it by running the project
formatter (e.g., npx prettier --write api/movement.js) or applying the same
Prettier rules in your editor to reformat the file so it passes the Prettier
check, then re-run the Prettier/CI check and commit the changed formatting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+18
to
+23
| if (!model._playerCapsulePending) { | ||
| model._playerCapsulePending = true; | ||
| flock.setPhysicsShape(modelName, "PLAYER_CAPSULE").then(() => { | ||
| delete model._playerCapsulePending; | ||
| }); | ||
| } |
There was a problem hiding this comment.
Missing error handling: rejected promise leaves player unable to move.
If setPhysicsShape rejects, the .then() callback never executes, _playerCapsulePending remains true, and subsequent frames skip re-initialization. The player would be permanently stuck.
Use .finally() to ensure the flag is cleared regardless of outcome, allowing retry on transient failures.
Proposed fix
if (!model._playerCapsulePending) {
model._playerCapsulePending = true;
- flock.setPhysicsShape(modelName, "PLAYER_CAPSULE").then(() => {
- delete model._playerCapsulePending;
- });
+ flock.setPhysicsShape(modelName, "PLAYER_CAPSULE").finally(() => {
+ delete model._playerCapsulePending;
+ });
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/movement.js` around lines 18 - 23, The code sets
model._playerCapsulePending=true and calls flock.setPhysicsShape(modelName,
"PLAYER_CAPSULE") but never clears the flag if the promise rejects, which can
permanently block re-initialization; change the promise handling on
flock.setPhysicsShape(...) in the block that sets model._playerCapsulePending so
the flag is cleared in a finally handler (or via
.then(...).catch(...).finally(...)) regardless of success or failure, ensuring
delete model._playerCapsulePending always runs and allowing retries on transient
errors.
The height<=2*radius check had edge cases where the airplane's bounding box orientation meant the default capsule wasn't detected as degenerate, leaving it as a sphere even though it should be switched. createPlayerCapsuleFromBoundingBox now sets cap.isPlayerCapsule=true. createCapsuleFromBoundingBox sets it false. moveForward switches any mesh whose cap.isPlayerCapsule is not true — reliably catches all cases regardless of capsule proportions. https://claude.ai/code/session_019iH8iKivgCCsF3m8MpYcsc
The horizontal step probe was boosting the capsule upward whenever it hit any surface ahead, including sloped terrain faces when descending. This launched the capsule airborne (killing forward control via airControlFactor=0) and caused it to stall at slope crests. Fix: check the hit surface normal before allowing a step-up boost. Downward slope faces have a normal with upDot > 0.5 (pointing mostly upward), so they are skipped. Only near-vertical surfaces (upDot < 0.5) — actual ledges — trigger the boost. https://claude.ai/code/session_019iH8iKivgCCsF3m8MpYcsc
The step-up probe started at model.position + 0.05. For humanoid characters localCenter.y ≈ capsuleHeight/2 so model.position.y sits at ground level — the probe fires just above ground as intended. But for wide/flat meshes (airplane) localCenter.y ≈ 0 so model.position.y is at the capsule centre (~0.3m above ground), putting the probe at 0.35m — high enough to hit the face of a downward slope and trigger a false step-up boost. Fix: compute the actual ground Y from model.position.y adjusted by (cap.height/2 - cap.localCenter.y), then anchor the probe 0.05m above that. The probe now fires near ground level for all capsule shapes, so a downward slope (whose face is at or below ground) is no longer hit. https://claude.ai/code/session_019iH8iKivgCCsF3m8MpYcsc
Short capsules (height <= stepHeight * 3, e.g. airplane at ~0.6m) don't benefit from the step-up boost — their hemispherical base naturally rides over small bumps. The probe was firing on slope-crest transition faces and micro-bumps, repeatedly boosting the capsule airborne and stalling it at the top of downward slopes. Tall capsules (characters at ~1.8m) still use the probe because they need help stepping over kerbs and low ledges. Also increased coyoteTimeMs 120→250ms so briefly airborne states on slope transitions don't immediately cut forward control. https://claude.ai/code/session_019iH8iKivgCCsF3m8MpYcsc
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 16-27: The capsule dimension math can still yield zero/near-zero
volumes for flat or thin meshes; clamp radius with a small epsilon (e.g., const
EPS = 1e-3) after computing radius and then ensure adjustedHeight is at least 2
* radius + EPS before computing cylinderHeight. Concretely: after computing
radius in the mesh sizing block, set radius = Math.max(radius, EPS); compute
adjustedHeight = Math.max(0, height - shrinkAmount) and then enforce
adjustedHeight = Math.max(adjustedHeight, 2 * radius + EPS) before computing
cylinderHeight = adjustedHeight - 2 * radius (and still Math.max(0, ...) if
desired). Update the symbols radius, adjustedHeight, shrinkAmount, and
cylinderHeight accordingly.
🪄 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: 086e6a51-c661-4241-a28e-00e8a1269dec
📒 Files selected for processing (2)
api/mesh.jsapi/movement.js
🚧 Files skipped from review as they are similar to previous changes (1)
- api/movement.js
Comment on lines
+16
to
+27
| const height = (localMax.y - localMin.y) * Math.abs(mesh.scaling.y); | ||
| const width = (localMax.x - localMin.x) * Math.abs(mesh.scaling.x); | ||
| const depth = (localMax.z - localMin.z) * Math.abs(mesh.scaling.z); | ||
|
|
||
| // Cap radius so the cylinder section is always at least height/3 tall, | ||
| // preventing degenerate sphere shapes for wide/flat meshes like aircraft. | ||
| const radius = Math.min(Math.min(width, depth) / 2, height / 3); | ||
|
|
||
| const shrinkAmount = 0.01; | ||
| const adjustedHeight = Math.max(0, height - shrinkAmount); | ||
| const cylinderHeight = Math.max(0, adjustedHeight - 2 * radius); | ||
|
|
There was a problem hiding this comment.
Clamp minimum capsule dimensions to avoid zero-volume player shapes
Lines 22 and 25-27 can still produce a zero/near-zero capsule for perfectly flat or ultra-thin meshes, which weakens/loses collisions on the exact path meant to handle those models. Add an epsilon floor for radius and ensure adjustedHeight >= 2 * radius + epsilon.
Proposed fix
+ const EPSILON = 1e-4;
- const height = (localMax.y - localMin.y) * Math.abs(mesh.scaling.y);
- const width = (localMax.x - localMin.x) * Math.abs(mesh.scaling.x);
- const depth = (localMax.z - localMin.z) * Math.abs(mesh.scaling.z);
+ const height = Math.max(
+ (localMax.y - localMin.y) * Math.abs(mesh.scaling.y),
+ EPSILON,
+ );
+ const width = Math.max(
+ (localMax.x - localMin.x) * Math.abs(mesh.scaling.x),
+ EPSILON,
+ );
+ const depth = Math.max(
+ (localMax.z - localMin.z) * Math.abs(mesh.scaling.z),
+ EPSILON,
+ );
- const radius = Math.min(Math.min(width, depth) / 2, height / 3);
+ const radius = Math.max(
+ EPSILON,
+ Math.min(Math.min(width, depth) / 2, height / 3),
+ );
const shrinkAmount = 0.01;
- const adjustedHeight = Math.max(0, height - shrinkAmount);
+ const adjustedHeight = Math.max(2 * radius + EPSILON, height - shrinkAmount);
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.
Suggested change
| const height = (localMax.y - localMin.y) * Math.abs(mesh.scaling.y); | |
| const width = (localMax.x - localMin.x) * Math.abs(mesh.scaling.x); | |
| const depth = (localMax.z - localMin.z) * Math.abs(mesh.scaling.z); | |
| // Cap radius so the cylinder section is always at least height/3 tall, | |
| // preventing degenerate sphere shapes for wide/flat meshes like aircraft. | |
| const radius = Math.min(Math.min(width, depth) / 2, height / 3); | |
| const shrinkAmount = 0.01; | |
| const adjustedHeight = Math.max(0, height - shrinkAmount); | |
| const cylinderHeight = Math.max(0, adjustedHeight - 2 * radius); | |
| const EPSILON = 1e-4; | |
| const height = Math.max( | |
| (localMax.y - localMin.y) * Math.abs(mesh.scaling.y), | |
| EPSILON, | |
| ); | |
| const width = Math.max( | |
| (localMax.x - localMin.x) * Math.abs(mesh.scaling.x), | |
| EPSILON, | |
| ); | |
| const depth = Math.max( | |
| (localMax.z - localMin.z) * Math.abs(mesh.scaling.z), | |
| EPSILON, | |
| ); | |
| // Cap radius so the cylinder section is always at least height/3 tall, | |
| // preventing degenerate sphere shapes for wide/flat meshes like aircraft. | |
| const radius = Math.max( | |
| EPSILON, | |
| Math.min(Math.min(width, depth) / 2, height / 3), | |
| ); | |
| const shrinkAmount = 0.01; | |
| const adjustedHeight = Math.max(2 * radius + EPSILON, height - shrinkAmount); | |
| 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 16 - 27, The capsule dimension math can still yield
zero/near-zero volumes for flat or thin meshes; clamp radius with a small
epsilon (e.g., const EPS = 1e-3) after computing radius and then ensure
adjustedHeight is at least 2 * radius + EPS before computing cylinderHeight.
Concretely: after computing radius in the mesh sizing block, set radius =
Math.max(radius, EPS); compute adjustedHeight = Math.max(0, height -
shrinkAmount) and then enforce adjustedHeight = Math.max(adjustedHeight, 2 *
radius + EPS) before computing cylinderHeight = adjustedHeight - 2 * radius (and
still Math.max(0, ...) if desired). Update the symbols radius, adjustedHeight,
shrinkAmount, and cylinderHeight accordingly.
When grounded, Havok applies small upward correction velocities to resolve capsule-floor penetration. These were being preserved by clampedVertical and causing visible bouncing, especially for short capsules (e.g. airplane) where the bounce is large relative to capsule height. Characters were unaffected because the same absolute bounce is negligible at their scale. Fix: zero the vertical velocity when grounded and the upward speed is below 1.0 m/s (typical solver correction range). Velocities above 1.0 m/s are preserved as deliberate upward movement (jumps, boosts). https://claude.ai/code/session_019iH8iKivgCCsF3m8MpYcsc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces a new
createPlayerCapsuleFromBoundingBoxmethod to handle physics capsule generation specifically for player characters, with improved constraints to prevent degenerate shapes for wide or flat meshes.Key Changes
createPlayerCapsuleFromBoundingBox): Creates physics capsules with a capped radius that ensures the cylindrical section is always at least 1/3 of the total height, preventing issues with aircraft-like or flat meshes"PLAYER_CAPSULE"as a new physics shape type insetPhysicsShape, allowing models to opt into the player-specific capsule behaviorapplyCapsuleToRootaccept a configurable creation function parameter, enabling reuse of the capsule application logic for both standard and player-specific capsulesImplementation Details
Math.min(Math.min(width, depth) / 2, height / 3)to maintain reasonable proportionshttps://claude.ai/code/session_019iH8iKivgCCsF3m8MpYcsc
Summary by CodeRabbit
New Features
Bug Fixes