Introduce Fleeing behavior on AI Commander

eh-jogos commented 1 year ago

This should finish the last item remaining from #8 and touch on some of the items for #16

It might also be a good first implementation so that we can discuss if this makes sense to you as a system for #16 or what needs to be changed for how you'd prefer to work.

Implementation Overview

setting navigation agent's target_position has been moved to PURSUIT state (old moving)
ai_commander now has it's own function in factory, so that it has a state machine with the extra FLEEING state
FLEEING was added and sets as a target_position a point a set distance away from the target, and moves there.
Other states have been extended so that they have additional logic to handle low health values:
- IDLING prioritizes running away or casting long range attacks when health is low. It will also cast in place rather than move into range because...
- ...when health is low CASTING will wait in place if the target is not in range for the ranged attack, so it will take more of a hit and run strategy
- ATTACKING will change to FLEEING only after it finishes its attack
- PURSUING will be immediately interrupted and changed to FLEEING on low health.

Limitations and/or Questions

Am I using the factory script/pattern correctly?

I don't have any experience with this kind of approach, but for every actor that has a different state machine, with additional behaviors that might or might not be on the base actor, we will have to create two new functions in factory, is this expected? Is this how it's supposed to work? Or do you have any suggestion to improve this aspect?

I guess another approach we could take, would be to create scenes for different state_machines, already set up with their states, and just load those scenes. This way we could just add them by passing the state machine scene path and avoiding this:

`update_state()` function

One thing I noticed while working on the state machine last week, but forgot to ask about or change is the update_state function. I get that semantically, the difference between update_state and physics_process is that one is for logic that can change the "state" of the state, while the other is the state processing what it has to "do".

But in the end, both are called during the engine's _physics_process. And I guess what bothers me the most is that update_state is called from Actor's _physics_process while physics_process is called from the state_machine's _physics_process:

I think this is something both of use overlooked when we did the state_machine refactor. Or was there a specific reason I'm not seeing for doing it this way?

Would it be alright to change it to this? (inside the state machine script)

func _physics_process(delta: float) -> void:
    current_state.update_state()
    current_state.physics_process(delta)

This way we know that update_state() always happens before or after the state processes. Maybe even return a boolean from it so that if the state has been changed, it won't still execute a last physics_process.

eh-jogos commented 1 year ago

forgot to add a video to the PR

https://github.com/Snayff/nqp3/assets/13070158/49e35ef3-a0ba-4775-8569-0f8f8248e178

Snayff commented 1 year ago

Am I using the factory script/pattern correctly?

We're probably using something between the factory and builder patterns tbh, which maybe doesnt help. Essentially, the factory should handle creation of objects for us, so that we can remove any issues of hierarchy or object instantiation.

In the example you shared, the only thing that should change is the list of states being passed to the the state machine. Under ref data I'd perhaps create a dict for used_states and then call that based on the actor type, e.g. player_commander, ai_commander etc., returning the list of states we need. 1 method for creating the state machine should be plenty. Does that make sense?

update_state() function

You're proposed change makes perfect sense. We might also want to change the phrasing, as update_state does not clearly explain what the method does.

eh-jogos commented 1 year ago

Just updated the PR with the state machine changes and the change to Factory and ref data you suggested in your comment! Also rebased it to fix merge conflics!

Snayff / nqp3