Allow freezing of command execution or capturing commands

[makspll]

What problem does this solve or what need does it fill?

Bevy 0.16 introduced this change: https://bevy.org/learn/migration-guides/0-15-to-0-16/#flush-commands-after-every-mutation-in-worldentitymut,
which causes commands to be flushed at entity or component modifications.

bevy_mod_scripting uses commands as a way of allowing users to execute callbacks on their scripts, which can arbitrarily affect &mut World including spawn/despawn entities, or add/remove components.

One thing these commands can also do is potentially queue other callback commands, which are expected to run only once the first callback is completed (because callbacks necessitate removing some resources from the world)

What this change means for BMS is the potential of a callback to run early if a script adds an entity, causing errors.

Ideally It should be possible for my callback runner to "capture" or "freeze" any new commands from executing until it itself completes.

What solution would you like?

Either:

• Command capture mechanism, i.e. Commands::set_queue, which lets us push commands to a different queue (perhaps in a function scope to avoid gutting commands generally), and then Commands::append that queue when ready
• Or Commands::freeze which sets a flag that prevents commands from applying for the duration that freeze is set
  But the two above don't let me "ignore" the entity commands, so that they can be flushed immediately, but then defer other commands (without some sort of filterin)

Maybe the simplest solution would be not flushing commands other than those necessary for EntityRefMut to work, mine (perhaps naive) assumption is that commands only ever get applied at ApplyDeferred sync points.

What alternative(s) have you considered?

I could:

• pull out those resources into Arcs, and lock on every access, which doesn't require changes in Bevy
• Make the command "retry" itself if it knows the handler state is missing, which could have false positives

Additional context

Example reproduction repo from a user facing this problem: https://github.com/Peepo-Juice/bevy_mod_min_repro
With the problem being seen here:

[nicopap]

This is complicated. There is very little discussion about this behavior in #16219, only a vague reference in the "objective" section phrased as "of course we need to flush". There is more discussion in the following issues though:

• Commands apply at unexpected times with exclusive World access #14621
• Commands queued by component hooks are not applied during exclusive world access #16034

Back in my day, the "of course" was "of course we need to specifically avoid flushing". But regardless, I never liked WorldEntityMut because it's so awkward.

Have you considered using DeferredWorld. It "smells" like your Commands::freeze idea. You have the guarantee that commands won't be applied. You can still spawn/despawn entities through commands, but they won't be applied until you... hmm let mecheck. It appears you get a DeferredWorld from a UnsafeWorldCell, then once you kill the DeferredWorld, you can run the flush on the &mut World you can then get from UnsafeWorldCell afterward.

Not sure that's the right approach. If so, we should definitively have a more sensible API.

[makspll]

Hmm, I could potentially use DeferredWorld in my command but AFAIU it would also mean providing DeferredWorld to my "script world"/ callbacks, which is probably not desirable. On a second look I don't think I could, the handler relies on scoping the resources:

• without removing them it's actually not possible to safely expose a &mut World
  • without removal we'd have active borrows on resources, while the scripts are running
  • and so I couldn't provide exclusive access to the world, which is required for some operations (adding systems for example, unless I use commands everywhere and expect my users to do that too)

I guess the problem is a bit two fold:

• I do want to be able to make structual changes to the world in real time, I like the immediate flushing here
• I don't like the fact that unrelated Commands are being flushed

[Peepo-Juice]

Perhaps we can add a method to the Commands trait that acts as a flag (should_immediately_flush()?) with a default impl returning true. That way, we maintain the current behavior while allowing some commands to opt out.

[makspll]

For now I am resorting to something like this in my commands tried doing the below, but that seems to immediately apply the re-queued command:

pub(crate) struct HandlerCommandWrapper<P: IntoScriptPluginParams, F: FnOnce(&mut World)>(
    F,
    usize,
    PhantomData<fn(P::C, P::R)>,
);

impl<F: FnOnce(&mut World), P: IntoScriptPluginParams> HandlerCommandWrapper<P, F> {
    /// Creates a new HandlerCommandWrapper with the given command and retry count
    pub fn new(command: F) -> Self {
        Self(command, 0, PhantomData)
    }
}

impl<F: FnOnce(&mut World) + Send + Sync + 'static, P: IntoScriptPluginParams> Command
    for HandlerCommandWrapper<P, F>
{
    fn apply(mut self, world: &mut bevy::prelude::World) {
        if HandlerContext::<P>::is_yoinked(world) {
            if self.1 < 100 {
                // re-queue the command for the next flush point
                self.1 += 1;
                world.commands().queue(self);
            } else {
                bevy::log::error!(
                    "HandlerCommandWrapper: Command {} failed to run after 100 retries, skipping.",
                    std::any::type_name::<F>()
                );
            }
        } else {
            // run the command
            (self.0)(world);
        }
    }
}

So there doesn't seem an easy way to get around this