-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
In use-after-despawn situation, make despawning system name available #2949
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -519,11 +519,15 @@ impl<'w, 's> SystemParamFetch<'w, 's> for CommandQueue { | |
#[inline] | ||
unsafe fn get_param( | ||
state: &'s mut Self, | ||
_system_meta: &SystemMeta, | ||
system_meta: &SystemMeta, | ||
world: &'w World, | ||
_change_tick: u32, | ||
) -> Self::Item { | ||
Commands::new(state, world) | ||
let name: &'static str = match system_meta.name { | ||
std::borrow::Cow::Borrowed(s) => s, | ||
std::borrow::Cow::Owned(_) => "owned", | ||
}; | ||
Commands::new(state, world).with_name(name) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I should just pass the Cow here 🐄 . It is in both cases cheap, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe it would be more appropriate to pass the whole SystemMeta, but well, I can't figure out the lifetime specifiers. And maybe the lifetime is not even guaranteed since the Despawn command, lands in a queue and the later the SystemMeta would land in some kind of buffer for the whole stage. |
||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
use bevy::prelude::*; | ||
|
||
fn main() { | ||
App::new() | ||
.add_startup_system(do_spawn) | ||
// This system despawns the entity in the Update stage. | ||
.add_system(do_despawn.system().label("despawn")) | ||
// This system inserts into the entity in the Update stage. | ||
// It is scheduled explicitly to run after do_despawn, so its Insert command | ||
// will panic because in the meantime the Despawn command despawed the entity. | ||
// | ||
// Here it is simple. But it could be as complicated a schedule like | ||
// the inserting system is some system of a third party plugin. | ||
// About this imagined system you as a user, | ||
// you don't immediately know it exists, | ||
// you don't know when it is scheduled, | ||
// you don't know if it is handling intermittent despawned entities well. | ||
// But eventually the parallel schedule is run in a way where | ||
// both systems, your despawner and the third party inserter, run in the same stage | ||
// in an order where they clash. | ||
// And when you don't know, which of my entity kinds was it? When was it despawned? | ||
// How to reproduce it? | ||
.add_system(do_insert.system().after("despawn")) | ||
.run(); | ||
} | ||
|
||
fn do_spawn(mut cmds: Commands) { | ||
let entity = cmds.spawn().insert(5u32).id(); | ||
println!("{:?} spawn", entity); | ||
} | ||
|
||
fn do_despawn(query: Query<Entity, With<u32>>, mut cmds: Commands) { | ||
for entity in query.iter() { | ||
println!("{:?} despawn", entity); | ||
cmds.entity(entity).despawn(); | ||
} | ||
} | ||
|
||
fn do_insert(query: Query<Entity, With<u32>>, mut cmds: Commands) { | ||
for entity in query.iter() { | ||
println!("{:?} insert", entity); | ||
cmds.entity(entity).insert("panic".to_string()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This set name thing is crude. I am sorry. It may could be a separate despawn* function on world instead which takes the despawn origin name.