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] - Fix pipeline initialisation of wireframe mode (fixes #1609) #1623

Closed
wants to merge 1 commit into from

Conversation

simgt
Copy link

@simgt simgt commented Mar 11, 2021

More details are in the associated issue #1609.

While looking for the source of this issue, I've noticed that the set and set_untracked methods aren't really DRY:

let id: HandleId = handle.into();
if self.assets.insert(id, asset).is_some() {
self.events.send(AssetEvent::Modified {
handle: Handle::weak(id),
});
} else {
self.events.send(AssetEvent::Created {
handle: Handle::weak(id),
});
}

let id: HandleId = handle.into();
if self.assets.insert(id, asset).is_some() {
self.events.send(AssetEvent::Modified {
handle: Handle::weak(id),
});
} else {
self.events.send(AssetEvent::Created {
handle: Handle::weak(id),
});

Shouldn't set call set_untracked? Also, given the bug that arose from a misusage of these functions, maybe some refactoring is needed?

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior C-Code-Quality A section of code that is hard to understand or change A-Rendering Drawing game state to the screen labels Mar 11, 2021
@jakobhellermann
Copy link
Contributor

The other pipelines use set_untracked as well:

pipelines.set_untracked(FORWARD_PIPELINE_HANDLE, forward_pipeline);

pipelines.set_untracked(SPRITE_PIPELINE_HANDLE, build_sprite_pipeline(shaders));

I think the PR is ready for cart™

@cart
Copy link
Member

cart commented Mar 12, 2021

This is the right fix. Thanks!

While looking for the source of this issue, I've noticed that the set and set_untracked methods aren't really DRY:

Yeah set could call set_untracked. That would be a welcome change.

@cart
Copy link
Member

cart commented Mar 12, 2021

bors r+

bors bot pushed a commit that referenced this pull request Mar 12, 2021
More details are in the associated issue #1609.

While looking for the source of this issue, I've noticed that the `set` and `set_untracked` methods aren't really DRY:
https://github.com/bevyengine/bevy/blob/68606934e32ab45828c628e1cefd3873273f8708/crates/bevy_asset/src/assets.rs#L76-L85

https://github.com/bevyengine/bevy/blob/68606934e32ab45828c628e1cefd3873273f8708/crates/bevy_asset/src/assets.rs#L91-L99

Shouldn't `set` call `set_untracked`? Also, given the bug that arose from a misusage of these functions, maybe some refactoring is needed?
@bors bors bot changed the title Fix pipeline initialisation of wireframe mode (fixes #1609) [Merged by Bors] - Fix pipeline initialisation of wireframe mode (fixes #1609) Mar 12, 2021
@bors bors bot closed this Mar 12, 2021
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-Bug An unexpected or incorrect behavior C-Code-Quality A section of code that is hard to understand or change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants