add: Dynamic changes to Behaviour Patterns at runtime #47

SirPigeonz · 2023-12-01T01:49:26Z

This PR makes both FSM and BTree systems work properly when their nodes are moved, removed, added at runtime.

On top of that many nodes are now fine when they don't have important for them nodes as a child or property references. This allows for preparing "empty" slots for intended behaviours or branches and modeling main behaviours with them, that is "slots", in mind.
Upcomming warning system can be used instead for important configuration warnings when behaviours are created by hand in the Editor.

Changes:

BTRoot, FiniteStateMachine, BTComposite, BTDecorator, FSMStateIntegratedBT and BTIntegratedFSM nodes now can reconfigure themselves when their children are modified at run-time
allow BTRoot to not have any BTBehaviour as a child
allow FiniteStateMachine to not have any FSMStates as a children
allow FiniteStateMachine to not have a initial state
allow FSMStateIntegratedBT to not have any BTRoot as a child
allow BTIntegratedFSM to not have any FSM as a child
BTIntegratedFSM has now default_status property enum for case where it doesn't have FSM child
new method in BTRoot - swap_entry_point()
new method in FSMStateIntegratedBT - swap_behaviour_tree()
new method in BTIntegratedFSM - swap_finite_state_machine()
make FSMStates and extended nodes update their transitions list when children are added/removed
FSMStateIntegratedBT disables autostart of its BTRoot
BTIntegratedFSM disables autostart of its FSM
made FSMState extensible without need to call super() on _ready()
made BTRandom extensible without need to call super() on _ready()
Added few missing documentation comments, to outline how nodes are intended to work and how they should be edited also at run-time.

Changes after review 1

removed _disable_autostart() in FSM and BTRoot
made BTRoot and FSM set their autostart to false and hide them in Editor Inspector if their parent is a integration type node
made BTRoot and FSM a @tool
added Engine.is_engine_hint guards to ready, callbacks and processes for BTRoot and FSM
added keep_group optional property to swap_'nodetype'() methods, it allows to preserve original nodes groups from swapped node in the new node

Changes after review 2

If you wish to add, remove, move FSMState nodes at run-time first add new FSMStates stop the FSM with method FiniteStateMachine.exit_active_state_and_stop and re-start it with method method FiniteStateMachine.start providing one of the new states either as start method property or change member FiniteStateMachine.initial_state before running start(). After this procedure you can delete unused states.

made active property read-only
modified start() in fsm.gd to accept FSMState property as a start point
new method exit_active_state_and_stop() to pair with start()
above two changes make FSM startable and stoppable for example to safely modify FSMStates and resume running of the FSM
removed some if guards from proccess function and made checks when something changes in the setup
made BTRoot cleanups and made it to not check for entry point in processing
changed some configuration warnings. Mostly changed statement that state "nodes must have child nodes", to "nodes SHOULD have child nodes to work" to inform user that they wont work but nothing bad will happen if they don't
removed warning for BTLeaf that "BTLeaf node must not have any children.". Reason is that there is no issue if it has one, it can prevent user to use some nice composition on top of BTLeaf for no reason :)

ThePat02 · 2023-12-01T09:55:53Z

Keep in mind that some things might change how everything is handled, since #17 requires every script to be a @tool!

SirPigeonz · 2023-12-01T16:28:19Z

Keep in mind that some things might change how everything is handled, since #17 requires every script to be a @tool!

Yup :) I already added few guards with if Engine.is_editor_hint().

SirPigeonz · 2023-12-02T01:36:37Z

@ThePat02
Ready for review :)

This time for sure xD

ThePat02 · 2023-12-02T09:10:40Z

Good work! I'll look into it when I finished implementing #17 to avoid any breaking conflicts.

SirPigeonz · 2023-12-02T16:27:05Z

Good work!

Senkyu! :3

I'll look into it when I finished implementing #17 to avoid any breaking conflicts.

Do as you prefer :)
Although, I would personally do opposite, because I changed some behaviours of the addon. Some configurations that were illegal and crash in the past now are legal just do nothing and would need different type of warning and asserts. (I'm just worried that some of your work will be not needed).

That said I tried to test it diligently as I could. Each node was tested alone and with others and main example also runs as used to.
I also plan to move to this version (2.0 + this PR) with my prototype so I will probably also catch regressions.

What I want the most is it for you to look into the design and tell me what you think and if I missed something, or should change something :)

examples/behaviour_example/actor/actor.gd

ThePat02

The FSM looks good. I'll check the BT later and do some testing!

addons/behaviour_toolkit/finite_state_machine/fsm.gd

addons/behaviour_toolkit/finite_state_machine/fsm_state.gd

addons/behaviour_toolkit/finite_state_machine/fsm_state_integrated_bt.gd

ThePat02

The FSM looks good. I'll check the BT later and do some testing!

ThePat02

One other thing I noticed: When changing states of a FSM, what happens if you change the active state? Did the current state exit?

SirPigeonz · 2023-12-03T22:00:35Z

One other thing I noticed: When changing states of a FSM, what happens if you change the active state? Did the current state exit?

Good question!
Hmm, I should add active_state guard for processing too.

When you change state with change_state() it runs states exit same with transitions.
Or do you mean is it running exit when deleted with Node API? Then no it doesn't run exit. Should it?

We could add method to FiniteStateMachine for deleting states that calls exit if state is current active_state, for situations it's needed.

What do you think?

SirPigeonz · 2023-12-03T23:46:26Z

@ThePat02
Adressed few points in new commit:

removed _disable_autostart() in FSM and BTRoot integration nodes
made BTRoot and FSM set their autostart to false and hide them in Editor Inspector if their parent is a integration type node
made BTRoot and FSM a @tool (needed for _validate_property(property))
added Engine.is_engine_hint guards to ready, callbacks and processes for BTRoot and FSM
added keep_group optional property to swap_'nodetype'() methods, it allows to preserve original nodes groups from swapped node in the new node
added guard to FSM process if active_state == null

ThePat02 · 2023-12-04T09:25:17Z

One other thing I noticed: When changing states of a FSM, what happens if you change the active state? Did the current state exit?

Good question! Hmm, I should add active_state guard for processing too.

When you change state with change_state() it runs states exit same with transitions. Or do you mean is it running exit when deleted with Node API? Then no it doesn't run exit. Should it?

We could add method to FiniteStateMachine for deleting states that calls exit if state is current active_state, for situations it's needed.

What do you think?

Well what is the workflow with dynamic changing of nodes? There needs to be some kind of failsafe when changin states manually and via code.

SirPigeonz · 2023-12-04T10:53:44Z

Well what is the workflow with dynamic changing of nodes? There needs to be some kind of failsafe when changin states manually and via code.

Ahhh now I understand, sorry 😁 Ok yeah, there should be some safe API to model states the way user needs for their use case. I will think more about it and write my API ideas.

ThePat02 · 2023-12-13T12:59:30Z

I think it is best to push this feature to a later version of the plugin (possibliy 2.1) as there are some other pitfalls and problems that come with this. I think it is very important to release the new syntax changes, as more and more people are interested in using it. I'd like to avoid problems like people having to rewrite their code with the new syntax too much.

SirPigeonz · 2023-12-13T14:38:00Z

I think it is best to push this feature to a later version of the plugin (possibliy 2.1) as there are some other pitfalls and problems that come with this. I think it is very important to release the new syntax changes, as more and more people are interested in using it. I'd like to avoid problems like people having to rewrite their code with the new syntax too much.

I'm fine if it will be in 2.1 if you want to release 2.0 early due to changes in API. I use latest code in my game either way.

Furthermore, I just finished a bigger commission for a client, so I will have more free time now to work on this and my game. I will probably finish a new version of this PR today or tomorrow.

ThePat02 · 2023-12-13T17:57:01Z

Sounds great. If you need anything, make sure to add me. I'll unsub for now so I don't get notis for every commit!

@tool

