Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Setting camera target to most negative number to handle arbitrary large maps #993

Merged
merged 3 commits into from
Aug 12, 2024

Conversation

aaronchongth
Copy link
Member

@aaronchongth aaronchongth commented Aug 8, 2024

What's new

  • Tweak camera target to most negative number to better accommodate almost-orthogonal view for extremely large maps
  • refactored camera control to prevent creation of camera every time zoom changes

Self-checks

  • I have prototyped this new feature (if necessary) on Figma
  • I'm familiar with and follow this Typescript guideline
  • I added unit-tests for new components
  • I tried testing edge cases
  • I tested the behavior of the components that interact with the backend, with an e2e test

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
@aaronchongth aaronchongth requested a review from koonpeng August 8, 2024 06:00
Copy link
Collaborator

@koonpeng koonpeng left a comment

Choose a reason for hiding this comment

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

Not sure what this affects, but if we know the size of the map, then could we calculate this dynamically? If not then is there value to make this an app-config?

… creation

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
@aaronchongth
Copy link
Member Author

This changes the 3D position that the camera points towards, raising it to a large negative number implies that the camera will almost always be pointing down vertically, no matter where the camera is moved to in 3D space. This helps mimic an orthogonal view.

Although this value falls under the category of bigger-is-better, there's no harm in using the scene size to calculate it dynamically I guess.

I've also refactored the camera controls a little to reduce repeated construction of cameras and its controller

@aaronchongth aaronchongth requested a review from koonpeng August 12, 2024 04:27
Copy link
Collaborator

@koonpeng koonpeng left a comment

Choose a reason for hiding this comment

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

Sounds like there is no harm in having a smaller value? If so then let's revert the dynamic calculation change.

@aaronchongth aaronchongth requested a review from koonpeng August 12, 2024 07:55
@aaronchongth
Copy link
Member Author

As explained the purpose of these changes in DM

Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
@aaronchongth aaronchongth changed the title Setting camera target to 10km instead of 1 to handle much larger maps Setting camera target to most negative number to handle much larger maps Aug 12, 2024
@aaronchongth aaronchongth changed the title Setting camera target to most negative number to handle much larger maps Setting camera target to most negative number to handle arbitrary large maps Aug 12, 2024
@aaronchongth aaronchongth merged commit b9189e8 into main Aug 12, 2024
2 checks passed
@aaronchongth aaronchongth deleted the ac/tweak-camera-target branch August 12, 2024 08:43
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants