ThePat02 / BehaviourToolkit

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

API Change: Provide frame `delta` as a method perameter in `tick()` and `_on_update()` #31

Closed SirPigeonz closed 1 year ago

SirPigeonz commented 1 year ago

For now BTree adds it's frame delta to a Blackboard to be accessed in a extended BTLeaf script. FSM for now doesn't provide frame delta but it's probably a bug :)

Not sure if it have any advantage over providing delta as parameter, it's quite cheap to add to a Blackboard but still slower than direct parameter and less cluncy to retrieve and delta between frames is very common thing to use. Only reson to store it in Blackboard would be to share it with other system, but BTree and FSM would update it every frame either way so this is not needed and I don't think anybody would want to change it in the middle of evaluating BTree or FSM. That would be a silly thing to do :D

Providing it directly in the method call property would be also much more intuitive for the user, and less stuff to learn from documentation for them.

I was thinking about changing API for tick() and _on_update() to:

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

Other thing maybe unrelated to this proposal (I can make new one if you wish). Shouldn't tick() be _tick()?

ThePat02 commented 1 year ago

The main reason for adding delta to the blackboard was the ability to call tick() without needing to pass a delta parameter if you want to test behaviour or have some custom and very specified ways to use the leaves/states. I'll look into it and research how other plugins/design patterns solve this.

Alternatively I could expose delta with something like blackboard.get_delta() for easier access.

SirPigeonz commented 1 year ago

The main reason for adding delta to the blackboard was the ability to call tick() without needing to pass a delta parameter if you want to test behaviour or have some custom and very specified ways to use the leaves/states. I'll look into it and research how other plugins/design patterns solve this.

Ah, ok I understand (I think). Althugh we could give _delta default value of 0.0 then you can run tick() without specyfing delta? Would that solve the issue?

Alternatively I could expose delta with something like blackboard.get_delta() for easier access.

Yup, that would be a nice quality of life feature, if we decide to not change current API.

BTW I want to use BT not only for AI but also for my skill system, thats why I'm interested in accessing delta often. If needed I can always use Node.get_physics_process_delta_time() because I know I will be runnin my skill system on PHYSICS frame.

ThePat02 commented 1 year ago

I just looked through the code and noticed that I don't even set delta for the FSM. Also I am quite unsure how this would interact with shared blackboards.

ThePat02 commented 1 year ago

After some consideration just passing delta is probably a good idea!

SirPigeonz commented 1 year ago

@ThePat02 Cool, thx for looking into this! :)

I can work on it today or tomorrow and submit a PR, but let me know if you want to do it yourself for some rason or I should wait for something before working on this :)

ThePat02 commented 1 year ago

@SirPigeonz Well, I probably will do some work tonight and rewrite some of the old code. I'll @ you here when things are stable enough for this change :) hopefully after tonight haha

ThePat02 commented 1 year ago

When this is completed, I will finally start working on testcase #15!

SirPigeonz commented 1 year ago

Cool waiting for the green light from you then :)

ThePat02 commented 1 year ago

Cool waiting for the green light from you then :)

Well actually you can start whenever you want. Doesn't seem like I will get much work done today!

SirPigeonz commented 1 year ago

I will work on it tomorrow then :)

Just got message from a client for job so I will be less available comming 2-3 weeks, but I think I will manage to finish it tomorrow or the next day.

ThePat02 commented 1 year ago

I will work on it tomorrow then :)

No need to worry. I am pretty happy with the current shape the plugin is in, so I don't think there is any priority on releasing anything right now! Thank you for working with me here!

SirPigeonz commented 1 year ago

I wanted to work on it today opened code and decided to wait for #41 it touches the same lines and resolving so many conflicts would be error prone and unpleasant :)

Time to test #41 instead! ;)

ThePat02 commented 1 year ago

Time to test https://github.com/ThePat02/BehaviourToolkit/pull/41 instead! ;)

Let me know if you find anything! It should be fine tho!

SirPigeonz commented 1 year ago

45 merged :) closing.