-
Notifications
You must be signed in to change notification settings - Fork 147
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
base: main
Are you sure you want to change the base?
Conversation
Gave this a very quick look. I've noticed that the states/transitions themselves all seem to still be their base class. |
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. and each method has its signal: and states' code ordered based on their node's position in the scene tree too: 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. |
Thanks for linking that issue and describing your approach.
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. 😃 Open to hear other approaches/considerations etc. |
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.
@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
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 |
I think we could do things in this order:
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! |
Fully onboard with this approach.
Let's work on them in the new one in this PR.
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:
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.
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)
absolutely agree! |
Thank you for the comprehensive explanation. |
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
Changes in d69ce7c:
I want to separate states in airborne to these three states:
@Phazorknight what do you think? |
Great updates, thanks!
I think this makes total sense and seems intuitive. |
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 fromCogitoPlayer
.CogitoPlayerStateDriven
is a copy ofCogitoPlayerDefault
that refactored and modified to work using state chart.COGITO_3_LobbyTestStateDrivenPlayer.tscn
andCOGITO_4_LaboratoryTestStateDrivenPlayer.tscn
are copy of current demo scenes that their player replaced byCogitoPlayerStateDriven
.COGITO_3_LobbyTestStateDrivenPlayer.tscn
.Implementation details:
