Added with_query to EntityCommands#9544
Added with_query to EntityCommands#9544bushrat011899 wants to merge 7 commits intobevyengine:mainfrom
with_query to EntityCommands#9544Conversation
| /// commands | ||
| /// .entity(player.entity) | ||
| /// .insert((Health(100), LuckyStart)) | ||
| /// /* ... */ | ||
| /// .with_query::<(&mut Health, &LuckyStart)>(|(mut health, _)| { | ||
| /// // Players with the LuckyStart component start with double health. | ||
| /// health.0 *= 2; | ||
| /// }); |
There was a problem hiding this comment.
This doesn't seem like an improvement compared to:
let player_commands = commands.entity(player.entity);
let base_health = 100;
if give_lucky_start {
player_commands.insert((Health(2 * base_health), LuckyStart));
} else {
player_commands.insert(Health(health));
}Or a system that applies the health bonus when an entity with a LuckyStart is spawned. Having the rule applied in a system would make it more discoverable and it would apply universally to anything that gets a LuckyStart such as enemies etc.
There was a problem hiding this comment.
| /// .with_query::<&mut Transform>(|mut transform| { | ||
| /// // Players should spawn slightly above their randomly chosen spawn point. | ||
| /// transform.translation += 0.1 * transform.up(); |
There was a problem hiding this comment.
Again, seems like this would be better implemented as a system, instead of adding a closure for every entity where you want to adjust its translation.
There was a problem hiding this comment.
I do think there's a material difference tho. Adding it as a system permanently adds it to the system schedule, adding whatever overhead (however small) that entails.
I will say that I don't necessarily believe my PR is an absolutely necessary change to Bevy, but it is the minimum reasonable solution (IMO) to the issue raised that this PR is attempting to close. If this PR does get rejected because the feature itself isn't worth adding, I do hope the issue does as well.
There was a problem hiding this comment.
I agree that this is an important distinction, and really what justifies this feature to me.
Co-authored-by: Alice Cecile <[email protected]>
alice-i-cecile
left a comment
There was a problem hiding this comment.
I think this is reasonably useful, but it needs a bit of polish before merging.
If others feel this is particularly useful or useless, I'm happy to follow your opinions there.
|
I see the utility, but I'll note that this feels like a more specialized form of one-shot systems (which I believe we will eventually support). Thinking holistically, would we want to support both this and one-shot systems? (We may -- I haven't formed a full opinion on this yet). Also, I think the inclusion of a Also, I think there should be some kind of error or warning if the command gets applied on an entity that doesn't match the query. |
|
Something that would achieve a similar effect is to implement commands
.spawn()
.add(|mut entity: EntityMut| {
entity.get_mut::<Health>().0 *= 2;
}); |
I like this much better! |
|
Closing this out; let's do that instead in a new PR. |
Objective
Solution
EntityCommands,with_querywhich allows a mutable query to be run for a specificEntity.Example
commands .entity(player.entity) .insert(ReallyComplexPlayerBundle::default()) // This adds Health to the Entity .with_query::<&mut Health>(|mut health| { health.0 = 100; });