Skip to content

Rendertarget cmp (adopted) #15353

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

Closed
wants to merge 3 commits into from
Closed

Conversation

bas-ie
Copy link
Contributor

@bas-ie bas-ie commented Sep 22, 2024

This is an adoption of #8300 (aevyrie). The only change I've made is removing Ord as suggested, and PartialOrd because I presumed that made sense.

Also took out FromReflect, because I see we get that for free now with Reflect.

Objective

  • The changes in Windows as Entities removed the ability to compare RenderTargets without bringing in additional query data. This should not be needed, as these types are directly comparable.

Solution

  • Reinstate derive macros.

@alice-i-cecile alice-i-cecile added this to the 0.15 milestone Sep 22, 2024
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 22, 2024
@bas-ie
Copy link
Contributor Author

bas-ie commented Sep 22, 2024

Ah, yes I was possible a bit naiive here! Given the CI failure, I presume I would need to conduct some surgery on sort_cameras to make this work?

@tychedelia
Copy link
Member

tychedelia commented Sep 22, 2024

Ah, yes I was possible a bit naiive here! Given the CI failure, I presume I would need to conduct some surgery on sort_cameras to make this work?

I'm not sure I totally understand the original justification for having different render targets in the same order. The implicit sort just feels like a bug waiting to happen. I think we should change the ambiguity code to enforce an explicit total order (i.e. unique per camera). Curious what others think.

@bas-ie
Copy link
Contributor Author

bas-ie commented Sep 22, 2024

I'm not sure I totally understand the original justification for having different render targets in the same order. The implicit sort just feels like a bug waiting to happen. I think we should change the ambiguity code to enforce an explicit total order (i.e. unique per camera). Curious what others think.

Awesome, I definitely am not qualified for that so I'll wait to hear the result!

@bas-ie bas-ie deleted the rendertarget-cmp branch September 23, 2024 22:05
@bas-ie
Copy link
Contributor Author

bas-ie commented Sep 23, 2024

Lack of clear consensus here in 2024, Discord discussion for further context.

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-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants