ThePat02 / BehaviourToolkit

A collection of tools for AI Behaviour in the Godot 4 Game Engine!
MIT License
303 stars 12 forks source link

Makes BTComposite nodes aware of changes in their children #44

Closed SirPigeonz closed 7 months ago

SirPigeonz commented 7 months ago

This makes possible to change children of a composite nodes at runtime, allowing for many cool stuff. 😄

I use it for injecting custom behaviors in a skill system in my game.

For AI it could be used to build mob generator, that takes scenes with pre-defined elements and creates many possible combinations, without need to create each scene manually by instancing sub-scenes.

ThePat02 commented 7 months ago

Nice feature! Here are my thoughts:

SirPigeonz commented 7 months ago

Good points! 😃

I think to merge this into main, we need to make sure that every node supports hot-swapping on runtime. I think things are very easily broken if your remove and add nodes to either a BT or a FSM while its running.

Thats true, at first I wanted to inject whole BTrees in my system starting with BTRoot but noticed that FSMStateIntegratedBT can't handle properly changing it's children at runtime. So I decided to do that in Composites because it looked like simple, clean and fitting place to do that, but making it available everywhere makes even more sense.

I can work on it if you want :) (First I will submit PR for 2.0 with delta change)

This will be also quite big change... should we make it 1.x feature or 2.x?

I think needing to call super() on every extended class is very tedious and can be confusing for new users. Maybe connecting the signal using @onready would solve this. Seems pretty hacky tho.

Is it even possible to use @onready for that? I wanted to makse setter that deos that but then I remembered that variables don't call setters on initialization. :( Unfortunately, and at the same time fortunately, manual super() in ready() will stay in GDS 2, technically is consistent and makes sense... but UX wise is meh, especially for Add-ons like this where you can and often should extend addon scripts... In 3.x it was called automatically in _ready() but many users complained that it's not consistent... now other half complainst that it's tedious... xD

I think we can minimize tediusness and make it less error prone and confusing if we would provide it in GDS templates? (I totally forgot to do that my bad! xD)

The function needs to check if the node is currently in the editor, because I will soon make all scripts a @tool in order to use configuration warnings.

Roger will do that! And 👍 🎉 🍾 for conf warning! :D

ThePat02 commented 7 months ago

Is it even possible to use @onready for that?

I was thinking of something like

@onready var blabla = wrapper()

func wrapper():
# Connect here
SirPigeonz commented 7 months ago

I was thinking of something like

@onready var blabla = wrapper()

func wrapper():
# Connect here

What?! I didn't know you can do that! D: This is cool.

That's actually a nice solution! I don't feel it's hacky at all and much better in our use case than super() that user has to remembder or not delete by accident...

ThePat02 commented 7 months ago

That's actually a nice solution! I don't feel it's hacky at all and much better in our use case than super() that user has to remembder or not delete by accident...

Since connect() does return Error you don't even need the wrapper.

ThePat02 commented 7 months ago

Is this PR redundant now that there is #47?

SirPigeonz commented 7 months ago

Is this PR redundant now that there is #47?

Ah I forgot. Yes, I left it just in case it might be needed but can be archived I think.