Change looping in ControlNodes and Decorators to make them interruptable

I think that this issue opened a quite interesting Pandora’s box about how looping through children is done in some nodes:

  1. If a SequenceNode only has Synchronous children, all of them are executed atomically/sunchronously. The sequence can not be interrupted in the middle.
  2. Similarly, as described in the issue, RetryUntilSuccessful and Repeat will call the same sychronous child multiple times in a single tick, preventing the possibility to interrupt it.

I dismissed the 1st topic, saying that ReactiveSequence is what you should use to make a Sequence interruptable, but the 2nd one made me realize that, this way to approach looping is probably wrong.

  • If we have a Sequence, if would be better to return RUNNING between one child and the next.
  • If we have RetryUntilSuccessful, we should return running between on execution of the child and the next one.

For of course, the devil is in the details!

There is a tendency to add sleeps between one tick and the next. This would add a delay between consecutive Nodes that could be unacceptable.

Any idea?

It’s an interesting question.

I’ve always thought the tick as an atomic operation, and because of that I would have also dismissed 1) but on the grounds that the tick it’s expected to be atomic between the moment it is called and up to the moment something returns RUNNING (or we exhaust nodes to execute and the root node returns SUCCESS/FAILURE). Of course, that implies executing multiple nodes “atomically” but that’s ok as far as those take a finite (and short) amount of time to execute so that the tick interval is short.

In this mental model, external events that happen concurrently with the tick need not forbidden, but their effects will depend on what the exact details of the nodes implementation are. Also the effects of nodes need to be immediate (as opposed, for instance, to being mediated by an external spinner in the tick loop during the next tick) for execution paths not to become impossible within atomically ticked sequences of nodes.

I dismissed the 1st topic, saying that ReactiveSequence is what you should use to make a Sequence interruptable

Maybe I’m wrong, but I think that ReactiveSequence has the same issue in case 1), because it is also atomic within a tick, and if all children were sync nodes, the Reactive Sequence will still only exit once all of them have been ticked or after one of them has returned SUCCESS.

Still, I don’t think this is an issue under the assumption of the atomic tick and that children take a finite amount of time. It’s kind of a feature of the system, but I can understand people who’d like to trigger an immediate hard-stop mid-tick not being really satisfied with that answer.

(2) is a completely different thing. It’s a non-interruptible busy-loop, and I find the busy-loop part even worse than the non-interruptible one. Even if it eventually repeats only a finite number of times, it can block the tick loop and potentially prevent reactive nodes uptree from changing their minds.

If we have a Sequence, if would be better to return RUNNING between one child and the next. If we have RetryUntilSuccessful, we should return running between on execution of the child and the next one. For of course, the devil is in the details! There is a tendency to add sleeps between one tick and the next. This would add a delay between consecutive Nodes that could be unacceptable.

I agree that that sounds like it would add a lot of additional time for implementations that sleep between ticks (most?), and I still find that the atomicity of sequence nodes is a feature, not a bug (IMHO). Also, returning RUNNING in between children in Reactive control nodes would also increase by a lot the average number of times the first few children get executed. E.g. a ReactiveSequence with N children that ends ups succeeding would query the first child N times, the second N-1, the third N-2, etc.

[post edit: actually, would you even be able to make progress in this case? if a reactive node always returns RUNNING between children, then tick A, then return RUNNING, then the same Reactive Sequence is ticked, but it’s reactive, so ticks A again before ticking B. After A, the sequence needs to return RUNNING again before moving to B and the cycle repeats :sweat_smile: If it does not in return RUNNING between reattempts to children already visited and only before visiting a new child, then the ReactiveSequence is again “non-interruptible” within that sequence of revisits]

Having looping nodes to return RUNNING in between attempts at their children sounds reasonable, though. The only affected people would be those that were currently affected by busy loops in cases like 2), and that’s very likely not a wanted feature of their systems. This addresses loops, but not the atomicity of sequences.

If stopping the tree mid-tick is still a wanted feature, all control nodes can be made to be haltable (I think the current implementation of the halt functions assumes tick and halt are not concurrent, though, right?)

My 2c.

You are right! I would need to fix all the existing ControlNodes to follow this new policy.

I am going to propose a solution in the next days and see if this make sense or not.

Good news. I am addressing this in the upcoming version of BT.CPP 4.0.

I might eventually port it back to version 3.X, since it is not an API change.