Component Lifecycle Hooks and a Deferred World

[james-j-obrien]

Objective

• Provide a reliable and performant mechanism to allows users to keep components synchronized with external sources: closing/opening sockets, updating indexes, debugging etc.
• Implement a generic mechanism to provide mutable access to the world without allowing structural changes; this will not only be used here but is a foundational piece for observers, which are key for a performant implementation of relations.

Solution

• Implement a new type DeferredWorld (naming is not important, StaticWorld is also suitable) that wraps a world pointer and prevents user code from making any structural changes to the ECS; spawning entities, creating components, initializing resources etc.
• Add component lifecycle hooks on_add, on_insert and on_remove that can be assigned callbacks in user code.

---

Changelog

• Add new DeferredWorld type.
• Add new world methods: register_component::<T> and register_component_with_descriptor. These differ from init_component in that they provide mutable access to the created ComponentInfo but will panic if the component is already in any archetypes. These restrictions serve two purposes:
  1. Prevent users from defining hooks for components that may already have associated hooks provided in another plugin. (a use case better served by observers)
  2. Ensure that when an Archetype is created it gets the appropriate flags to early-out when triggering hooks.
• Add methods to ComponentInfo: on_add, on_insert and on_remove to be used to register hooks of the form fn(DeferredWorld, Entity, ComponentId)
• Modify BundleInserter, BundleSpawner and EntityWorldMut to trigger component hooks when appropriate.
• Add bit flags to Archetype indicating whether or not any contained components have each type of hook, this can be expanded for other flags as needed.
• Add component_hooks example to illustrate usage. Try it out! It's fun to mash keys.

Safety

The changes to component insertion, removal and deletion involve a large amount of unsafe code and it's fair for that to raise some concern. I have attempted to document it as clearly as possible and have confirmed that all the hooks examples are accepted by cargo miri as not causing any undefined behavior. The largest issue is in ensuring there are no outstanding references when passing a DeferredWorld to the hooks which requires some use of raw pointers (as was already happening to some degree in those places) and I have taken some time to ensure that is the case but feel free to let me know if I've missed anything.

Performance

These changes come with a small but measurable performance cost of between 1-5% on add_remove benchmarks and between 1-3% on insert benchmarks. One consideration to be made is the existence of the current RemovedComponents which is on average more costly than the addition of on_remove hooks due to the early-out, however hooks doesn't completely remove the need for RemovedComponents as there is a chance you want to respond to the removal of a component that already has an on_remove hook defined in another plugin, so I have not removed it here. I do intend to deprecate it with the introduction of observers in a follow up PR.

Discussion Questions

• Currently DeferredWorld implements Deref to &World which makes sense conceptually, however it does cause some issues with rust-analyzer providing autocomplete for &mut World references which is annoying. There are alternative implementations that may address this but involve more code churn so I have attempted them here. The other alternative is to not implement Deref at all but that leads to a large amount of API duplication.
• DeferredWorld, StaticWorld, something else?
• In adding support for hooks to EntityWorldMut I encountered some unfortunate difficulties with my desired API. If commands are flushed after each call i.e. world.spawn() // flush commands .insert(A) // flush commands the entity may be despawned while EntityWorldMut still exists which is invalid. An alternative was then to add self.world.flush_commands() to the drop implementation for EntityWorldMut but that runs into other problems for implementing functions like into_unsafe_entity_cell. For now I have implemented a .flush() which will flush the commands and consume EntityWorldMut or users can manually run world.flush_commands() after using EntityWorldMut.
• In order to allowing querying on a deferred world we need implementations of WorldQuery to not break our guarantees of no structural changes through their UnsafeWorldCell. All our implementations do this, but there isn't currently any safety documentation specifying what is or isn't allowed for an implementation, just for the caller, (they also shouldn't be aliasing components they didn't specify access for etc.) is that something we should start doing? (see 10752)

Please check out the example component_hooks or the tests in bundle.rs for usage examples. I will continue to expand this description as I go.

See #10839 for a more ergonomic API built on top of this one that isn't subject to the same restrictions and supports SystemParam dependency injection.

[github-actions[bot]]

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

[github-actions[bot]]

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

[Diddykonga]

+1 For StaticWorld, as commands are the natural way of modifying world structure, and they are Deferred, which DeferredWorld can not do.

Edit: Oof, I should have looked at the code first.

[maniwani]

+1 For StaticWorld, as commands are the natural way of modifying world structure, and they are deferred, which DeferredWorld can not do.

It's called DeferredWorld in the sense of "you can't make immediate structural changes, you can only queue commands."

[Nilirad]

About finding a better name for DeferredWorld

The word static is synonym of immutable. I'm concerned this might mislead people into thinking FixedWorld cannot be mutated.

The main property of DeferredWorld is its peculiar mutability model. Let's see the mutability levels of World to help us find an appropriate name:

• Immutable. This is like passing &World to a function. It's functionally equivalent to systems with read-only parameters.
• Mutable with structural immutability. Non-exclusive systems can directly mutate the World, not arbitrarily, but only via system parameters, which prevent structural changes by design. However, systems can plan structural changes by using the Commands system parameter. DeferredWorld works in a functionally equal manner, where the only difference is the addition of a managed pointer that has the same structural immutability guarantees of system parameters.
• Fully mutable. Functions owning World, functions with parameter &mut World, and exclusive systems have full mutability potential, including structural mutability.

At this point we understand that we lack terminology for non-structural mutability. This kind of mutability ensures in other words that the layout of the World will remain the same despite allowing other kinds of mutability. This reminds me a lot about rigid bodies in physics who cannot be deformed (in theory), and, even a better analogy, solid state hardware, like SSDs, which have no mechanical moving parts, but they can change state internally.

Some names that came up to my mind are: SolidWorld, RigidWorld and HardWorld

Those names however don't clarify that the data structure is just a pointer to the World, not the real thing. I think we can live without that, or maybe we can add Ptr or Handle as suffix. Most importantly, they are based on arbitrary analogies that may not be immediate to others.

LLMs to the rescue!

I just had a conversation with ChatGPT (GPT-3.5 model) and I got some more inspiration:

• StructurallyStableWorldView: "view" conveys the feel of an indirection layer but IMO is too generic and doesn't stress enough the concept of mutability.
• ManagedWorldAccessor: Drops the "structurally stable" for a more generic "managed accessor" that can be clarified in API docs.
• WorldGuard: even more concise. The word "guard" implies protection from unauthorized access.
• StructuralWorldGuard: less concise than WorldGuard, but indicates the specificity of the guard.

[iiYese]

Could some way of having implicit hooks be considered? Maybe via the Component trait?

pub trait Component {
    // ..
    fn init_default_hooks(info: &mut ComponentInfo) {}
}

These method just gets called when initializing the component info. It would allow not needing to explicitly register hooks which is particularly useful for generic components where you might want the same hook behavior for all types.

[james-j-obrien]

Yep, I think that's a good idea and had considered something similar. I can fold that into this PR pretty easily 👍

[jnhyatt]

As @Nilirad pointed out, normal systems can access the world as mutable with structural immutability. Maybe we could sidestep the naming issue altogether by making hooks take systems? The entity and component_id parameters could be wrapped into a HookContext and passed as an input to the system. This way we wouldn't have to explain what kind of changes the user can and can't make, users already know how to use systems and what they can and can't do with them.

Example

.on_add(|ctx: In<HookContext>, mut commands: Commands| { ... })

[james-j-obrien]

As @Nilirad pointed out, normal systems can access the world as mutable with structural immutability. Maybe we could sidestep the naming issue altogether by making hooks take systems? The entity and component_id parameters could be wrapped into a HookContext and passed as an input to the system. This way we wouldn't have to explain what kind of changes the user can and can't make, users already know how to use systems and what they can and can't do with them.

The problem is this has a "huge" overhead for instantiating the system state that would be running in the middle of the hot loop for component insertion/deletion. If you want to run a system when a component is removed define a hook that schedules a one shot system, then you can take advantage of all the things a system can do. We can even define a method that takes a system and returns a hooks that adds that system to the command queue to make this easy, but I do not think making all hooks systems is a good idea is it encourages people to do long running tasks in hooks which should be avoided.

There is also currently no restriction on mutable world access for SystemParam which means we would need to go in and add a DeferredWorld like restriction to a lot of them anyway. By using one-shot systems that isn't a problem.

