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 lookAt method for PositionComponent #1891

Merged
merged 33 commits into from
Sep 18, 2022
Merged

feat: Add lookAt method for PositionComponent #1891

merged 33 commits into from
Sep 18, 2022

Conversation

ufrshubham
Copy link
Member

@ufrshubham ufrshubham commented Sep 6, 2022

Description

This PR adds a new method called lookAt for PositionComponent. It is a convenience method which rotates the component to make it point towards/look at the given target position.

Additionally, this PR also adds a angleTo method which can be used to get the calculated angle for lookAt. It will be useful if someone want to smoothly rotate towards target using effects or manual lerping.

Checklist

  • The title of my PR starts with a [Conventional Commit] prefix (fix:, feat:, docs: etc).
  • I have read the [Contributor Guide] and followed the process outlined for submitting PRs.
  • 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.

Breaking Change

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

Related Issues

Fixes #1107

@spydon spydon requested review from st-pasha and a team September 6, 2022 06:56
@spydon
Copy link
Member

spydon commented Sep 6, 2022

@st-pasha didn't you make a function like this once? But for Radiance maybe? I think you had another name for it if I remember correctly.

@spydon spydon requested a review from a team September 6, 2022 07:01
@spydon
Copy link
Member

spydon commented Sep 6, 2022

Found it, #1107 should be done before, or together with this

@ufrshubham
Copy link
Member Author

Okay, now I am confused, or maybe my understanding of this new property is wrong😅. In any application dealing with transformations, angles are always w.r.t something. Just like how position is w.r.t to a origin. In most cases angle=0 implies a vector pointing up. Looking at how angle works in Flame as well, this seems to be the case. So following that logic, my understanding of angularOffset was that it is just to help orient the component in the desired direction.

For example, if a sprite is pointing downwards (as shown in the image), angle=0 implies the default orientation of the sprite (which in this case is, pointing down).
image

But with the help of the angularOffset, we are trying to allow users to offset the rotation by whatever angle they want. So if they want the angle=0 to mean that the sprite points up, they can just set the angularOffset to -pi/2 or pi/2. That is the reason why I applied the angularOffset to transform.angle in constructor, getter and setter.

I am not able to understand how this can be achieved without actually changing angle of the component.😅

@spydon
Copy link
Member

spydon commented Sep 6, 2022

The angle will be in regards to the angulaOffset, but you don't have to do canvas transformations to achieve that, because the sprite is already rotated in that direction.

So if you have a sprite looking to the right and set angularOffset = pi/2 and angle = pi the sprite will look to the left, without having to add angularOffset to the transform. Does it make sense? :)

@ufrshubham
Copy link
Member Author

Okay, I think I get what you mean conceptually. Will have to play around with the code to wrap my head around it. I'll make modifications to this PR accordingly. I'm marking this as draft till I get it working.

@ufrshubham ufrshubham marked this pull request as draft September 6, 2022 14:16
@ufrshubham ufrshubham requested review from spydon and removed request for st-pasha September 10, 2022 07:44
@ufrshubham ufrshubham marked this pull request as ready for review September 10, 2022 07:50
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.

Almost there! Just two comments

packages/flame/lib/src/components/position_component.dart Outdated Show resolved Hide resolved
packages/flame/lib/src/components/position_component.dart Outdated Show resolved Hide resolved
@spydon spydon requested review from st-pasha and a team September 10, 2022 20:58
- Adds `angleTo` method which returns the calculated angle
- Refactors `lookAt` to use `angleTo`
- Improves doc comments
@spydon
Copy link
Member

spydon commented Sep 12, 2022

It would be good with some docs for this and a tiny example in the examples directory (or an embedded example in the docs).

@ufrshubham
Copy link
Member Author

It would be good with some docs for this and a tiny example in the examples directory (or an embedded example in the docs).

Here we go

LookAtExamples.mp4

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.

Just some tiny things left!

doc/flame/components.md Outdated Show resolved Hide resolved
doc/flame/components.md Outdated Show resolved Hide resolved
doc/flame/components.md Outdated Show resolved Hide resolved
doc/flame/components.md Show resolved Hide resolved
examples/lib/stories/components/look_at_example.dart Outdated Show resolved Hide resolved
@spydon spydon requested review from st-pasha and a team September 13, 2022 11:20
@spydon spydon requested a review from a team September 13, 2022 12:47
@spydon spydon merged commit 720c356 into flame-engine:main Sep 18, 2022
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.

Add orientation property do PositionComponent
4 participants