Add register_system_cached to DeferredWorld.

[coolcatcoder]

Objective

Allow registering systems when uncached, while inside on_add and its ilk.

Solution

Add register_system_cached to DeferredWorld.

Testing

I tried it in an on_add and it seemed to work just fine. The first time the component was spawned, it took the uncached branch and registered the system. The second time the component was spawned, it took the cached branch and retrieved the cached SystemId.

Minor Flaw

If this function is called twice for the same system before commands are executed, then it will register the system twice.
If you wish me to, then I can make it send out a warning when this occurs.

[janhohenheim]

Maybe get_or_insert_system would be more idiomatic?

[coolcatcoder]

World already has register_system_cached, and so I named this function likewise.

[ItsDoot]

Can you add the test you made for it to the test suite?

[coolcatcoder]

Can you add the test you made for it to the test suite?

I'll try!

[coolcatcoder]

I think I have added the test successfully, and it passes.

[chescock]

This pattern of sometimes, but not always, deferring work with a command can make code hard to reason about. I think that's what you describe above as a "minor flaw", but I worry it's actually more insidious.

Like, if you use this in an on_add hook, it will work when spawning entities one-by-one, but will fail when used with spawn_batch, since that runs many hooks without flushing commands. It would be easy for a library author to use this without testing spawn_batch, and then a user of the library getting odd behavior without being able to fix it.

We've been bitten by similar issues with sometimes-deferred work and spawn_batch before; see #19356 for an example with relations.

---

Is the goal here to have a type-erased version of run_system_cached? I assume the reason you aren't calling it directly is that you want to pick the system at runtime.

Would it work to use a function pointer? If you do something like

let f: fn(&mut World) -> Result<(), RegisteredSystemError> =
    |world| world.run_system_cached(some_system);

then you have something Copy and pointer-sized that runs a system and caches it in the world, but that doesn't require world access to create or de-duplicate.