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] - Apply buffers in ParamSet #4677

Closed
wants to merge 2 commits into from

Conversation

DJMcNab
Copy link
Member

@DJMcNab DJMcNab commented May 6, 2022

Objective

Solution

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label May 6, 2022
@DJMcNab DJMcNab added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events and removed S-Needs-Triage This issue needs to be labelled labels May 6, 2022
@BoxyUwU
Copy link
Member

BoxyUwU commented May 6, 2022

I wonder if we ought to make that a non defaulted function to avoid this kind of error in the future, it would be only a single line of "noise" on trait impls that dont need to apply anything, and manually implementing this trait to begin with is remarkably niche (tbh same line of thought for new_archetype given that is pretty important to not mess up overriding the default of...)

@alice-i-cecile alice-i-cecile added this to the Bevy 0.8 milestone May 6, 2022
@alice-i-cecile
Copy link
Member

@DJMcNab Can you add a quick test for this? Once that's done, I'm happy to merge it.

@DJMcNab
Copy link
Member Author

DJMcNab commented May 6, 2022

Presumably we can't merge it until CI resurrects itself?

@alice-i-cecile
Copy link
Member

#4675 was r+'ed an entire 4 minutes before you posted that ;)

@alice-i-cecile
Copy link
Member

bors r+

@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 May 6, 2022
bors bot pushed a commit that referenced this pull request May 6, 2022
# Objective

- Fix #4676

## Solution

- Fixes #4676
- I have no reason to think this isn't sound, but `ParamSet` is a bit spooky
@bors bors bot changed the title Apply buffers in ParamSet [Merged by Bors] - Apply buffers in ParamSet May 6, 2022
@bors bors bot closed this May 6, 2022
@DJMcNab DJMcNab deleted the paramset-apply branch May 6, 2022 19:10
robtfm pushed a commit to robtfm/bevy that referenced this pull request May 10, 2022
# Objective

- Fix bevyengine#4676

## Solution

- Fixes bevyengine#4676
- I have no reason to think this isn't sound, but `ParamSet` is a bit spooky
exjam pushed a commit to exjam/bevy that referenced this pull request May 22, 2022
# Objective

- Fix bevyengine#4676

## Solution

- Fixes bevyengine#4676
- I have no reason to think this isn't sound, but `ParamSet` is a bit spooky
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- Fix bevyengine#4676

## Solution

- Fixes bevyengine#4676
- I have no reason to think this isn't sound, but `ParamSet` is a bit spooky
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events 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
Development

Successfully merging this pull request may close these issues.

Commands get ignored when using ParamSet (with minimum example)
4 participants