Replace constructor name checks with instanceof for physics shapes#466
Replace constructor name checks with instanceof for physics shapes#466tracygardner merged 2 commits intomainfrom
Conversation
…ection The physics shape update (updatePhysics) was silently failing on the GitHub Pages production build because the Babylon.js classes are minified: constructor.name is mangled (e.g. "a") so the switch on "_PhysicsShapeCapsule" and "_PhysicsShapeMesh" never matched, causing updatePhysics to return early and the physics shape to stay at its original size after scaling. - Replace getShapeTypeFromPhysics to use instanceof checks instead of constructor.name, which works correctly in both dev and minified builds - Add explicit instanceof branches for PhysicsShapeCapsule and PhysicsShapeMesh in updatePhysics, bypassing the string-based detection - Add metadata fallback in the remaining else branch as extra defence - Fix the same constructor.name pattern in animate.js and mesh.js https://claude.ai/code/session_017rYZBuYo2r36GLPJirf6ir
📝 WalkthroughWalkthroughReplaces string-based constructor name checks with runtime Changes
Sequence Diagram(s)sequenceDiagram
participant Updater as updatePhysics
participant Parent as Parent Node
participant Factory as createPhysicsShape
participant Physics as Babylon Physics Shape
Parent->>Updater: call updatePhysics(parent)
Updater->>Parent: inspect parent.physics.shape
alt shape instanceof PhysicsShapeCapsule
Updater->>Updater: detectedShapeType = "CAPSULE"
Updater->>Factory: createPhysicsShape("CAPSULE", ...)
Factory-->>Updater: newShape / null
alt newShape created
Updater->>Parent: assign new shape, return
else failed
Updater-->>Parent: return (early)
end
else shape instanceof PhysicsShapeMesh
Updater->>Updater: detectedShapeType = "MESH"
Updater->>Factory: createPhysicsShape("MESH", ...)
Factory-->>Updater: newShape / null
alt newShape created
Updater->>Parent: assign new shape, return
else failed
Updater-->>Parent: return (early)
end
else
Updater->>Updater: detectedShapeType = getShapeTypeFromPhysics(...) || parent.metadata?.physicsShapeType
Updater->>Factory: createPhysicsShape(detectedShapeType, ...)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Deploying flockdev with
|
| Latest commit: |
ae28c0a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d8cf8542.flockdev.pages.dev |
| Branch Preview URL: | https://claude-fix-physics-scale-upd.flockdev.pages.dev |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
api/animate.js (1)
23-30: Consider adding null-safety forflock.BABYLON.PhysicsShapeCapsule.The
instanceofcheck lacks the null-safety guard used ingetShapeTypeFromPhysics(flock?.BABYLON?.PhysicsShapeCapsule &&). Ifflock.BABYLON.PhysicsShapeCapsuleis undefined (e.g., during early initialization or in certain build configurations), this would throw aTypeError.♻️ Optional: Add null-safety guard for consistency
if ( !physicsMesh || !physicsMesh.physics || !physicsMesh.physics.shape || - !(physicsMesh.physics.shape instanceof flock.BABYLON.PhysicsShapeCapsule) + !(flock?.BABYLON?.PhysicsShapeCapsule && physicsMesh.physics.shape instanceof flock.BABYLON.PhysicsShapeCapsule) ) { return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/animate.js` around lines 23 - 30, The instanceof check can throw if flock.BABYLON.PhysicsShapeCapsule is undefined; update the guard in the early-return block to use a null-safe check similar to getShapeTypeFromPhysics (e.g., ensure flock?.BABYLON?.PhysicsShapeCapsule exists before using instanceof) so that the condition reads consistently and avoids a TypeError when PhysicsShapeCapsule is missing; locate the check around physicsMesh and replace the raw flock.BABYLON.PhysicsShapeCapsule reference with the null-safe form.api/physics.js (1)
169-184: Consider adding null-safety guards for consistency withgetShapeTypeFromPhysics.The
instanceofchecks on lines 169 and 173 accessflock.BABYLON.PhysicsShapeCapsuleandflock.BABYLON.PhysicsShapeMeshdirectly without the null-safety guards used ingetShapeTypeFromPhysics(lines 6, 8). While this code path likely executes after Babylon is initialized, adding guards would improve defensive consistency.Note: The existing checks for
PhysicsShapeBox,PhysicsShapeSphere, andPhysicsShapeCylinder(lines 146, 154, 161) also lack guards, so this is a pre-existing pattern—addressing it could be deferred.♻️ Optional: Add null-safety guards for consistency
- } else if (physicsShape instanceof flock.BABYLON.PhysicsShapeCapsule) { + } else if (flock?.BABYLON?.PhysicsShapeCapsule && physicsShape instanceof flock.BABYLON.PhysicsShapeCapsule) { detectedShapeType = "CAPSULE"; newShape = createPhysicsShape(mesh, "CAPSULE"); if (!newShape) return; - } else if (physicsShape instanceof flock.BABYLON.PhysicsShapeMesh) { + } else if (flock?.BABYLON?.PhysicsShapeMesh && physicsShape instanceof flock.BABYLON.PhysicsShapeMesh) { detectedShapeType = "MESH"; newShape = createPhysicsShape(mesh, "MESH"); if (!newShape) return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/physics.js` around lines 169 - 184, The instanceof checks for flock.BABYLON.PhysicsShapeCapsule and PhysicsShapeMesh should be guarded against a missing Babylon namespace like getShapeTypeFromPhysics does; update the branches in the function (the checks that set detectedShapeType and call createPhysicsShape for "CAPSULE" and "MESH") to first ensure flock?.BABYLON?.PhysicsShapeCapsule / flock?.BABYLON?.PhysicsShapeMesh exist before using instanceof (e.g., replace direct instanceof with a short-circuit: flock?.BABYLON?.PhysicsShapeCapsule && physicsShape instanceof flock.BABYLON.PhysicsShapeCapsule), keep the same behavior of returning early if createPhysicsShape returns falsy, and mirror this pattern for both capsule and mesh checks so createPhysicsShape(mesh, "CAPSULE"/"MESH") only runs after the guarded instanceof succeeds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@api/animate.js`:
- Around line 23-30: The instanceof check can throw if
flock.BABYLON.PhysicsShapeCapsule is undefined; update the guard in the
early-return block to use a null-safe check similar to getShapeTypeFromPhysics
(e.g., ensure flock?.BABYLON?.PhysicsShapeCapsule exists before using
instanceof) so that the condition reads consistently and avoids a TypeError when
PhysicsShapeCapsule is missing; locate the check around physicsMesh and replace
the raw flock.BABYLON.PhysicsShapeCapsule reference with the null-safe form.
In `@api/physics.js`:
- Around line 169-184: The instanceof checks for
flock.BABYLON.PhysicsShapeCapsule and PhysicsShapeMesh should be guarded against
a missing Babylon namespace like getShapeTypeFromPhysics does; update the
branches in the function (the checks that set detectedShapeType and call
createPhysicsShape for "CAPSULE" and "MESH") to first ensure
flock?.BABYLON?.PhysicsShapeCapsule / flock?.BABYLON?.PhysicsShapeMesh exist
before using instanceof (e.g., replace direct instanceof with a short-circuit:
flock?.BABYLON?.PhysicsShapeCapsule && physicsShape instanceof
flock.BABYLON.PhysicsShapeCapsule), keep the same behavior of returning early if
createPhysicsShape returns falsy, and mirror this pattern for both capsule and
mesh checks so createPhysicsShape(mesh, "CAPSULE"/"MESH") only runs after the
guarded instanceof succeeds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 49167dea-c064-478b-9042-4380eb15fb53
📒 Files selected for processing (3)
api/animate.jsapi/mesh.jsapi/physics.js
Add flock?.BABYLON?.PhysicsShapeCapsule && guards before instanceof in animate.js and the new CAPSULE/MESH branches in physics.js, consistent with the pattern already used in getShapeTypeFromPhysics. https://claude.ai/code/session_017rYZBuYo2r36GLPJirf6ir
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/physics.js`:
- Line 1: The file has a Prettier formatting violation; run Prettier to reformat
api/physics.js (e.g., run prettier --write on that file) and commit the updated
formatting; ensure the top-level declaration (let flock;) and surrounding
whitespace/line endings are normalized by Prettier so the CI formatting check
passes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7f20926e-3634-46d4-9957-9d228e25beec
📒 Files selected for processing (2)
api/animate.jsapi/physics.js
✅ Files skipped from review due to trivial changes (1)
- api/animate.js
| @@ -1,15 +1,13 @@ | |||
| let flock; | |||
There was a problem hiding this comment.
Address the Prettier formatting warning.
The CI pipeline reports a Prettier formatting issue for this file. Run prettier --write api/physics.js to fix the formatting before merging.
🧰 Tools
🪛 GitHub Actions: Prettier
[warning] 1-1: Prettier reported formatting is not compliant (file listed under checks).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/physics.js` at line 1, The file has a Prettier formatting violation; run
Prettier to reformat api/physics.js (e.g., run prettier --write on that file)
and commit the updated formatting; ensure the top-level declaration (let flock;)
and surrounding whitespace/line endings are normalized by Prettier so the CI
formatting check passes.
Summary
Refactored physics shape type detection to use
instanceofchecks instead of relying on constructor name strings. This improves type safety and reliability across the physics, animation, and mesh modules.Key Changes
switchstatement based onconstructor.namewith explicitinstanceofchecks againstflock.BABYLONphysics shape classesparent.metadata?.physicsShapeTypewhen shape type cannot be determinedinstanceofinstead of constructor name comparisoninstanceofinstead of constructor name comparisonImplementation Details
instanceofchecks which are more reliable than string-based constructor name matching, especially across module boundaries or after minificationflock?.BABYLON?.PhysicsShapeCapsule) before performinginstanceofoperationshttps://claude.ai/code/session_017rYZBuYo2r36GLPJirf6ir
Summary by CodeRabbit