Design of Star Composites

[d-bucur]

I'm a bit confused about the star variants for composites (both Selector and Sequence). My understanding is that the only difference between the normal version and the star version is that the star will continue execution with the currently RUNNING child on successive ticks, while the normal version will re-evaluate them in order and might select a previous child, even if there was already a RUNNING one. If I am not missing anything here I think this design has a few defects:

• The naming. Star doesn't mean anything and is confusing. The concept it's trying to do is to re-enter a previous state, so maybe call it Resumable instead? Or call the 'normal' one Interruptable
• Code duplication: the code between the two is practically identical, with just a few lines to skip previous children on iteration (some overhead in this, could just enter into the current index) selector vs selector star
• Defaults: I think the more common scenario is to have a Star version as it was very confusing at first to have my RUNNING behaviour not executing on successive ticks, and reading the docs I realized I would almost always use the Star variant, so I think this should be the default behaviour.

Proposal:
Unify normal and star versions into a single BeehaveNode, and have a flag to select re-entry into the running state. You could call it is_resumable (enabled by default, true: SelectorStar, false: Selector), or is_interruptable (disabled by default)

Not sure how this interacts with the recently added interruptions functionality as I haven't tried that out yet or fully understand how it works, but maybe there is some overlap?

At one point I though it could be an idea to have the flag set globally on a BeehaveRoot so that the execution resumes directly into the previously running node, but having the flexibility to set different values on each composite is definitely better.

[bitbrain]

This also relates slightly to #46, as we could indeed introduce flags on the Sequence and Selector nodes to control its forwarding behaviour, rather than offering different nodes.

One advantage of the *Star nodes is that its icon is instantly recognisable in the behaviour tree, whilst it is more difficult to spot where the flag was set to when all the Selector or Sequence node icons look the same (unless they are named accordingly)

[lostptr]

The Reactive proposal is the default behaviour for now. I think we should set the *Star as the default and call the other one as Reactive like so:

• Sequence -> ReactiveSequence
• SequenceStar -> Sequence
  And so on...

However, I would like them to be sepate nodes since they do different things in the end of the day. Sure, they have some duplicate code and I believe that it should be fixed by inheritance, not by flags so that it doesn't increase complexity. Otherwise, we could just write one big composite node with a bunch of flags and it would work but it would be a huge mess :/

As for the interruption functionality, as long as the running_child is correct, the interruptions should work fine.

[d-bucur]

@lostptr Yeah that makes sense. Looking at other implementations for BTs they all seem to favor having separate nodes, even if they do similar things. And it helps to have different icons for them in the editor.

I agree that the naming in bt.cpp is more descriptive, but minor nitpick: would call it SequenceReactive to maintain the prefix of Sequence so that it helps with code completion and it makes it clearer than it is a variant of Sequence. And if there would be a need for SequenceWithMemory you would have them all with the same convention (which bt.cpp weirdly breaks)

[bitbrain]

One challenge is that renaming nodes or script files may be breaking projects that use this addon already. We either "bite the bullet" early or it will have to wait until version 3.x of this addon (which may not see the light for years)

[d-bucur]

There's probably not much value in doing it now as it is just a rename of the defaults, but moving from Godot3 to Godot4 would be good timing for breaking changes as I imagine long term projects now would be using Godot3.

[bitbrain]

The current main branch is Godot 4 already. We could do the renaming now for Godot 4, considering that it is not released yet.