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

[Merged by Bors] - Add associated constant IDENTITY to Transform and friends. #5340

Closed
wants to merge 4 commits into from

Conversation

irate-devil
Copy link
Contributor

Objective

Since identity is a const fn that takes no arguments it seems logical to make it an associated constant.
This is also more in line with types from glam (eg. Quat::IDENTITY).

Migration Guide

The method identity() on Transform, GlobalTransform and TransformBundle has been deprecated.
Use the associated constant IDENTITY instead.

@alice-i-cecile alice-i-cecile added A-Transform Translations, rotations and scales C-Code-Quality A section of code that is hard to understand or change C-Usability A simple quality-of-life change that makes Bevy easier to use C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide labels Jul 16, 2022
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.

Nice use of deprecation :)

@mockersf
Copy link
Member

mockersf commented Aug 1, 2022

while marking as deprecated is nice, at this stage of Bevy I would prefer to break directly

Copy link
Contributor

@Weibye Weibye left a comment

Choose a reason for hiding this comment

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

Looks good, no need for the deprecation.

crates/bevy_transform/src/components/transform.rs Outdated Show resolved Hide resolved
crates/bevy_transform/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@Weibye Weibye left a comment

Choose a reason for hiding this comment

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

Looks good!

@Weibye Weibye added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Aug 28, 2022
@mockersf
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Aug 30, 2022
# Objective
Since `identity` is a const fn that takes no arguments it seems logical to make it an associated constant.
This is also more in line with types from glam (eg. `Quat::IDENTITY`).

## Migration Guide

The method `identity()` on `Transform`, `GlobalTransform` and `TransformBundle` has been deprecated.
Use the associated constant `IDENTITY` instead.

Co-authored-by: devil-ira <justthecooldude@gmail.com>
@bors bors bot changed the title Add associated constant IDENTITY to Transform and friends. [Merged by Bors] - Add associated constant IDENTITY to Transform and friends. Aug 30, 2022
@bors bors bot closed this Aug 30, 2022
@irate-devil irate-devil deleted the transform-identity branch October 21, 2022 15:10
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
…ngine#5340)

# Objective
Since `identity` is a const fn that takes no arguments it seems logical to make it an associated constant.
This is also more in line with types from glam (eg. `Quat::IDENTITY`).

## Migration Guide

The method `identity()` on `Transform`, `GlobalTransform` and `TransformBundle` has been deprecated.
Use the associated constant `IDENTITY` instead.

Co-authored-by: devil-ira <justthecooldude@gmail.com>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
…ngine#5340)

# Objective
Since `identity` is a const fn that takes no arguments it seems logical to make it an associated constant.
This is also more in line with types from glam (eg. `Quat::IDENTITY`).

## Migration Guide

The method `identity()` on `Transform`, `GlobalTransform` and `TransformBundle` has been deprecated.
Use the associated constant `IDENTITY` instead.

Co-authored-by: devil-ira <justthecooldude@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Transform Translations, rotations and scales C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide C-Code-Quality A section of code that is hard to understand or change C-Usability A simple 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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants