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

fix: Camera no longer "sticks" to boundary with BoundedPositionBehavior #2307

Merged
merged 10 commits into from
Feb 2, 2023

Conversation

st-pasha
Copy link
Contributor

Description

Previously, when using BoundedPositionBehavior, the camera would "stick" to the world boundary as soon as the target moves outside. With this PR, the camera will now try to keep as close to the target as possible, even if it moves outside of the world boundary. Thus, the camera will now behave as if the boundary became "slippery".

Also added method nearestPoint() in class Shape.

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

Closes #1905

if (_radius == 0) {
return _center;
}
return _tmpResult
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we don't want to at least return a new vector? Could be quite strange behavior if the user saves this away and then runs this function again for another circle and then both the vectors are the result of the last one, or well they are the same object. I know we have this pattern at some places, but not sure if it is good... At the very least we should warn the user in the dartdoc that they should clone the vector if they intend to save it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I have a note about it in the doc-comment.

We kinda don't have any consistency in how we approach this. There are methods that return new vectors, there are those that return existing vectors, and then those that sometimes do one thing and other times another.

It might be useful to do some benchmarking and figure out what's the actual cost of creating new vectors, and how would that compare to a "hand-crafted" Vector2 class which simply contain two doubles x, y as fields (thus, a bit like mutable Offset).

For now, though, we can go with this API, since it's easier to loosen up in the future if need be. Going in the other direction instead (i.e. we returned new objects, but then switched to reused objects) would be a breaking change.

@spydon spydon merged commit 914dc6a into main Feb 2, 2023
@spydon spydon deleted the ps.camera-boundary branch February 2, 2023 08:07
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.

Camera should be able to slide along the world boundary
2 participants