Skip to content

Replace constructor name checks with instanceof for physics shapes#466

Merged
tracygardner merged 2 commits intomainfrom
claude/fix-physics-scale-update-hMQP2
Mar 24, 2026
Merged

Replace constructor name checks with instanceof for physics shapes#466
tracygardner merged 2 commits intomainfrom
claude/fix-physics-scale-update-hMQP2

Conversation

@tracygardner
Copy link
Contributor

@tracygardner tracygardner commented Mar 24, 2026

Summary

Refactored physics shape type detection to use instanceof checks instead of relying on constructor name strings. This improves type safety and reliability across the physics, animation, and mesh modules.

Key Changes

  • physics.js: Replaced switch statement based on constructor.name with explicit instanceof checks against flock.BABYLON physics shape classes
  • physics.js: Enhanced shape detection logic to fall back to parent.metadata?.physicsShapeType when shape type cannot be determined
  • animate.js: Updated capsule shape validation to use instanceof instead of constructor name comparison
  • mesh.js: Updated capsule shape detection to use instanceof instead of constructor name comparison

Implementation Details

  • The new approach uses instanceof checks which are more reliable than string-based constructor name matching, especially across module boundaries or after minification
  • Added null-safety checks (flock?.BABYLON?.PhysicsShapeCapsule) before performing instanceof operations
  • Consolidated shape detection logic to be consistent across all three modules
  • Maintains backward compatibility by falling back to metadata-based shape type detection when needed

https://claude.ai/code/session_017rYZBuYo2r36GLPJirf6ir

Summary by CodeRabbit

  • Refactor
    • Improved runtime detection and handling of physics shapes for capsules and meshes, replacing brittle name-based checks with reliable type checks.
    • Adjusted shape-detection and recreation logic to avoid incorrect geometry updates and to early-exit when shapes cannot be safely updated, improving stability of physics-driven mesh interactions.

…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
@coderabbitai
Copy link

coderabbitai bot commented Mar 24, 2026

📝 Walkthrough

Walkthrough

Replaces string-based constructor name checks with runtime instanceof checks for Babylon physics shapes across animation, mesh, and physics modules; adds explicit capsule/mesh branches and a metadata fallback in physics update logic.

Changes

Cohort / File(s) Summary
Animation guard
api/animate.js
Replaced shape.constructor.name string comparison with instanceof flock.BABYLON.PhysicsShapeCapsule (optional-chained) to decide early return in updateCapsuleShapeForAnimation.
Mesh capsule creation
api/mesh.js
createHorizontalCapsuleFromBoundingBox now uses instanceof flock.BABYLON.PhysicsShapeCapsule instead of comparing constructor.name to "_PhysicsShapeCapsule" to detect existing capsule shapes before extracting dimensions.
Physics detection & update
api/physics.js
Replaced constructor-name checks with instanceof for PhysicsShapeCapsule and PhysicsShapeMesh in getShapeTypeFromPhysics. updatePhysics gains explicit branches for capsule and mesh shapes that set detectedShapeType, recreate shapes via createPhysicsShape(...) with early returns on failure, and the generic fallback now uses `getShapeTypeFromPhysics(parent.physics)

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰
I hopped through code, nose in the fray,
Swapped flimsy names for types that stay.
Capsules and meshes now recognized true,
No more string-guessing—hip hop hooroo! 🎩

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: refactoring physics shape type checks from constructor.name string comparisons to instanceof checks across multiple files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/fix-physics-scale-update-hMQP2

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Mar 24, 2026

Deploying flockdev with  Cloudflare Pages  Cloudflare Pages

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

View logs

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
api/animate.js (1)

23-30: Consider adding null-safety for flock.BABYLON.PhysicsShapeCapsule.

The instanceof check lacks the null-safety guard used in getShapeTypeFromPhysics (flock?.BABYLON?.PhysicsShapeCapsule &&). If flock.BABYLON.PhysicsShapeCapsule is undefined (e.g., during early initialization or in certain build configurations), this would throw a TypeError.

♻️ 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 with getShapeTypeFromPhysics.

The instanceof checks on lines 169 and 173 access flock.BABYLON.PhysicsShapeCapsule and flock.BABYLON.PhysicsShapeMesh directly without the null-safety guards used in getShapeTypeFromPhysics (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, and PhysicsShapeCylinder (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

📥 Commits

Reviewing files that changed from the base of the PR and between d4e1d32 and 0983f38.

📒 Files selected for processing (3)
  • api/animate.js
  • api/mesh.js
  • api/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
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 0983f38 and ae28c0a.

📒 Files selected for processing (2)
  • api/animate.js
  • api/physics.js
✅ Files skipped from review due to trivial changes (1)
  • api/animate.js

@@ -1,15 +1,13 @@
let flock;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

@tracygardner tracygardner merged commit 7fcba7a into main Mar 24, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants