Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: Add optional world input to CameraComponent.canSee #2616

Merged
merged 10 commits into from Jul 21, 2023

Conversation

ufrshubham
Copy link
Collaborator

@ufrshubham ufrshubham commented Jul 21, 2023

Description

CameraComponent.canSee wasn't performing any kind of sanity checks on the given components world or mounted-ness. This PR adds these checks to correctly return false if the component is not mounted or if the optional world is not the same as camera's current target world.

Checklist

  • I have followed the [Contributor Guide] when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • [-] I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

Related Issues

NA

Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Hmm, I wonder if it is worth looking up the component's world, since this method is used for culling it needs to be as efficient as possible. Maybe just allow for an optional world to be sent in that the component belongs to? Or just write in the dartdoc that the user should check that it belongs to the camera component's world first, it's probably a very niche use-case when that check needs to be performed anyways.

The mounted check is good though!

@ufrshubham ufrshubham changed the title fix: Check component's world in CameraComponent.canSee feat: Add optional world input to CameraComponent.canSee Jul 21, 2023
@ufrshubham
Copy link
Collaborator Author

Maybe just allow for an optional world to be sent in that the component belongs to?

Decided to go with this.

The mounted check is good though!

Also added a .isMounted check for the world.

Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Good stuff!

packages/flame/lib/src/camera/camera_component.dart Outdated Show resolved Hide resolved
Co-authored-by: Lukas Klingsbo <me@lukas.fyi>
@spydon spydon merged commit 1cad0b2 into main Jul 21, 2023
6 checks passed
@spydon spydon deleted the devkage/can-see-world-checks branch July 21, 2023 21:12
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.

None yet

2 participants