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] - put update_frusta::<Projection> in UpdateProjectionFrusta set #7526

Closed
wants to merge 1 commit into from
Closed

[Merged by Bors] - put update_frusta::<Projection> in UpdateProjectionFrusta set #7526

wants to merge 1 commit into from

Conversation

jakobhellermann
Copy link
Contributor

Objective

  • less ambiguities

  • update_frusta<PerspectiveProjection> is in UpdatePerspectiveFrusta and update_frusta<OrthographicProjection> is in UpdateOrthographicFrusta, but UpdateProjectionFrusta is empty and update_frusta<Projection> is directly in PostUpdate

Solution

  • put update_frusta<Projection> in UpdatePerspectiveFrusta set

Before
image
After
image

@jakobhellermann jakobhellermann added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events labels Feb 6, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2023

Your PR increases Bevy Minimum Supported Rust Version. Please update the rust-version field in the root Cargo.toml file.

@alice-i-cecile
Copy link
Member

Your PR increases Bevy Minimum Supported Rust Version. Please update the rust-version field in the root Cargo.toml file.

@mockersf this doesn't seem right.

alice-i-cecile
alice-i-cecile approved these changes Feb 6, 2023
View reviewed changes
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.

Good change: this makes more sense.

@alice-i-cecile alice-i-cecile 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 Feb 6, 2023
@alice-i-cecile alice-i-cecile added this to the 0.10 milestone Feb 6, 2023
@alice-i-cecile alice-i-cecile added the A-Rendering Drawing game state to the screen label Feb 6, 2023
james7132
james7132 approved these changes Feb 6, 2023
View reviewed changes
Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

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

bors r+

bors bot pushed a commit that referenced this pull request Feb 6, 2023
@jakobhellermann
put update_frusta::<Projection> in UpdateProjectionFrusta set (#7526
e7e3b2c 
)

# Objective

- less ambiguities

- `update_frusta<PerspectiveProjection>` is in `UpdatePerspectiveFrusta` and `update_frusta<OrthographicProjection>` is in `UpdateOrthographicFrusta`, but `UpdateProjectionFrusta` is empty and `update_frusta<Projection>` is directly in `PostUpdate`

## Solution

- put `update_frusta<Projection>` in `UpdatePerspectiveFrusta` set

**Before**
![image](https://user-images.githubusercontent.com/22177966/217019086-22709204-0e39-4ffc-a43b-0175f86e17ec.png)
**After**
![image](https://user-images.githubusercontent.com/22177966/217019117-a28329a1-6614-490c-873f-773efadf6f41.png)
@bors
Copy link
Contributor

bors bot commented Feb 6, 2023

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title put update_frusta::<Projection> in UpdateProjectionFrusta set [Merged by Bors] - put update_frusta::<Projection> in UpdateProjectionFrusta set Feb 6, 2023
@bors bors bot closed this Feb 6, 2023
@jakobhellermann jakobhellermann deleted the remove-frusta-ambiguity branch February 6, 2023 21:11
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

@james7132 james7132 james7132 approved these changes

Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior 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
0.10
Development

Successfully merging this pull request may close these issues.
3 participants
@jakobhellermann @alice-i-cecile @james7132