Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

SirPigeonz
Copy link
Contributor

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 :)

@SirPigeonz
Copy link
Contributor Author

Resubmit of #47

@ThePat02 ThePat02 added the enhancement New feature or request label Jan 4, 2024
@ThePat02 ThePat02 added this to the v2.1.0 milestone Jan 4, 2024
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
Copy link
Contributor Author

@ThePat02 Rebased the branch ready for final review! :)

@ThePat02 ThePat02 self-requested a review January 7, 2024 21:27
@ThePat02
Copy link
Owner

ThePat02 commented Jan 7, 2024

Can you give me a quick overview of the changes? I'll look into it later this week!

@SirPigeonz
Copy link
Contributor Author

SirPigeonz commented Jan 7, 2024

Can you give me a quick overview of the changes? I'll look into it later this week!

Not much from last time you checked. I just had to fix PR because its previous target branch was deleted.

In rebase, I just had to merge few minor stuff mostly, conflicts in documentation. There is now more info how nodes work internally and how to work with them dynamically at runtime if somebody wishes to use that.

I also slightly modified some warning so they will be less, "you can't" -> "you shouldn't, but it's fine if you want to do it".

If you ask about most of the patch, summarizing it's mostly making base nodes smarter when handling non-typical setups, so they will not do weird stuff while being modified at runtime. Plus, it allows for some nice tricks using Godot node/scene system.

On top of that, some API to make modifying BT setup at runtime easier and cleaner. Although using standard API for nodes should still work, user just needs to be more aware how the BT nodes work internally.

@ThePat02
Copy link
Owner

ThePat02 commented Jan 7, 2024

On top of that, some API to make modifying BT setup at runtime easier and cleaner. Although using standard API for nodes should still work, user just needs to be more aware how the BT nodes work internally.

Great, I will take a closer look on that too! Thanks for your effort!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants