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

allow to explicitly define the z-order #9713

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JHNP727
Copy link

@JHNP727 JHNP727 commented Sep 6, 2023

Objective

This allows to specify an explicit z value for ordering instead of relying on the z translation value which might already be used for actual positioning.

Solution

We add an extra optional field that can be set. The default behavior of using the z translation does not change.


Changelog

Added

  • Allows to specify an explicit sprite ordering value when using the 2D renderer

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@JHNP727 JHNP727 force-pushed the add-explicit-z-index branch from 6f6004d to 14c8d97 Compare September 6, 2023 19:03
@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen labels Sep 7, 2023
@alice-i-cecile
Copy link
Member

Related to #1275.

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Very much in favor of moving away from using the Transform's z value for ordering 2D objects, one way or another.

It feels like this would be better suited as a distinct component (so then it can be reused across sprites and UI, and avoid increasing the size of Sprite) rather than as a field on Sprite.

@alice-i-cecile alice-i-cecile added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Sep 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2023

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

@CMorrison82z
Copy link

CMorrison82z commented Oct 27, 2023

Status ? I don't think using Transform for this is a good idea as it could have unintended side effects. Rather, I think it should affect to Bevy Render order instead

Edit : I currently create another entity for the sprite and set the parent to the desired entity and this would avoid most side-effects

@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Oct 27, 2023
@alice-i-cecile alice-i-cecile added X-Contentious There are nontrivial implications that should be thought through and removed X-Controversial There is active debate or serious implications around merging this PR labels Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide X-Contentious There are nontrivial implications that should be thought through
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants