8000 Add state driven player controller using state chart addon by mrezai · Pull Request #409 · Phazorknight/Cogito · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add state driven player controller using state chart addon #409

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

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

Conversation

mrezai
Copy link
Contributor
@mrezai mrezai commented May 16, 2025

This is an experimental implementation of a state driven player controller using state chart addon.
I don't know this approach is acceptable or not but IMO it seems promising.
I started from platformer example in state chart project.
I made some modification in the Cogito code to simplify testing. Some of these changes will be removed later.
List of changes:

  • CogitoPlayer is now an empty base class.
  • CogitoPlayerDefault is current implementation and derived from CogitoPlayer.
  • CogitoPlayerStateDriven is a copy of CogitoPlayerDefault that refactored and modified to work using state chart.
  • COGITO_3_LobbyTestStateDrivenPlayer.tscn and COGITO_4_LaboratoryTestStateDrivenPlayer.tscn are copy of current demo scenes that their player replaced by CogitoPlayerStateDriven.
  • New game will start COGITO_3_LobbyTestStateDrivenPlayer.tscn.

Implementation details:
Screenshot from 2025-05-16 22-38-24

  • Root node of state chart is a ParallelState that has 2 CompoundStates(Moving and BodyPosture) and one AtomicState(PushingObjects).
  • States like Idle, Walking, Sprinting and Sliding are in Moving -> Grounded.
  • Jumping and Falling are in Moving -> Airborne.
  • I implemented Crouching, Standing and Sitting as BodyPostures, for example player can be in Idle state and at same time in one of Crouching or Standing state, but when player is in standing state and is moving, the final state will be walking, and if player is in crouching state and is moving, the final state will be sneaking.

@Phazorknight
Copy link
Owner

Gave this a very quick look.
I'm not super familiar with the stat charter, but seems pretty solid.

I've noticed that the states/transitions themselves all seem to still be their base class.
Is your next step in moving the actual player controller code into these states?

@mrezai
Copy link
Contributor Author
mrezai commented May 17, 2025

If you mean moving each state's code to a separate class, it's possible but it seems it isn't nessesary. You can find in this issue why this could even complicated things more.
We can organize current code by moving related code of each state to its region, I done this partially, for example walking state's code is like this:

Screenshot from 2025-05-17 07-38-35

and each method has its signal:

Screenshot from 2025-05-17 07-39-01

and states' code ordered based on their node's position in the scene tree too:

Screenshot from 2025-05-17 07-53-23

I think code traversing is fast enough, you can select a state, open its signals and open its code by double clicking on one of its signal but if your question is more about separation of concerns, I haven't a strong opinion about it.

@Phazorknight
Copy link
Owner

If you mean moving each state's code to a separate class, it's possible but it seems it isn't nessesary.

Thanks for linking that issue and describing your approach.
It makes total sense to me, and is one of the things I ran into when exploring state machines in general: It quickly becomes this debate about "does it make sense to separate out everything" and then you get exponential transitions etc.

We can organize current code by moving related code of each state to its region,

This is probably my preferred approach as well. I'm fine with keeping more logic in one script, as long as it's human-readable. 😃
A big issue that makes the current one unwieldy is its structure, leaning heavily on bools / condition checks. Moving this into state charts, while keeping the actual processes inside the script is probably a good approach to improve this.

Open to hear other approaches/considerations etc.

@mrezai
Copy link
Contributor Author
mrezai commented May 19, 2025

I refactored code by moving all states related codes to their regions, for example sittable functions and variables moved to sitting state region.
All states are in state chart region and in this region, shared variables are at the beginning of the code block and next are shard functions.
@Phazorknight what would be the next step?
There are some parts of code that I'm not sure about their correctness, I will list them here to discuss about them.

@Phazorknight Phazorknight added the enhancement New feature or request label May 19, 2025
@Phazorknight
Copy link
Owner
Phazorknight commented May 19, 2025

I refactored code by moving all states related codes to their regions, for example sittable functions and variables moved to sitting state region.

Nice, yeah, the cleanup in this definitely helps.

what would be the next step?

@mrezai Depends on what behaviour / improvement you are aiming for with this PR?

I think just having the same behavior and functionality as the old/main character controller is probably a good start - and good enough for me.

For extra features I'd love to see some of the "interaction states" tracked in the state chart as well, so for example if the player is

  • currently wielding a wieldable
  • carrying a carryable object like a box
  • interacting with an interface (like the keypad or readable)

I've used the current state of this PR to see if I can use the state signals with an animation tree to give the player character a shadow, and it worked pretty well:

Cogito_PlayerAnimationShadows_2025-05-19.mp4

@mrezai
Copy link
Contributor Author
mrezai commented May 20, 2025

Depends on what behaviour / improvement you are aiming for with this PR?
I think just having the same behavior and functionality as the old/main character controller is probably a good start - and good enough for me.

I think we could do things in this order:

  • Fix bugs of the player controller. I don't know which one is better, fix bugs in the old character controller first then port them to new implementation, or only fix them directly in new one and deprecate the old one?
  • Refactor code where is_showing_ui and is_movement_paused are used. The way how these boolean used in old implementation seems inconsistent to me. Do all states needs them or only some specific ones?
  • Add new states like "interaction states" that you mentioned and two other states that I think they will be very useful: ledge climbing and swimming.

I've used the current state of this PR to see if I can use the state signals with an animation tree to give the player character a shadow, and it worked pretty well:

Nice, I consider this as a sign of that we are on the right track and there is hope at least for a while, until we find a better approach later!

@Phazorknight
Copy link
Owner
Phazorknight commented May 20, 2025
< 8000 tr class="d-block">

Fully onboard with this approach.

I don't know which one is better, fix bugs in the old character controller first then port them to new implementation, or only fix them directly in new one and deprecate the old one?

Let's work on them in the new one in this PR.

Refactor code where is_showing_ui and is_movement_paused are used. The way how these boolean used in old implementation seems inconsistent to me. Do all states needs them or only some specific ones?

Agree. I'm not 100% sure anymore what my approach was with these, but I think there were mainly for input control.

There's two specific states or checks that are needed:

  • One that blocks all player controller input for stuff like cutscenes or scene-transitions. (eg. an is_controllable bool that's usually on, and if not, no input goes through, including no way to open pause menu etc).

  • One that just blocks player movement, but keeps interaction keys active - to use for graphical interfaces, containers, keypads, inventories etc. That's what the is_showing_ui was mostly for (this was also used to overwrite certain inputs to close those interfaces instead of opening the pause menu).

Any GUI scene usually will have its own input processing, but can/should check the player controller for a state to make sure they don't conflict or cause unwanted behaviour, so they will need to check for either these states or bools.

Add new states like "interaction states" that you mentioned

This would tie in to what I mentioned above: We could give the player states a flag that gives devs control over when and what the player can interact with while in certain states (for example: stopping the player from using a keypad while falling)

ledge climbing and swimming

absolutely agree!
The mentioning of swimming reminds me of this old PR that was never merged that had a decent implementation of swimming/water and even implemented buyant objects - #246

@mrezai
Copy link
Contributor Author
mrezai commented May 21, 2025

Thank you for the comprehensive explanation.
I will add a list of bugs to the first comment for better tracking and when they are fixed, I will continue the work on other things.

Phazorknight and others added 2 commits May 21, 2025 07:21
Cogito Player State Driven:
- Added an Animation Player and Animation Tree
- Created a script that uses signals from the state chart to update player animations.
- Early implementation
@mrezai
Copy link
Contributor Author
mrezai commented May 26, 2025

Changes in d69ce7c:

  • Fall damage was working only when there was no input_direction, now it works all the time.
  • Fall damage was constant but now if player falls on the ground with higher velocity, gets more damage.
  • Rolling state added and camera control is restricted like sliding.
  • Fixed collisions were enabled/disabled incorrectly in rolling sate.
  • It was possible to jump in the middle of rolling, now it isn't.

I want to separate states in airborne to these three states:

  • AirControl, player can control character in air when fall velocity is low. Player can grab ladders(and ledges) in this state.
  • Jumping, it's like the above state.
  • FreeFall, player can't control character in air when fall velocity is more than a defined threshold. Player can't grab ladders in this state.

@Phazorknight what do you think?

@Phazorknight
Copy link
Owner

Great updates, thanks!

I want to separate states in airborne to these three states:

  • AirControl, player can control character in air when fall velocity is low. Player can grab ladders(and ledges) in this state.
  • Jumping, it's like the above state.
  • FreeFall, player can't control character in air when fall velocity is more than a defined threshold. Player can't grab ladders in this state.

I think this makes total sense and seems intuitive.
Especially for FreeFall, as a dev I'd want control over when the player would enter this state, so exposing the velocity threshold would be great, as would having an option to set it in a way that the player would never enter it in case devs want to not have the player lose control.

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
0