Skip to content

Conversation

@mrezai
Copy link
Collaborator

@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
Collaborator 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
Collaborator 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
Collaborator 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

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
Collaborator 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.

@mrezai
Copy link
Collaborator 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.

@mrezai
Copy link
Collaborator Author

mrezai commented Jun 27, 2025

@Phazorknight AirControl and FreeFall states added, please review, thanks.

@lordsologamedev
Copy link

lordsologamedev commented Jun 27, 2025

Hello i just wanted to ask i am currently using my own node based system and i want a way for my player to transition between scenes , i think i need to set the physics process to false in order for my player character to properly switch to the nextscene any advice?
statemachine

@Phazorknight
Copy link
Owner

Hello i just wanted to ask i am currently using my own node based system and i want a way for my player to transition between scene

@lordsologamedev Please create an issue - this repo is not the place to bring this up.

@mrezai will try to review over the weekend, thanks!

@Phazorknight
Copy link
Owner

@mrezai I think this is pretty good. These two states seem to work as I'd imagine.

Going back to having other scripts/objects check for certain player states:
As I understand this, directly checking for a certain state should be avoided and is bad form. I can see why, as this would make adding/removing states very complicated and increase spaghetti code.

As a start to think about how to tackle this, I looked into when I mentioned GUI Handling:

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.

The opening/closing input handling of the Pause menu and Inventory screen is actually processed in the cogito_player script.
I think this would be a great starting point on how to tie the states into condition checks here.

My idea would be to be the following - but it might be a bit convoluted. As you're more familiar with the state chart addon, let me know your thoughts.

  • On CogitoPlayer, create a new exported node array called something like no_pause_menu_states , this would be a list of state nodes the dev can set, in which if the player is currently in, they can't open the pause menu (for example so you can't save while falling to your death)
  • On ready or set_state methods, a method would be called that connects to all the enter/exit signals of the nodes in theno_pause_menu_states to the corresponding methods that set a bool which allows/disallows the open of the pause menu
  • When running the game, the pause menu then can only be opened when in a state thats NOT listed in the no_pause_menu_states array.

@mrezai
Copy link
Collaborator Author

mrezai commented Jul 1, 2025

I like this idea but before any implementation, I want check some features of statechart like guard and condition state, maybe we can implement it in a more idiomatic way.

@mrezai
Copy link
Collaborator Author

mrezai commented Jul 6, 2025

@Phazorknight the latest changes:

  • disable_interaction_in_air added to InteractionComponent and when is checked, player can't interact with objects like keypad.
  • no_pause_menu_states array and is_player_in_unpausable_state flag added.
  • godot-statecharts updated to version 0.22.0.

I'll work on statechart saving/loading.

@Phazorknight
Copy link
Owner

@mrezai Still looks pretty good!

no_pause_menu_states array and is_player_in_unpausable_state flag added.

amazing, didn't expect this to be added so quickly.

disable_interaction_in_air added to InteractionComponent and when is checked, player can't interact with objects like keypad.

thanks for this! Works great, though I have to admit I'd implemented this differently.
I'd make this work the same way the no_pause_menu_states works, basically adding another array for no_interaction_states which would disallow opening interfaces while in it. That way, if the states change/get modified, not every interactive object needs to be update and devs don't have to go into a script to edit/add to.
Might be worth considering if we include opening the inventory screen to this array or make a third array for it. 🤔

I'll work on statechart saving/loading.

Sounds good. A third thing to consider would be to disallow saving the game in certain states (to avoid enabling the player to save the game right before falling to their death). Of course if we simply disallow opening the menu, then players can't save either way - that should ideally make this a bit easier.

@mrezai mrezai force-pushed the state-chart-based-cogito-player branch from 433fa44 to 04d8d45 Compare July 14, 2025 05:31
@mrezai
Copy link
Collaborator Author

mrezai commented Jul 14, 2025

@Phazorknight I rebased this branch on main to include latest changes, you need to get it again.
Last commit:
Add statechart saving/loading and other related improvements:

  • Sync state driven demo scenes with main branch
  • Add no_interaction_states
  • Replace some state flags in player code with enums
  • Set better values for player's parameters like fall damage
  • Add helper function for crouching and sprinting
  • Update loading/saving code based on above changes

I've added no_interaction_states that prevents all interaction in its states but didn't remove disable_interaction_in_air because for example in jumping state I want to interact with all interactables except keypad and it seems this is the simplest way to do it.

Might be worth considering if we include opening the inventory screen to this array or make a third array for it.

This needs more thinking!

@mrezai
Copy link
Collaborator Author

mrezai commented Jul 24, 2025

Ledge climbing added, it needs some polish later:

2025-07-24-1.mp4

@mrezai
Copy link
Collaborator Author

mrezai commented Aug 8, 2025

Swimming added + some refactoring, still some refactoring and bug fixing needed.
@Phazorknight If you have some sounds for swimming, please add them, thanks.

2025-08-08.18-43-31.mp4

@Phazorknight
Copy link
Owner

@mrezai this looks awesme and very promising.

I've added some water sounds I found in c99e0d2, feel free to grab those.

Correct me if I'm wrong, but looks like this is close to being ready?

@mrezai
Copy link
Collaborator Author

mrezai commented Aug 13, 2025

Thanks

Correct me if I'm wrong, but looks like this is close to being ready?

You are right, it might be ready in two weeks but I think it would be a good idea if someone more familiar with statechart review current implementation, mostly to prevent wrong usage of the framework, and I hope @derkork could help us in this regard if he has time and interested in this project.

mrezai and others added 4 commits August 23, 2025 09:34
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
@Phazorknight
Copy link
Owner

Hey, I was quite busy last week but looks like you've done a lot of work on this. Will take a look at this the next few days.

If animations added in commit 3618352 are from Mixamo, we must remove them because based on its license, it can't be used in frameworks even if they are open source.

They are not from Mixamo. I'm using the same as the NPC animations, which are listed in the Cogito Credits and are CC0, so no need to remove.

I think activation of StateChartDebugger should be added o CogitoSettings.

Makes sense, shouldn't be too tricky to add.

@mrezai
Copy link
Collaborator Author

mrezai commented Sep 2, 2025

I think activation of StateChartDebugger should be added o CogitoSettings.

Makes sense, shouldn't be too tricky to add.

I added this in 9403e51 but there might be other changes required.

@Phazorknight
Copy link
Owner

Hey mrezai,
I tried to pull the updates in this PR but git flagged some merge conflicts from your branch somehow. I've selected to keep the files from PR 409, but that might've been the wrong option as I don't see the new/added areas in the Laboratory demo scene.

If you could add a new push/commit so I could retry, that'd be great. Sorry for the inconvenience!

@mrezai
Copy link
Collaborator Author

mrezai commented Sep 3, 2025

This branch is rebased onto the main to be in sync with it and this is the reason of the merge conflicts.
The simplest way is remove/rename your local branch and pull it again, or using one of these methods: https://stackoverflow.com/questions/1125968/how-do-i-force-git-pull-to-overwrite-local-files

@Phazorknight
Copy link
Owner

Thanks, was able to fix the branch issue on my end and I think I was able to pull the latest.
This is pretty cool! Some notes:

  • Mid-air control seems to be way too high on default. I was able to instantly change directions mid-jump.

  • Ledge-climb seems to have very long reach, I see this is an exposed property so I'll try to play around with this. Otherwise it seems to work as intended. Love that the ledge climb works climbing into a crouch-state.

  • Swimming feels pretty good, would be nice to be able to rise/sink with Space/Crouch buttons, but this is overall very good so far.

  • I'd consider adding the water gravity coefficent to the water_area so it can affect other rigidbodies. Players will toss items or carryables into the water and it feels off that they seem to not get affected by the water.

Might be interesting to look into this old PR that also had water implementation: #246

@mrezai
Copy link
Collaborator Author

mrezai commented Sep 4, 2025

Thanks for feedback, I'll take a look as soon as I can.
I tested #246 and it has some issues. It uses godot-floatable-body but I don't know how feasible integrating it would be in the current implementation.

@mrezai
Copy link
Collaborator Author

mrezai commented Sep 4, 2025

@Phazorknight first and second issues are fixed.
Set ARM_LENGTH to smaller values to test first one.

…don:

- Add in water floating feature to the CogitoObject and CogitoProjectile.
- WaterArea extends FluidArea3D.
- Rifle mass set to 3.5.
- One CarrayableBox added to swimming test area.
@mrezai
Copy link
Collaborator Author

mrezai commented Sep 10, 2025

I added godot-floatable-body to project and based on it, added some properties to CogitoObjects.
Result:

2025-09-10.mp4

@Phazorknight Phazorknight self-requested a review October 1, 2025 16:44
@Phazorknight
Copy link
Owner

Sorry for the long delay but I finally got some time to look after COGITO more.

Swimming feels great and the floatable bodies seem to work very well too.
I'm always a bit apprehensive when adding external addons but this one seems pretty straightfoward and compact.

Mid-air control feels so much better now as well. I assume you're pretty close with this being ready to merge?

@mrezai
Copy link
Collaborator Author

mrezai commented Oct 2, 2025

No worries at all!

I think it might be better to release version 1.2 without merging this feature and after that I can rebase this onto the version 1.2 to be usable for brave early adopters!
Then we can merge it into the master branch, remove CogitoPlayerDefault, add breaking features and fixes like safe saving/loading, then release Cogito version 2.

My reasons for the above suggestion:

IMO merging this branch and other breaking changes into new major version is better idea than merging it into version 1.2.

@Phazorknight
Copy link
Owner

@mrezai Yeah sounds good. Your reasoning makes sense to me.

Then we can merge it into the master branch, remove CogitoPlayerDefault, add breaking features and fixes like safe saving/loading, then release Cogito version 2.

Might try to make it version 1.3 first 😇
I'll also see how updating to Godot 4.5 looks at the moment.

There are other things in Godot 4.5 like godotengine/godot#93142 that seems useful.

thanks for pointing this out! I hadn't seen this before and this seems like it'd be super useful for Cogito. Will take a look.

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.

3 participants