Search Unity

C# Proper State Machine

Discussion in 'Scripting' started by Rodlaiz, Jan 19, 2016.

  1. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    A class can inherit multiple interfaces but not multiple classes - that's the biggest functional difference between the 2 approaches. Generally, you'd want to use an abstract class when you're actually defining some level of common behavior. An abstract class full of abstract methods should probably just be an interface since the purpose of an interface is to define the contractual obligations of the implementation.

    One could argue that they function roughly the same in this case but intent matters when other people are reading your code (and "other people" will also include you 3 years from now when you don't have all the context in your head anymore). If your intent is "anything passed here must implement these methods" then you should use an interface because that's what an interface is intended for.
     
    petey and mrCharli3 like this.
  2. PudgeCake

    PudgeCake

    Joined:
    Aug 8, 2019
    Posts:
    4
    Going back to the original design shown by Kelso. If you don't mind indulging in yet another question about this.
    What's the best way to handle communication between the Unit and the current active state?

    Say your Unit, while walking, picks up a speed boost and now the walking state needs to go faster. Do you have the state operate entirely independently from the Unit and have it detect the collision itself? Do you have the Unit pick up the event, and then relay it to the state via the statemachine ( the statemachine needs pass-through methods for everything). Or do you give the Unit a reference to the currentState from the statemachine and then pass everything through there.

    I'm trying to get it clear in my head before I start: I've got a situation where the "unit" can receive a lot of interactions from the rest of the game, and they all play out differently depending on what state the unit is in. Each state needs it's own logic for which state to transition to next (each state likely using different information to make this determination), and to be able to respond to and interact with other elements in the world.

    edit - something else just occurred to me:
    Couldn't all of the logic in the StateMachine class be contained by the Unit itself? I'm not sure what the StateMachine class is adding.
     
    Last edited: Sep 4, 2022
  3. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    You're talking about events that ultimately change data which is something else entirely. There are a lot of options, the easiest of which is to just keep the speed as a member of the unit and have the state access it
    Code (csharp):
    1.  
    2. public class Unit
    3. {
    4.     private int speed;
    5.    
    6.     public int GetCurrentSpeed()
    7.     {
    8.         return speed + GetSpeedBonuses();
    9.     }
    10. }
    11.  
    12. public class MoveState
    13. {
    14.     private readonly Unit owner;
    15.  
    16.     public MoveState(Unit owner)
    17.     {
    18.          this.owner = owner;
    19.     }
    20.  
    21.     public void Execute()
    22.     {
    23.         owner.transform.position += owner.transform.forward * owner.GetCurrentSpeed() * Time.deltaTime;
    24.     }
    25. }
    26.  
    Your approach will depend on your specific needs.

    It could, if you weren't re-using your state machine for other systems (we did, as I mentioned previously). However, even if you weren't, I'd argue that you're now making your Unit class responsible for more than 1 thing which I would disagree with from a design perspective.
     
    petey likes this.
  4. Zajoman

    Zajoman

    Joined:
    May 31, 2014
    Posts:
    15
    I strongly suggest to make it as simple as possible. No classes, no interfaces, no needless OOP.

    Just use an enum and switch over it. You really don't need any of that boilerplate fluff. "Better code reusability", "it should only do one thing", "easier to debug" are all nice, fancy phrases, but in practice, they mean nothing.

    It's WAY more straightforward to simply write this

    Code (CSharp):
    1. switch (state)
    2. {
    3.     case State.Idle:
    4.         Idle();
    5.         break;
    6.     case State.Chasing:
    7.         Chase();
    8.         break;
    9.     case State.Attacking:
    10.         Attack();
    11.         break;
    12. }
    than to construct OOP castles in the clouds for no gain whatsoever.

    You reuse code by calling the same function from multiple places. For instance, if the monster is idling, you probably want it to check for enemies (most likely the player) by sight, or maybe by hearing, or whatever.

    When the monster is chasing the target, you might still want it to make periodic sight checks to see if it can still see the target or it has lost it. If it hasn't seen the target for too long, maybe it's time for the monster to give up the chase and go idle again.

    You just call the same function from multiple "states". Bam, code reused. If the context is different based on the state or some unique conditions, just parametrize the function so you get expected results.

    As for debugging, I can't picture a simpler scenario than this to debug. I don't see a single reason why a large structure of classes should be any easier to debug than a single one.

    If you have way too many states and switching would become cumbersome, use function pointers. So instead of directly calling different functions based on an enum, you always call one delegate and you just change where it points whenever the state changes.

    The thing is, if the logic of your game needs to be complex, no amount of boilerplate abstractions will save you from it. Games are complex and so is the code. You don't need any more code than that which simply does exactly what needs to be done.

    "It should only do one thing..." Like, what? A typical monster in a game does a whole LOT of things. Why would you put these arbitrary constraints on yourself? The code that drives the monster should implement exactly as much logic as the monster needs to perform. Period.

    Another benefit of this approach is that your code is in a single place, more or less, not spread throughout tens of classes, hidden behind layers of abstractions and whatnot.

    As for the speed boost example, I suggest your "state machine" knows EVERYTHING about the monster (or unit or whatever it is). It should have access to everything and therefore very simply reason about anything that matters to any particular state. This whole notion of hiding information from entity to entity and from system to system is nonsensical and will only hinder your ability to make things work.
     
    Last edited: Sep 6, 2022
    Kurt-Dekker likes this.
  5. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,742
    Zajoman, you are my hero... no needless OOP. If I had an Internet to give, I would award it to you directly.

    It is sad how much time is wasted trying to solve brain-dead-simple problems by first trying to generalize or abstract them. In the end the problem is solved poorly (or as in this case not at all), and the codebase suffers.

    My earlier take:

    https://forum.unity.com/threads/c-proper-state-machine.380612/page-2#post-8127749
     
  6. R1PFake

    R1PFake

    Joined:
    Aug 7, 2015
    Posts:
    542
    Enum switch is fine for simple cases, but in more "advance" case you have more logic like state change (enter, exit) methods.

    Of course you can put all of that in your one class, you can write your whole game logic in one class if you want, but I rather have a clean structure where every state logic is in a own file instead of mixing it all in one big class.

    Navigation between different state classes is not an issue with a proper IDE.

    The state implementation isn't even much "boilerplate" because you would need the same methods anyways, you just move them to a different class.

    You don't have to be sad that "much time is wasted" because for an experienced dev the state machine pattern is just as easy to use and debug as the enum version.

    The only people who ask questions here are beginners who never used it before and try to learn. The orginal OP already used enum switches and was asking for feedback, so there is no reason to "gatekeep" other solutions, just because you personally don't like OOP.

    It's not wasted time if they try to improve by learning something new, so in the future they will have enough experience to know in which case they should use the enum switch and in which case they rather use a proper state machine or maybe even something more complex (like a behavior tree, if their next game requires such logic).

    Btw claming that the state pattern solves it "poorly or not at all" is just plain wrong, because this pattern can achieve everything the enum solution could achiev and more.

    Simple code is always preferred, but in some cases "complex" code is better, because of the overal benefits. A important skill of a good developer is to balance these things and to know when the "complex" code is better, because neither of these solutions is the "best" in all cases. And the easiest way to get that skill is to just try new things and get new experience.
     
    Last edited: Sep 6, 2022
    petey and KelsoMRK like this.
  7. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    Did you read the thread? The OP did this and asked for feedback on it.

    I've provided the justification for why we went with this approach multiple times in this thread, so I won't re-hash again.
    https://forum.unity.com/threads/c-proper-state-machine.380612/#post-2473004
    https://forum.unity.com/threads/c-proper-state-machine.380612/#post-4880189
    https://forum.unity.com/threads/c-proper-state-machine.380612/#post-5126150
    https://forum.unity.com/threads/c-proper-state-machine.380612/#post-5127134
    https://forum.unity.com/threads/c-proper-state-machine.380612/#post-5787187

    Couldn't disagree more. You spend far more time reading code than you do writing it, why would you make it harder on yourself? What you think you save on the front end you'll lose on the back end tracking down issues.

    I disagreed then and I disagree now :) We shipped a complex game with that FSM. It worked amazingly well. An enum switch would have been an absolute nightmare.

    With that said - and I'm reiterating what has been debated here multiple times: If your use case is not complex, you don't have to do this. I have never once said this is the magic bullet of solutions.
     
    Aethenosity, Kurt-Dekker and R1PFake like this.
  8. Zajoman

    Zajoman

    Joined:
    May 31, 2014
    Posts:
    15
    Sure, you still have to write the same logic in the end, but instead of adding one value to an enum and writing one new function, you create a whole new class with all this in it. I guess it's personal taste. To me, it's needless typing; to others, it may very well be just fine.

    Yes, right, and my feedback is that OP was doing it right in the first place. There was never any need to build a big structure over something so simple. Suggesting that something is good while something else is bad is not, in my opinion, "gatekeeping". It's up to OP to form an opinion, based on all the answers.

    Yes, right, and that's exactly what I did: I gave my feedback on it, while also providing a different viewpoint on things suggested by others (mainly you).

    Well, yes, you provided justification. And I disagree with it. It's not like by simply providing it makes it the only relevant approach, right?

    Nothing you wrote cannot also be achieved by simple enum-switch-style FSM. The class-based FSM doesn't have more capabilities or power or anything. The difference is the structure. And I say it's a needlessly complex structure.

    What I mean by that is that the claimed benefits simply aren't there. "Reusability" and "easy to read & debug" are by all means worthy goals, but they are not automatically achieved by creating needless structures over simple problems. And conversely, they are not automatically lost by writing straightforward code.

    We shipped rather complex games as well (https://store.steampowered.com/app/629690/Vaporum/ and https://store.steampowered.com/app/1161880/Vaporum_Lockdown/) with a dead-simple FSM (in terms of structure) for all enemies. Some of them have unique behaviors, use various skills and movement patterns, yet we never had any need for anything but a simple switch.

    I picture in your game, you have many states or sub-states, perhaps your state structure is more granular, and I believe that you did well for your use case. In our games, we only had a few states and these were more complex, but that never detracted from readability or reusability.

    I don't dare claim that using a generic class-based FSM would have been an absolute nightmare for our project :D What I can claim with absolute confidence is that an enum-switch FSM can do everything a class-based can, without all the fluff.

    Well said.
     
  9. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    Never said it was the only relevant approach. Quite the opposite - you can find multiple instances in this thread of me suggesting other solutions, including switching over an enum. Also not sure how you can "disagree" with someone's experience? "Here's what worked really well for us in a complex use case". "I disagree!".....ok - it still worked really well for us? And I'm relaying this experience so maybe people avoid pitfalls? Not sure what you're getting at there.

    Never said it couldn't. The structure is just that, structure. When you have a higher degree of complexity (which can mean more complex states, lots of states, lots of variation, etc etc) then the structure this pattern provides can be a life saver.

    This is exactly my point. It.Depends.

    Then I would suggest not tossing out throwaway advice such as:
     
  10. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    Unrelated but - this is terrible advice. Don't do this. It might seem like a great idea until you're trying to track down why a boolean value gets flipped from true to false and that property is referenced in 100's of places. All the time you saved making everything accessible to everything is lost to 1 debugging session - and you will be debugging.
     
  11. Zajoman

    Zajoman

    Joined:
    May 31, 2014
    Posts:
    15
    I don't disagree with your experience. That makes no sense. I disagree with your justification for why you went with this approach.

    This sounds like you're defending what you said earlier, or trying to justify why you suggested your approach to OP. Seemed to me like you were using those earlier posts as argument ammo. I disagree with that; not with your experience that the FSM you used worked well for you.

    I don't see why not. It's a clearly better approach in my opinion, so yes, that is exactly my advice.
     
  12. Zajoman

    Zajoman

    Joined:
    May 31, 2014
    Posts:
    15
    I suppose this comes from experience, otherwise you wouldn't know this.

    In my experience over the years, the more we shift away from all the OOP religion like SOLID and other nonsense, the better off and more productive we are. Yes, bugs happen. In all codebases, no matter how meticulously constructed. Still, we are yet to bump into the issue you're describing here.

    I can't picture how a single bool could be referenced in HUNDREDS of places (maybe you're just exaggerating to make a point). In reality, it's a few at best, and it never takes more than a few minutes to track. On the other hand, typing all that getter/setter private virtual whatever stuff surely takes time and is tedious as hell. Moreover, you're writing a game, not a public API. You KNOW what you're doing. You don't need to safeguard yourself. If some boolean ain't supposed to be manipulated directly, you either simply know this, because you wrote it, or it will very quickly show up as a bug.

    I know this sounds dangerous, but it simply isn't in PRACTICE. That's the only thing that matters. Not some academic made up notions.
     
  13. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    I would prefer the code enforces this.

    The time it takes to create a new enum for a state and create a function to support it is essentially equal to the time it takes to create a class to represent that state The difference is, your switch statement is coupled to the encapsulating class. I appreciate being skeptical of SOLID and other OOP principles but one of the things it's very good at is limiting the blast radius of the changes you commit. If everything is accessible to everyone all the time and all your state logic is in 1 big class then any change to that class has the potential to affect any other state - and has the potential to affect those states in unpredictable ways.

    How well does this scale to multiple developers? Who's responsible for making sure everyone is aware of this institutional knowledge? How well does this work when you have to revisit the code 3 years from now and you've forgotten all the context that went into your decisions? Again - I'd rather the code show the intent. If you're not supposed to mutate a piece of data, the code should prevent you from doing so.

    If something sounds like a bad idea, it probably is.

    Again, I'm not terribly interested in re-litigating the things that have been said in this thread. Generally I've found that the folks who rail against this pattern haven't had to deal with a significant enough level of complexity to warrant it - which is fine! You don't have to make complex game mechanics! We did. This pattern helped immensely to mitigate that complexity - both in initial implementation but also in the years of subsequent changes, tweaks, and fixes we had to do. This is also not a pattern I made up out of thin air. It's literally based on a C++ one I read in a book on Game AI.

    It's unfortunate because I feel like this thread must be at the top of search results for Unity AI/FSM stuff (given the cadence at which it gets new responses) and I think newcomers are going to come in here and just get a lot of noise instead of actual value.
     
    R1PFake likes this.
  14. PudgeCake

    PudgeCake

    Joined:
    Aug 8, 2019
    Posts:
    4
    Thanks for coming back to this for my question Kelso. I didn't intend to kick-off the same arguments again, my apologies.
    Just for interest, I tried out a few different implementations and in the end decided on something quite different that I think best for my very specific use-case. I created a Prefab for each possible state and the statemachine swaps in and out the different prefabs as children, as required. The prefabs hold the different state classes, but can also carry all of the distinctly different components that the different states require. And gives the possibility of a Prefab holding a persistent memory since I don't destroy the prefab, but just deactivate it. The base "Unit" holds a persistent data struct to pass info between the Prefabs.
    I don't think this is going to be useful to most usecases. But thus far it's working quite nicely for mine.

    Once again, thank you for giving me a wider perspective to chew through my options.
     
    KelsoMRK likes this.
  15. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    No need to apologize - you didn't do anything wrong :)

    That does seem like an interesting approach. Glad you found something that works for you!
     
  16. Pvt_Hudson

    Pvt_Hudson

    Joined:
    Jan 20, 2016
    Posts:
    41
    I like the structure presented by KelsoMRK separating what is done for a given state.

    However, where does the actual "thinking" typically occur ? Do the states themselves handle the decision making for when they are complete or external factors dictate the need to transition to another state... or does the Unit essentially need a long switch statement handling each possible current state and when it needs to transition ?
     
  17. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    I've generally found that letting the states decide works the best, but it really depends on what works better for you. For instance - if you were doing an attack state, your Execute logic might look something like this:

    * Is my target alive? If not, transition back to an idle state.
    * Can I see my target? If I can't, transition back to an idle state.
    * Is my target in range? If not, transition to an attack move state where my destination is my target's current location.
    * Is the cooldown on my weapon fire rate ready? If not, do nothing and wait for the next frame.
    * If I can fire my weapon, attack the target (whatever that means in your game).

    And from a code perspective, you would just "early out" after any of those transitions
    Code (csharp):
    1.  
    2. if (!TargetIsAlive() || !CanSeeTarget())
    3. {
    4.     stateMachine.ChangeState(new IdleState(owner));
    5.     return;
    6. }
    7.  
    8. if (!TargetInRange())
    9. {
    10.     stateMachine.ChangeState(new AttackMoveState(owner, target));
    11.     return;
    12. }
    13.  
    14. // etc etc
    15.  
    If you're talking about the "next level up", as in - player AI versus individual unit AI - that's another subject as you're more into goal driven behavior and a bunch of other things.
     
    Pvt_Hudson likes this.
  18. devrandomzero

    devrandomzero

    Joined:
    Aug 2, 2020
    Posts:
    43
    Hi KelsoMRK,
    about this section of your FSM:
    Code (CSharp):
    1. void Start()
    2. {
    3.     stateMachine.ChangeState(new TestState(this));
    4. }
    I was wondering if caching every state could be helpful or not.

    Thank you

    Bye
     
  19. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    I could see it if you wanted to "resume" a state that had already been running, but that gets pretty tricky. Otherwise, no I don't think caching them achieves anything.
     
  20. devrandomzero

    devrandomzero

    Joined:
    Aug 2, 2020
    Posts:
    43
    Thank you, I was also thinking about garbage collection but maybe the benefits are negligible.
     
  21. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    If that's a concern in your case, you can make each state a singleton that processes each entity in that state in turn each frame. We actually did that originally in Fractured State but refactored away from it due to the amount of information we had to keep on the entity itself (since the singleton has to effectively be stateless).
     
    devrandomzero likes this.