Non-blocking data structure creation and destruction: "instant commands"

[alice-i-cecile]

RENDERED

Motivation

Commands allow users to mutate the world in arbitrary ways, so long as they wait for the next hard-sync point to do so.
But this unscoped power has several serious implications for performance Commands have several serious issues:

• their behavior is unpredictable and overly broad, as they have the ability to mutate the world arbitrarily
• because of this unscoped behavior, commands can only be applied at the next hard sync point
  • this delay makes prototyping harder, forces many more hard syncs than otherwise would be required, and consistently confuses new users
• because of this unscoped behavior, commands can only be applied one at a time, in slow, unbatched sequence

Summary

By adding a fourth, maximally privileged tier of data access (addition and removal > mutation > read-only > detection), we can move virtually all common functionality out of Commands.
This allows us to completely deprecate commands (and FromWorld) in favor of scoped data access plus exclusive systems which watch for specific events, simplifying the code base and API and significantly improving both performance and usability.

[alice-i-cecile]

There's some serious "how on earth do we implement this" work to be done still: I'm very much open to comments, suggestions and critiques on that front.

I want to examine the source more and do some light prototyping before attempting that however.

The main implementation complexity is in handling table-storage components: we can't instantly add / remove them in ways that modify the archetypes. I think our two main options here are:

1. Statically determine which archetypes might be needed, and pre-allocate them at app-creation.
2. Temporarily store new components of this type as sparse-set components, then convert them to dense storage when you have exclusive access.

Obviously approach 1 would have dramatically better scaling with the number of component additions / removals, but I'm not sure if it's feasible. The set of possible archetypes is likely to be quite large, even with scoped access to component insertion. This is particularly true without archetype invariants to restrict the set further.

[TheRawMeatball]

Even if the existing commands had synchronous versions added, I'd prefer to keep the existing commands api around, as it's a neat way to defer complicated work, and enables apis such as bevyengine/bevy#2234.

Furthermore, I think as an initial version we should not work on insertion to existing entities, but make it ergonomic to spawn new ones. This may or may not be a viable long-term strategy, but it'll greatly simplify initial implementation.

[TheRawMeatball]

Also btw, maybe add a rendered link?

[Lythenas]

If I understand correctly the goal is to allow Forge tier changes to happen in the middle of a stage (instead of just at the end with commands).

I think you don't explicitly state this but I assume the changes made through this api would be immediate. IMO there are also problems with that (maybe more complicated for the user to deal with than commands).

1. With commands the only thing you need to know is that they happen at the end of the stage. With the new api you basically force the users to explicitly order most systems. (IMO this will be very confusing for new users and probably for everyone). If you don't specify system order (or do it wrong) you might get different behavior of you systems every time you start your app or even every frame.
2. Even in one system this might become problematic. E.g. how would this system act or would it simply not compile?

// this could be something like a projectile that duplicates
// when it hits an enemy
fn system(foundry: Foundry<&X>, query: Query<&X, With<Duplicate>>) {
  // does the loop see the new entities
  // (if yes this would be an infinite loop)
  for x in query.iter() {
    foundry.spawn().insert(x.clone());
  }
}

I see three solutions:

1. don't compile
2. panic if you iterate a query where this happens (like a ConcurrentModificationException in java) then the user just needs to collect things and insert them after the loop and
3. only apply the changes at the end of the system (i.e. when Foundry is dropped).

Just as @TheRawMeatball says. Even if you extract all the functionality from Commands into other system parameters. I think it is worth keeping commands. I know that you can basically replace them with events + exclusive systems but I think current Commands are a lot more convenient if you want to delay things to the end of the stage.

ResForge<T> would be equivalent to having an Option<ResMut<T>> and Commands (or just Commands if you don't care about the old value or if it exists). This seems more like an edge case.

Also I don't 100% like the names but I also can't think of anything much better. I think I would prefer more "technical" terms. Foundry and Forge seem more like "in-game words".

[alice-i-cecile]

More discussion on this topic has occured on Discord.

[alice-i-cecile]

If I understand correctly the goal is to allow Forge tier changes to happen in the middle of a stage (instead of just at the end with commands).

Correct :) The main motivation for this is to allow simpler, more local gameplay code.

With commands the only thing you need to know is that they happen at the end of the stage. With the new api you basically force the users to explicitly order most systems. (IMO this will be very confusing for new users and probably for everyone). If you don't specify system order (or do it wrong) you might get different behavior of you systems every time you start your app or even every frame.

I agree with this. My experience making games in Bevy is that this is already largely true; system ordering ambiguities are pretty reliably a source of bugs. However, I definitely think the existing tools are inadequate for this. I wouldn't want to see this merged until bevyengine/bevy#2381 or something comparable is merged.

Even in one system this might become problematic. E.g. how would this system act or would it simply not compile?

Ah! That's a very interesting example. I think the right solution is to collect Forge changes and apply them on system drop.

