search

ThePat02 / BehaviourToolkit

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

I want to add Selectable 'process_frame' for BTree and FSM + API changes question #26

Closed SirPigeonz closed 7 months ago

SirPigeonz commented 7 months ago

Yo :) I've noticed that BT work on _process() by default. For my use case I will need it to work on physics frame not idle, so I wanted to create PR with functionality to select between IDLE and PHYSICS frames processing as an @export in BTree and FSM.

I poked at Beehave and their master also has it now (althoug AssetLib version doesnt). I would probably implement it in similar way, although they don't provide delta in tick() so users while coding the leafs will have to guess which process the Tree is using.

Thata said, while checking the BT code I've noticed that you store current delta in the Blackboard resource which is fine by me, but is there a reason to not provide it as a property in the tick() ?

Something like:

func tick(_delta: float, _actor: Node, _blackboard: Blackboard) -> Status:
    return Status.SUCCESS

I think this would be more performant, easier to optimise for GDScript, less error prone from user perspective and easier for user to understand how to use the API.

I also think that default should be PHYSICS_FRAME_PROCESS becasue BTree will most likely by used mostly to modify Actor that is a CollisionObject2D or CollisionObject3D. And if user would need to run his BTree or FSM for some visual stuff where he want the code to run on every IDLE frame he will simply switch from default PHYSICS process to IDLE in the inspector. :)

What do you think?

ThePat02 commented 7 months ago

Thank you for your input! I was actually planning on rewriting/refactoring the BT stuff anyways, since you probably don't want the tick to run on each _process tick anyways. This also includes the renaming of the Status enum to something like BehaviourStatus and put it directly in the base class of all nodes. Giving the option to switch between IDLE and PHYSICS is a good idea!

I poked at Beehave and their master also has it now (althoug AssetLib version doesnt). I would probably implement it in similar way, although they don't provide delta in tick() so users while coding the leafs will have to guess which process the Tree is using.

I could be wrong, but I think Beehave also stores its delta on the blackboard.

ThePat02 commented 7 months ago

So in conclusion: Both BT and FSM need an export to toggle different process types and a way to tell them how often they should run. You can take a look at the branch releated to #3 where I started toying around with grouping Patterns together and have them called in alternating order. There needs to be a way to have them update on a lower rate to save on performance, as not all AIs/NPCs need super responsive behaviour.

ThePat02 commented 7 months ago

Could you open a seperate issue for the API changes? I think that is something interesting to discuss for a 2.0.0 release, while the selectable process should be be pretty easy to add. If you have any PRs in mind, feel free to add them too!

ThePat02 commented 7 months ago

@SirPigeonz take a look at #27 and tell me what you think of it. Is that what you had in mind?

ThePat02 commented 7 months ago

@SirPigeonz Thank you for this very useful contribution. I haven't put a lot of thought into this thing yet, as I was waiting until I really started implementing #3. I just merged your PR! I'd be happy to discuss further API changes for the 2.0.0 version in another issue. If anything is okay on your end, I'll happily close this one!