This PR makes both FSM and BTree systems work properly when their nodes are moved, removed, added at runtime. On top of that many nodes are now fine when they don't have important for them nodes as a child or property references. This allows for preparing "empty" slots for intended behaviours or branches and modeling main behaviours with them, that is "slots", in mind. Upcomming warning system can be used instead for important configuration warnings when behaviours are created by hand in the Editor. Changes: - `BTRoot`, `FiniteStateMachine`, `BTComposite`, `BTDecorator`, `FSMStateIntegratedBT` and `BTIntegratedFSM` nodes now can reconfigure themselves when their children are modified at run-time - allow `BTRoot` to not have any BTBehaviour as a child - allow `FiniteStateMachine` to not have any FSMStates as a children - allow `FiniteStateMachine` to not have a initial state - allow `FSMStateIntegratedBT` to not have any BTRoot as a child - allow `BTIntegratedFSM` to not have any FSM as a child - `BTIntegratedFSM` has now default_status property enum for case where it doesn't have FSM child - new method in `BTRoot` - `swap_entry_point()` - new method in `FSMStateIntegratedBT` - `swap_behaviour_tree()` - new method in `BTIntegratedFSM` - `swap_finite_state_machine()` - make `FSMStates` and extended nodes update their transitions list when children are added/removed - `FSMStateIntegratedBT` disables autostart of its BTRoot - `BTIntegratedFSM` disables autostart of its FSM - made `FSMState` extensible without need to call super() on _ready() - made `BTRandom` extensible without need to call super() on _ready() - Added few missing documentation comments, to outline how nodes are intended to work and how they should be edited also at run-time. Changes after review 1 - removed `_disable_autostart()` in FSM and BTRoot - made `BTRoot` and `FSM `set their `autostart` to `false` and hide them in Editor Inspector if their parent is a integration type node - made `BTRoot` and `FSM` a @tool - added Engine.is_engine_hint guards to ready, callbacks and processes for `BTRoot` and `FSM` - added `keep_group` optional property to `swap_'nodetype'()` methods, it allows to preserve original nodes groups from swapped node in the new node Changes after review 2 If you wish to add, remove, move `FSMState` nodes at run-time first add new `FSMStates` stop the FSM with method `FiniteStateMachine.exit_active_state_and_stop` and re-start it with method method `FiniteStateMachine.start` providing one of the new states either as start method property or change member `FiniteStateMachine.initial_state` before running `start()`. After this procedure you can delete unused states. - made `active` property read-only - modified `start()` in fsm.gd to accept `FSMState` property as a start point - new method exit_active_state_and_stop() to pair with `start()` - above two changes make FSM startable and stoppable for example to safely modify `FSMStates` and resume running of the FSM - removed some `if` guards from proccess function and made checks when something changes in the setup - made BTRoot cleanups and made it to not check for entry point in processing - changed some configuration warnings. Mostly changed statement that state "nodes must have child nodes", to "nodes SHOULD have child nodes to work" to inform user that they wont work but nothing bad will happen if they don't - removed warning for `BTLeaf` that "BTLeaf node must not have any children.". Reason is that there is no issue if it has one, it can prevent user to use some nice composition on top of `BTLeaf` for no reason :)

Formatting will format it :)

SirPigeonz · 2023-12-13T18:35:50Z

@ThePat02
Ready for review again :)

ThePat02 · 2023-12-14T14:45:29Z

I'll take a look at it after finishing up the other changes. BTW I added a formatting action you can steal for your own projects.

SirPigeonz changed the title ~~Draft: Makes Behaviors more susceptible for modifications at runtime~~ Draft: Makes Behaviors more open to modifications at runtime Dec 1, 2023

SirPigeonz force-pushed the runtime-dynamic branch 4 times, most recently from e1368a1 to 4898f61 Compare December 1, 2023 23:02

SirPigeonz changed the title ~~Draft: Makes Behaviors more open to modifications at runtime~~ Draft: Makes Behaviors more dynamic an open for modyfications at runtime Dec 1, 2023

SirPigeonz marked this pull request as ready for review December 1, 2023 23:10

SirPigeonz changed the title ~~Draft: Makes Behaviors more dynamic an open for modyfications at runtime~~ Makes Behaviors more dynamic an open for modyfications at runtime Dec 1, 2023

SirPigeonz changed the title ~~Makes Behaviors more dynamic an open for modyfications at runtime~~ Make Behaviors more dynamic an open for modyfications at runtime Dec 1, 2023

SirPigeonz changed the title ~~Make Behaviors more dynamic an open for modyfications at runtime~~ Make Behaviors more dynamic and open for modyfications at runtime Dec 1, 2023

SirPigeonz changed the title ~~Make Behaviors more dynamic and open for modyfications at runtime~~ Make Behaviors more dynamic and open for modifications at runtime Dec 1, 2023

SirPigeonz marked this pull request as draft December 1, 2023 23:19

SirPigeonz force-pushed the runtime-dynamic branch from 4898f61 to 99b8ef0 Compare December 2, 2023 01:35

SirPigeonz marked this pull request as ready for review December 2, 2023 01:36

ThePat02 self-requested a review December 2, 2023 08:43

ThePat02 reviewed Dec 3, 2023

View reviewed changes

examples/behaviour_example/actor/actor.gd Outdated Show resolved Hide resolved

ThePat02 requested changes Dec 3, 2023

View reviewed changes

ThePat02 reviewed Dec 3, 2023

View reviewed changes

ThePat02 changed the title ~~Make Behaviors more dynamic and open for modifications at runtime~~ add: Dynamic changes to Behaviour Patterns at runtime Dec 3, 2023

ThePat02 added this to the v2.0.0 milestone Dec 3, 2023

ThePat02 assigned SirPigeonz Dec 3, 2023

ThePat02 added the enhancement New feature or request label Dec 3, 2023

ThePat02 mentioned this pull request Dec 3, 2023

Makes BTComposite nodes aware of changes in their children #44

Closed

ThePat02 reviewed Dec 3, 2023

View reviewed changes

ThePat02 marked this pull request as draft December 9, 2023 12:24

ThePat02 modified the milestones: v2.0.0, v2.1.0 Dec 13, 2023

ThePat02 mentioned this pull request Dec 13, 2023

add: Configuration Warnings and Asserts #46

Merged

SirPigeonz force-pushed the runtime-dynamic branch from 6642533 to b194734 Compare December 13, 2023 14:47

SirPigeonz force-pushed the runtime-dynamic branch 2 times, most recently from b0eab07 to 7984206 Compare December 13, 2023 18:24

SirPigeonz force-pushed the runtime-dynamic branch from 7984206 to 7adf554 Compare December 13, 2023 18:29

Removed line breaks in @onready connects

88c631d

Formatting will format it :)

SirPigeonz marked this pull request as ready for review December 13, 2023 18:35

SirPigeonz requested a review from ThePat02 December 14, 2023 08:57

ThePat02 deleted the branch ThePat02:2.0.0 December 30, 2023 16:51

ThePat02 closed this Dec 30, 2023

ThePat02 mentioned this pull request Dec 31, 2023

Convert BehaviourToolKit to use resources and graphic edit #65

Closed

SirPigeonz mentioned this pull request Jan 4, 2024

add: Dynamic changes to Behaviour Patterns at runtime Nr.2 #73

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add: Dynamic changes to Behaviour Patterns at runtime #47

add: Dynamic changes to Behaviour Patterns at runtime #47

SirPigeonz commented Dec 1, 2023 •

edited

Loading

ThePat02 commented Dec 1, 2023

SirPigeonz commented Dec 1, 2023

SirPigeonz commented Dec 2, 2023

ThePat02 commented Dec 2, 2023

SirPigeonz commented Dec 2, 2023 •

edited

Loading

ThePat02 left a comment

ThePat02 left a comment

ThePat02 left a comment

SirPigeonz commented Dec 3, 2023

SirPigeonz commented Dec 3, 2023 •

edited

Loading

ThePat02 commented Dec 4, 2023

SirPigeonz commented Dec 4, 2023

ThePat02 commented Dec 13, 2023

SirPigeonz commented Dec 13, 2023

ThePat02 commented Dec 13, 2023

SirPigeonz commented Dec 13, 2023

ThePat02 commented Dec 14, 2023

add: Dynamic changes to Behaviour Patterns at runtime #47

add: Dynamic changes to Behaviour Patterns at runtime #47

Conversation

SirPigeonz commented Dec 1, 2023 • edited Loading

ThePat02 commented Dec 1, 2023

SirPigeonz commented Dec 1, 2023

SirPigeonz commented Dec 2, 2023

ThePat02 commented Dec 2, 2023

SirPigeonz commented Dec 2, 2023 • edited Loading

ThePat02 left a comment

Choose a reason for hiding this comment

ThePat02 left a comment

Choose a reason for hiding this comment

ThePat02 left a comment

Choose a reason for hiding this comment

SirPigeonz commented Dec 3, 2023

SirPigeonz commented Dec 3, 2023 • edited Loading

ThePat02 commented Dec 4, 2023

SirPigeonz commented Dec 4, 2023

ThePat02 commented Dec 13, 2023

SirPigeonz commented Dec 13, 2023

ThePat02 commented Dec 13, 2023

SirPigeonz commented Dec 13, 2023

ThePat02 commented Dec 14, 2023

SirPigeonz commented Dec 1, 2023 •

edited

Loading

SirPigeonz commented Dec 2, 2023 •

edited

Loading

SirPigeonz commented Dec 3, 2023 •

edited

Loading