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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - can specify an anchor for a sprite #3463

Closed
wants to merge 4 commits into from
Closed

[Merged by Bors] - can specify an anchor for a sprite #3463

wants to merge 4 commits into from

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented Dec 28, 2021 •
edited
Loading

Objective

Solution

  • Add an enum for an anchor point for most common values, with a variant for a custom point
  • Defaults to Center to not change current behaviour

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Dec 28, 2021
@mockersf mockersf added A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible and removed S-Needs-Triage This issue needs to be labelled labels Dec 28, 2021
@alice-i-cecile
Copy link
Member

Can you add a doc alias for "pivot" or "pivot point" for this to improve discoverability?

MrGVSV
MrGVSV reviewed Jan 3, 2022
View reviewed changes
crates/bevy_sprite/src/sprite.rs
Comment on lines +37 to +38
/// Custom anchor point. Top left is `(-0.5, 0.5)`, center is `(0.0, 0.0)`. The value will
/// be scaled with the sprite size.
Copy link
Member

Choose a reason for hiding this comment

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

Just a thought, but should this be (0, 0) bottom left to (1, 1) top right? I think that might be clearer for those who want to select an anchor as an absolute position using Custom rather than as an offset from the center point (which could also maybe play into UI stuff as well).

Copy link
Member

Choose a reason for hiding this comment

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

I like that (0, 0) is the default behavior personally.

It doesn't matter much as long as it's clearly documented though.

Copy link

Choose a reason for hiding this comment

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

The default being centered is good in my opinion as well.

speakk
speakk approved these changes Jan 20, 2022
View reviewed changes
Copy link

@speakk speakk left a comment

Choose a reason for hiding this comment

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

This looks good to me, and the feature a part of pretty "basic" sprite workflow in my opinion, so I'm very happy to see it.

crates/bevy_sprite/src/sprite.rs
Comment on lines +37 to +38
/// Custom anchor point. Top left is `(-0.5, 0.5)`, center is `(0.0, 0.0)`. The value will
/// be scaled with the sprite size.
Copy link

Choose a reason for hiding this comment

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

The default being centered is good in my opinion as well.

crates/bevy_sprite/src/sprite.rs
Anchor::CenterRight => Vec2::new(0.5, 0.0),
Anchor::TopLeft => Vec2::new(-0.5, 0.5),
Anchor::TopCenter => Vec2::new(0.0, 0.5),
Anchor::TopRight => Vec2::new(0.5, 0.5),
Copy link

Choose a reason for hiding this comment

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

I really like that you provided these anchors for convenience.

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jan 20, 2022
@alice-i-cecile
Copy link
Member

bors try

@bors
Copy link
Contributor

bors bot commented Jan 20, 2022

try

Merge conflict.

@chrisburnor chrisburnor mentioned this pull request Jan 26, 2022
Merged
@cart
Copy link
Member

cart commented Apr 4, 2022

bors r+

bors bot pushed a commit that referenced this pull request Apr 4, 2022
@mockersf
can specify an anchor for a sprite (#3463)
8e864fd 
# Objective

- Fixes #1616, fixes #2225
- Let user specify an anchor for a sprite

## Solution

- Add an enum for an anchor point for most common values, with a variant for a custom point
- Defaults to Center to not change current behaviour


Co-authored-by: François <[email protected]>
@bors
Copy link
Contributor

bors bot commented Apr 4, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title can specify an anchor for a sprite [Merged by Bors] - can specify an anchor for a sprite Apr 4, 2022
@bors bors bot closed this Apr 4, 2022
aevyrie pushed a commit to aevyrie/bevy that referenced this pull request Jun 7, 2022
@mockersf @aevyrie
can specify an anchor for a sprite (bevyengine#3463)
264e25c 
# Objective

- Fixes bevyengine#1616, fixes bevyengine#2225
- Let user specify an anchor for a sprite

## Solution

- Add an enum for an anchor point for most common values, with a variant for a custom point
- Defaults to Center to not change current behaviour


Co-authored-by: François <[email protected]>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@mockersf @ItsDoot
can specify an anchor for a sprite (bevyengine#3463)
8011a08 
# Objective

- Fixes bevyengine#1616, fixes bevyengine#2225
- Let user specify an anchor for a sprite

## Solution

- Add an enum for an anchor point for most common values, with a variant for a custom point
- Defaults to Center to not change current behaviour


Co-authored-by: François <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@AndrewJakubowicz AndrewJakubowicz AndrewJakubowicz left review comments

@MrGVSV MrGVSV MrGVSV left review comments

@superdump superdump superdump approved these changes

@speakk speakk speakk approved these changes

@IceSentry IceSentry IceSentry approved these changes

@B-Reif B-Reif B-Reif approved these changes

Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pivot options for Sprite Sprite pivot (offset)
9 participants
@mockersf @alice-i-cecile @cart @superdump @speakk @IceSentry @B-Reif @AndrewJakubowicz @MrGVSV