[code-simplifier] Simplify GeospatialCamera flyTo logic and improve consistency#17991
Conversation
- Extract duplicate flyTo target setup logic into private _setFlyToTargets() method - Fix inconsistent undefined check (use !== instead of !=) in updateFlyToDestination - Simplify zoom speed calculation by removing unnecessary intermediate variable - Improves DRY principle by eliminating 6 lines of duplicated code Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR simplifies the GeospatialCamera system by extracting a repeated code block into a private helper, fixing an inconsistent comparison operator, and inlining a single-use intermediate variable in the zoom speed calculation. It is part of a series of cleanups following the recent PR #17990 which introduced API improvements.
Changes:
- Extracted duplicated fly-to target setup logic from
updateFlyToDestination()andflyToAsync()into a new private_setFlyToTargets()helper method - Fixed
!= undefined(loose equality) to!== undefined(strict equality) in the pitch comparison - Simplified zoom speed calculation by inlining the
zoomTargetDistancevariable using a ternary instead of alet+ nullish coalescing pattern
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
packages/dev/core/src/Cameras/geospatialCamera.ts |
Introduces _setFlyToTargets() private helper, delegates from updateFlyToDestination() and flyToAsync(); fixes strict equality for targetPitch check |
packages/dev/core/src/Cameras/geospatialCameraMovement.ts |
Simplifies zoom speed multiplier calculation by replacing let + nullish coalescing with a const + ternary |
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/17991/merge/index.html#WGZLGJ#4600 Links to test your changes to core in the published versions of the Babylon tools (does not contain changes you made to the tools themselves): https://playground.babylonjs.com/?snapshot=refs/pull/17991/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/17991/merge#BCU1XR#0 If you made changes to the sandbox or playground in this PR, additional comments will be generated soon containing links to the dev versions of those tools. |
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
Code Simplification - 2026-02-28
This PR simplifies recently modified code in the GeospatialCamera system to improve clarity, consistency, and maintainability while preserving all functionality.
Files Simplified
packages/dev/core/src/Cameras/geospatialCamera.ts- Extracted duplicate flyTo logic, fixed comparison operatorpackages/dev/core/src/Cameras/geospatialCameraMovement.ts- Simplified zoom speed calculationImprovements Made
1. Reduced Duplication (DRY Principle)
updateFlyToDestination()andflyToAsync()_setFlyToTargets()2. Fixed Inconsistent Comparison Operator
!=while line 264 used!==for undefined checkstargetPitch != undefinedtotargetPitch !== undefined3. Simplified Zoom Speed Calculation
zoomTargetDistanceused only once with nullish coalescingChanges Based On
Recent changes from PR #17990 (merged 2026-02-28):
calculateUpVectorFromPointToRef)@experimentaltagsTesting
The simplifications preserve exact functionality:
packages/dev/core/test/unit/Cameras/geospatialCamera.test.tsReview Focus
Please verify:
_setFlyToTargets()method correctly handles all casesStatistics
Automated by Code Simplifier Agent - analyzing code from the last 24 hours