Hooks are intended to be a low level construct that isn't used by most users, the purpose of it's introduction is to build more ergonomic abstractions like observers and make other ECS improvements. Hence the limitations such as only have one hook per type per component and them all sharing the same function signature.

To support dependency injection in reactivity is better done as an extension to observers.

[jnhyatt]

@james-j-obrien I understand, thanks for the clarification.

[JoJoJet]

I love to see this finally getting developed.

I think we could remove most of the unsafe code for DeferredWorld by having it wrap &mut World instead of UnsafeWorldCell. Based on the way it's being used, I don't think it utilizes disjoint world access, so there's not much reason to use UnsafeWorldCell.

[james-j-obrien]

I think we could remove most of the unsafe code for DeferredWorld by having it wrap &mut World instead of UnsafeWorldCell. Based on the way it's being used, I don't think it utilizes disjoint world access, so there's not much reason to use UnsafeWorldCell.

I agree, I initially had it as &mut World and then swapped to a world cell for some reason while iterating, but I don't think there's a need for it anymore.

The only concern is it allows implementors of DeferredWorld to accidentally make structural changes through that reference without having to justify the safety.

Edit: The reason I had done this was to satisfy aliasing rules, have reverted to UnsafeWorldCell in a future commit.

[nicopap]

In terms of naming, my thinking is as follow:

DeferredWorld comes from "world where we can only execute deferred operations". But I think it's false right? We can mutate components as long as we don't remove/add them.

What are we trying to name? A variant of "world" where one cannot add/remove entities/components.

In this case StaticWorld is a pretty good choice, and we could content ourselves with it. But I just like to go to the end of my reasoning.

Click here to read my reasoning

What about FixedSizeWorld? Can't grow or shrink it. "Structural" also works StructurallyFrozenWorld (though that's 23 characters :hot_face:) FrozenEntitiesWorld (after all, what we avoid with our DeferredWorld is moving around entities) FrozenStructureWorld.

Continuing on this trail, we reach MonomorphWorld, which expresses that there is "one" (mono) "morph" (shape). But looking up the dictionary, we find "monomorph" to mean nothing. "monomorph" is the greek version. The latin version would be "uniforma". UniformWorld? Doesn't sound that bad, though I don't think it conveys the lack of change, rather the unique element. StasimorphWorld StabiliShapeWorld, StabiliformWorld, FixedShapeWorld.

Looking at a thesaurus for "stable" we have:

• constant
• nonchanging
• unchanging
• changeless
• unaltering
• unvarying
• uniform
• stable
• immutable
• invariable
• fixed
• perma-

Ok, so a "metamorphic rock" is a rock which shape has changed so much it is structurally different. The adjective for non-metamorphic rocks might be what we are after. The antonym seems to be "Igneous" or "Sedimentary"… not that useful.

So I think FixedShapeWorld is a pretty good name too.

[iiYese]

@nicopap

DeferredWorld comes from "world where we can only execute deferred operations". But I think it's false right? We can mutate components as long as we don't remove/add them.

No that's correct. Deferred operations are operations that can have structural mutations. Not all mutations are structural and not all deferred operations have structural mutations.

In this case StaticWorld is a pretty good choice.

That would be more sus because static in many programing contexts including rust also means immutable.

As for the other suggestions I don't think the distinction in mutation is easy to make with this style of naming. Most of them also conflate all mutability which is misleading. Rather than trying to describe the thing it would be better if we avoid describing what it's not. Such a term would be deliberately vague and can capture what the meaning actually is over time. Deferred fits.

[Nilirad]

The problem with DeferredWorld is there isn't anything inherently deferred in it, the deferred actions just come from using Commands.

[iiYese]

@Nilirad A similar concept already exists in rust: Clone. Things that are Clone don't inherently have side effects on duplication but can have side effects on duplication. The side effects come from whether or not you need to do other things to keep the state of your application valid like incrementing counters or performing allocations.

[Nilirad]

@iiYese I don't think that the concept of cloning implies (at least explicitly) the absence of side effects on duplication. However, deferred concept apart, the thing I dislike the most about DeferredWorld is the fact that the name gives, at least to me, the perception of an allocated World, while in reality it is just a pointer with methods. The name I suggested some days ago, WorldGuard, takes the pointer problem into account, and while not being specific on what it guards against, and acts as a meta-programming guard (if the change is structural, I don't give you the API) that prevents an entire class of mutations to World, which can be described on the doc comment (which already does).