Just as @TheRawMeatball says. Even if you extract all the functionality from Commands into other system parameters. I think it is worth keeping commands. I know that you can basically replace them with events + exclusive systems but I think current Commands are a lot more convenient if you want to delay things to the end of the stage.

Yeah, I wanted to propose the stronger version of this first, but I'd be happy to compromise on having two APIs depending on which characteristics you wanted. Not thrilled with the complexity increase, but the tradeoffs are real.

ResForge would be equivalent to having an Option<ResMut> and Commands (or just Commands if you don't care about the old value or if it exists). This seems more like an edge case.

Not quite: right now, resources inserted via commands aren't added immediately :( This is a common source of frustration for beginners, and I think would be much cleaner to implement than the entity-component equivalents.

Also I don't 100% like the names but I also can't think of anything much better. I think I would prefer more "technical" terms. Foundry and Forge seem more like "in-game words".

Yeah, I'm in agreement. I decide to pick something evocative and neutral over something misleading like "create". I think my preference would be something directly comparable to "mutable", but I've never seen this concept explicitly laid out before so I don't have existing naming conventions to draw on.

[hymm]

I'm a bit worried about the scheduling of the Foundry API for removing or adding components from an existing entity. Spawning and despawning entities only need access to one archetype, but adding and removing components on an existing entitiy is a move operation and thus needs access to two archetypes.

fn remove_honored(foundry: Foundry<&Honored>, player_entity: Res<PlayerEntity>){
    // Removing components works the same way as inserting them;
    // just use the .remove method
    foundry.entity(player_entity.0).remove::<Honored>();
}

The system graph is built from figuring out the potential component accesses from the function signature. The above function moves an entity from the set of archetypes with the Honored component to the set of archetypes without the Honored component. Taking the union of those two sets means that this system might have conflicting access with all archetypes. Since this system can potentially access all archetypes it cannot be run at the same time as any other system.

As @TheRawMeatball suggested on discord, we could narrow our archetypes with filters.

// You can access specific entities using Forge::entity()
fn remove_honored(foundry: Foundry<&Honored, With<Player>>, player_entity: Res<PlayerEntity>){
    // Removing components works the same way as inserting them;
    // just use the .remove method
    foundry.entity(player_entity.0).remove::<Honored>();
}

This would allow other systems to run in parallel.

My worry is that beginning users would default to the first function as it is simpler. But In practice I'm not sure how problematic allowing this footgun to exist is. Immediate spawning, despawning, adds, and removes might be worth the trade off.

Note that Queries with Forge's have the same problem. Though it may be more intuitive in this case as we're more used to dealing with queries.

fn add_death(query: Query<Forge<Dead>>) {
	for forge_dead in query.iter_mut() {	
		forge_dead.insert(Dead);
       }
}

On a related note I think you want to have a separate SystemParam for adding/removing components from spawning/despawning. As you can narrow the archetypes for spawning/despawning entities to just the archetypes with Component A. While adding and removing a component needs a bigger set of archetypes.

[alice-i-cecile]

On a related note I think you want to have a separate SystemParam for adding/removing components from spawning/despawning. As you can narrow the archetypes for spawning/despawning entities to just the archetypes with Component A. While adding and removing a component needs a bigger set of archetypes.

I agree :)

I'm a bit worried about the scheduling of the Foundry API for removing or adding components from an existing entity. Spawning and despawning entities only need access to one archetype, but adding and removing components on an existing entitiy is a move operation and thus needs access to two archetypes.

Yeah; fundamentally this idea is predicated on a workaround for dense tables. I don't have the energy to devote to solving that problem at the moment, so I'm going to close this out. Thanks for the feedback; I am still optimistic that we can make this work, but the costs and complexity are high enough that it's not worth prioritizing.

[hymm]

Sad to see this closed as it does seems like it is possible (just a lot of work). A couple of last thoughts:

• There might be value in just adding in typed commands, especially for larger code bases. It would make the data flow through the program more explicit.

• Would make sense to add something a sync_on_run builder function to systems as a stop gap for some of the use cases? It would introduce a hard sync into the middle of the schedule.

.add_system(my_command_system.sync_on_run())

[alice-i-cecile]

Sad to see this closed as it does seems like it is possible (just a lot of work).

Just a matter of prioritization :) There's a lot of other work to improve Bevy that I want to prioritize first, and I suspect that some of it will help make this area clearer. If someone else wants to take up this mantle, feel free to use this RFC as a starting point!

There might be value in just adding in typed commands, especially for larger code bases. It would make the data flow through the program more explicit.

Yes, this will also be required if we want any form of automatic batching.

Would make sense to add something a sync_on_run builder function to systems as a stop gap for some of the use cases? It would introduce a hard sync into the middle of the schedule.

I suspect we'll get something along these lines in various scheduler improvements that @Ratysz has planned. .hard_before was the other related idea that I had, where you ensure that a system is on the opposite side of a sync point from the label.

It matches the mental model well, but introduces complex nonlocal behavior as everything gets recalibrated by making changes.