The only thing that I can see in favor of DeferredWorld is that it was initially called DeferredWorld and we became so accustomed calling it DeferredWorld that we perceive it being the best name we can think of.

Anyway, the definitive name of DeferredWorld is not of utmost importance after all: it will be passed as a closure argument, where the type is inferred. The only occasion for it popping up is when viewing the docs to check out its methods. If James didn't come up with the naming proposal, probably nobody would have ever noticed.

PS: I found the perfect name: BikeshedWorld

[iiYese]

@Nilirad

I don't think that the concept of cloning implies (at least explicitly) the absence of side effects on duplication.

It doesn't necessarily imply them. Things that are Copy are also Clone.

[Nilirad]

I was thinking, instead of manually wrapping World APIs, which is prone to desync over time, would it be possible to move structurally invariant methods of World into a dedicated trait? This way we also have an explicit way to separate the two types of methods.

I laid out a simplified model. Here is the glossary:

• isomorphic_method represents a structurally invariant method, like get_mut.
• heteromorphic_method represents a strurally variant method, like spawn.

Before:

impl World {
	fn isomorphic_method(...) {...}
 	fn heteromorphic_method(...) {...}
}

impl DeferredWorld {
	fn isomorphic_method(...) {
 		self.world.isomorphic_method(...);
    }
}

After, with the trait:

trait IsomorphicWorldExt {
 	fn isomorphic_method(...);
}

impl World {
	fn heteromorphic_method(...) {...}
}

impl IsomorphicWorldExt for World {...}

impl IsomorphicWorldExt for DeferredWorld {...}

This has the benefit of not leaving behind DeferredWorld when isomorphic World methods change, given that we add new methods on the trait.

APIs must still be replicated in DeferredWorld's impl (maybe a derive macro or other types of macros are possible), but at least if methods are correctly added to World or to the IsomorphicWorldExt trait, there is no need to manually check everything. Plus, there's no plausible concern of accidentally adding a heteromorphic method in IsomorphicWorldExt.

I don't believe this is a complete solution, given that there are already other World extension traits that can include an isomorphic/heteromorphic mix of methods. Either we add them manually, or split those traits. In any case, each of those changes can be done incrementally, even after this PR gets merged.

[james-j-obrien]

I agree that there should probably be some refactoring to reduce the API duplication. One consideration I mention in the PR description is making World Deref to DeferredWorld so that the isomorphic methods can be implemented on DeferredWorld directly and not duplicated at all but using a trait is also a good option to consider. Either way I also agree that those refactors can be done in follow-up PRs.

[JoJoJet]

One consideration I mention in the PR description is making World Deref to DeferredWorld so that the isomorphic methods can be implemented on DeferredWorld directly and not duplicated at all but using a trait is also a good option to consider. Either way I also agree that those refactors can be done in follow-up PRs.

The way DeferredWorld is currently defined, it's not possible to make World deref to it. ~~You'd have to define the type as #[repr(transparent)] struct DeferredWorld(world), and std::mem::transmute an &mut World to an &mut DeferredWorld.~~

~~I think we might as well make that refactor in this PR. I don't think it would be a very big change, though it doesn't really matter what order we do it in.~~

[nicopap]

To be perfectly clear, the naming of DeferredWorld is really not important, and I was just throwing suggestions, I'm fine with whatever is chosen. DeferredWorld, StaticWorld, FixedWorld are all OK in my opinion.

I just didn't complete the review, therefore couldn't drop an approval/request for change yet. I'll get to it shortly.

[sjmueller]

Consider the name StructuredWorld which avoids the implication of immutability, but does confer that the structure of the world is rigid.

[github-actions[bot]]

You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.

[hymm]

I'm concerned about the 2 vec's and the 2 takes on world.command_queue when applying a command queue. Those are all going to allocate in something that is pretty hot. Has anyone run benchmarks on this yet?

[JoJoJet]

I think if we use a SmallVec for the stack of command queues, it would avoid the allocation in almost all cases.