Search Unity

  1. Click here to see what's on sale for the "Best of Super Sale" on the Asset Store
    Dismiss Notice
  2. Good news ✨ We have more Unite Now videos available for you to watch on-demand! Come check them out and ask our experts any questions!
    Dismiss Notice

Official Generic State Machine

Discussion in 'Open Projects' started by Dustin_00, Sep 29, 2020.

  1. cirocontinisio

    cirocontinisio

    Unity Technologies

    Joined:
    Jun 20, 2016
    Posts:
    444
    The wireframing is a discussion about the appearance of the whole UI, of which the inventory is only part of.
    As for the inventory, I'll reply in the other thread.
     
  2. cirocontinisio

    cirocontinisio

    Unity Technologies

    Joined:
    Jun 20, 2016
    Posts:
    444
    Habemus State Machine!

    I finally managed to merge one of the proposed solutions. We had 7 fully-fledged StateMachine PRs (plus a couple of others I closed previously)!! In the end, I landed on the very uninspiringly-titled "Yet Another State Machine" by @deivsky.

    In all fairness, all the solutions proposed were good in some way. Unfortunately they almost all rely very heavily on ScriptableObjects which means that, in spite of the idea of reusing code and making it designer-friendly, the reality is that you will be writing new classes for conditions and actions all the time, plus creating plenty of extra SOs to accommodate just slightly different behaviour or data, since all actions/conditions/transitions/states are intrinsically linked together. Oh well.
    The ones relying on MonoBehaviours though used plenty of GameObjects nested in a hierarchy, which in my opinion is not a great idea due to the initialisation and activation/deactivation costs.

    However, of all solutions the one I chose stood out for its easy-to-understand code and class organisation. So let's move forward with it, and we can keep tweaking it all together.
    To put it to the test, I remade the character controller with it. It took me a while, but I think I got back all the functionality as it was. It only generated 60 additional files :eek:

    Finally, I want to recognise all of you and will be keeping track of your state machines as if we merged your PR too. I will also be reaching out in the PR themselves for names so you can get the much coveted "OP Contributor" role on Discord, if you care.

    I hope you don't mind, I'll go on and close all the PRs.
     
    Eyap, Proryanator and kcastagnini like this.
  3. kcastagnini

    kcastagnini

    Joined:
    Dec 14, 2019
    Posts:
    46
    Thanks for your update, I can't wait to start contributing to this implementation!
    Kudos to @deivsky for building this system!

    Maybe now we can start discussing a more code oriented FSM for boring stuff that don't require the touch of game designers like a GameManager? :p
     
    deivsky likes this.
  4. cirocontinisio

    cirocontinisio

    Unity Technologies

    Joined:
    Jun 20, 2016
    Posts:
    444
    The reason why I chose the state machine I chose is because I want us to use it for anything. It's generic enough and it would do. Also, don't worry, it still requires a fair amount of coding to do anything :D
     
    Eyap and kcastagnini like this.
  5. kcastagnini

    kcastagnini

    Joined:
    Dec 14, 2019
    Posts:
    46
    Gotcha!
     
    cirocontinisio likes this.
  6. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    Hahah I didn't mean to sound uninspiring. In reality, I'm super excited with this project and very much inspired to contribute in any way I can. :)

    I'm really happy and thankful that we're using my implementation, but I'm also happy that we can now move forward. Now that we have a base, instead of each of us working on their own, we can all work together to improve on it, and other systems that were waiting on it can begin development. This means that there are now more opportunities to collaborate and contribute. ;)

    Thanks! And please, do contribute! I want to revisit the Transitions workflow and I think your solution with TransitionTables was the cleaner. I think if we go with it though, a whole EditorWindow would be needed, possibly grouping Transitions by "From State".

    I'm also currently exploring having the Transitions of a State be editable from within the same State inspector, so not just being able to add and remove Transitions, but also add and remove Conditions and setting the Target State of the Transitions all in one place.

    More ideas are welcomed. I think having a nice, easy-to-understand Editor is key, and I'm not too great with Editor scripting so I'm definitely gonna need help with that.
     
  7. invadererik

    invadererik

    Joined:
    Oct 31, 2010
    Posts:
    141
    here are some small things I notice so far:
    1) states cant have their own variables ( per state vars )
    2) I dislike these lines: "stateMachine.GetComponent<Character>();" because we need to fetch a component all the time and it ties it to that specific comp. Would be fancier if you could actually target specific variables instead, like name a variable and point it to another (ie connect a local to a shared one).
     
    cirocontinisio likes this.
  8. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    Hey, good points. Here's my reasoning:
    1) States don't have their own variables because they don't do anything on their own. Just like transitions, they don't have any logic besides routing the OnEnter, OnExit and OnUpdate callbacks to the actions/conditions. Actions and conditions, on the other hand, can have their own variables (see AscendActionSO.cs and IsMovingConditionSO.cs).
    2) I agree, maybe when we have injection we can think of a workaround. As of now, the easiest way to work with shared data is by using MonoBehaviours. Fetching the component is only done once, so it's inexpensive. I don't know how targeting specific variables would work, what if you have two components with the same variable?
     
    cirocontinisio likes this.
  9. cirocontinisio

    cirocontinisio

    Unity Technologies

    Joined:
    Jun 20, 2016
    Posts:
    444
    Is it too mad to think of a system using ScriptableObjects, where we have an SO for atomic data (Vector3, a bool, a float) and different objects point to it?
    In this case, for instance both the Character script and any condition/action could reference one Vector3 SO, some would write to it, some would just read to it. Obviously who writes what and when is all left to the user's implementation!

    However, I hardly see any use for this... I mean: 90% of the behaviour I wrote for the character's state machine is specific to the character. For instance, when it calculates the falling ratio for a jump, or the ascending speed.
    So why make it generic? It's not like you will have a state machine for a feather falling that you can reuse...

    I'd say let's keep this in mind, and move forward. We can still change the code down the line and introduce such a system (in fact it just takes a couple of SO scripts and change the type of references in the Actions).
     
  10. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    I think it would be a little mad because you would have to create an instance of each VariableSO for each instance of FSM. For example, you'd need a MovementVectorSO for everything that moves. And how do we make EnemyA point to EnemyAMovementVectorSO and EnemyB point to EnemyBMovementVectorSO, when both are using the same EnemyMoveActionSO?
     
  11. cirocontinisio

    cirocontinisio

    Unity Technologies

    Joined:
    Jun 20, 2016
    Posts:
    444
    That's a good point, I was clearly thinking of the player character, of which only one instance of the state machine exists.
     
  12. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    We could have a hub of ScriptableObjects in the StateMachine and get them by type but I think we already had this conversation :D
     
    cirocontinisio likes this.
  13. Zold2012

    Zold2012

    Joined:
    Feb 4, 2014
    Posts:
    67
    yeah this is really hard to tinker with compared to the initial protagonist controller and i feel really dumb for not paying attention to this thread. The fields to edit basic settings are all over the place. furthermore, the scriptable object fields don't seem to be editable during gameplay which is a straight downgrade from a basic prefab with a single character controller component.
    I've seen a talk where a studio did this and the designers got so used to it they thought vanilla unity was a downgrade when they tried to do stuff at home. useful for UI stuff, can't remember the talk for the life of me.
     
  14. Neonage

    Neonage

    Joined:
    May 22, 2020
    Posts:
    191
    Uh, oh.. I was so busy on other stuff that I stopped receiving notifications on this thread. :oops:

    Everything is easy with a nice editor:


    Create unique variables at runtime and point to them instead?


    20:30
     
    Zold2012 likes this.
  15. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    We can put everything into ScriptableObjects and feed that to the FSM, then it'd be all in one place and modifiable at runtime.

    If we're instantiating them at runtime why not just making them regular classes? Debugging comes to mind, I guess... But anyway, that can also be done.

    I think that the implementation is generic enough to allow for a fully SO-based workflow, as well as a MonoBehaviour workflow, and adding more features on top.

    Now, if we don't like how the currently-only-usage-of-it works, we can change that. Obviously, we can also change the FSM implementation, but I feel like we have to make the distinction between what's part of the implementation itself, and what's just simply the way we're using it.

    Totally. I'm currently attempting to make a StateMachineEditorWindow where we can add and remove actions, transitions and conditions. I've never really bothered to make Editor tools because I've only worked with myself, so it's probably not good, but this is what I've gotten so far:

    - Added a "Edit State Machine" button in the StateMachine component, that opens up the EditorWindow:
    upload_2020-10-27_0-21-46.png

    - The EditorWindow lists all the States in the StateMachine:
    upload_2020-10-27_0-22-39.png

    - When selecting a State, it lists all the actions, transitions and conditions in it:
    upload_2020-10-27_0-26-56.png

    - As of now, adding and removing actions and conditions works as expected. Adding and removing transitions is not enabled yet. Changing the target state of a transition sometimes causes things to break, as well as undoing.

    I'm trying to get it to work and then I'll think more about the layout, but feel free to bash me, or make your own version. Once I've sorted things out a little better, I'll submit a PR so whoever wants to can help me.
     
    kcastagnini, Eyap and Neonage like this.
  16. Proryanator

    Proryanator

    Joined:
    Sep 22, 2013
    Posts:
    29
    Don't mind at all, I loved this whole effort and thread!

    @deivsky yooou got it! :D I dropped off for a while but I'll take a look at your solution to see what I can learn!
     
    cirocontinisio, deivsky and Eyap like this.
  17. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    I've pushed a PR to change the StateTransitionSO to TransitionTableSO. More details in the description.

    It's just that, no Editor scripts yet. As I was working on the Editor I realised more and more how coupling the states and the transitions together was wrong, so I had to pause that and make the change. I'm gonna work on the Editor now based on that PR in the hopes that it gets merged. Go check it out!
     
    kcastagnini and Zak_Ipatov like this.
  18. laurentlavigne

    laurentlavigne

    Joined:
    Aug 16, 2012
    Posts:
    4,036
    I watched the Youtube. More than once did the OP click on the wrong thing or stumbled so at first glance it looks ripe for mistakes, just not on the programmer's side. From experience when you transfer logic to the editor you also transfer the potential for bugs to bad drag and drop. Cyclical graph are easier to grasp visually so I think that a visualization is needed.
    I won't participate in this open project because I have too much to do but I am very interested to see how this specific aspect evolves.
     
    cirocontinisio and Neonage like this.
  19. cirocontinisio

    cirocontinisio

    Unity Technologies

    Joined:
    Jun 20, 2016
    Posts:
    444
    Looks good to me, definitely an improvement!

    I do agree with you, but I think the key could be just in putting all the initial settings in one object. Probably a MonoBehaviour is a good fit? In fact, I already implemented the Character MB on the character GameObject to act as a "repository of shared data" between the Actions, maybe that's where the initial data also needs to live?
     
    kcastagnini likes this.
  20. kcastagnini

    kcastagnini

    Joined:
    Dec 14, 2019
    Posts:
    46
    I need to catch up, I have been very busy these days! Everything looks awesome!
     
    cirocontinisio likes this.
  21. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    I've pushed a commit to my last PR that adds a full editor for the transition table workflow. It allows to easily edit, add, remove and sort transitions, conditions, and states. Please try it out and tell me what you think.
     
  22. WagnerGFX

    WagnerGFX

    Joined:
    Aug 26, 2013
    Posts:
    27
    I've made a specialized debugger for the state machine that gives a more in-depth log of the state transition with the conditions results:

    ObjectName state changed
    PreviousState >>> NewState

    [TransitionName]
    > ConditionName == ResultBool [✔]
    > ConditionName == ResultBool [✘]


    • The first 2 lines will show in the log while the transition and conditions will only show in the lower detailed panel.
    • Added the previous state name to avoid any need to check previous logs.
    • The result is the raw boolean from the condition evaluation.
    • The check/uncheck mark at the end will tell if the condition was met by the transition logic.
    • The only current drawback is that some core classes need to hold references to the StateMachine and the SO of origin.
    With this more in-depth log is possible to check how the transitions are being triggered and if there are any condition issues, making future tests much easier to do.

    I would like to know if this is an interesting addition and if there are any other suggestions.

    My branch with these changes: fsm-debugger
    Now with a Pull Request
     
    Last edited: Oct 30, 2020
    Eyap, MUGIK, deivsky and 1 other person like this.
  23. javimtib92

    javimtib92

    Joined:
    Aug 15, 2020
    Posts:
    7
    Hello! I wanted to ask something, I've saw deivsky editor work and I think it's awesome and super useful but I've been thinking if we could go a step further and implement a generic graph editor view for the state machine.

    Maybe it's not a good idea because Graph View isn't very documented yet but I think we could try to do a pretty simple Graph View for this. For example I did some experiments (I'm very new to game development imagine to editor scripting)
    and I came up with this. I created 3 kinds of nodes, Conditions, State And Actions List. I'm not sure about actions list thought.

    If anyone is interested and wants to collaborate to continue with this then I can share my branch and we collaborate together there.

    My only concern is going to far with this and then tie together a lot our current implementation of the state machine with the editor. I'm open to suggestions :)

    upload_2020-10-30_11-17-46.png
     
    Eyap likes this.
  24. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    This is a great start, debugging capabilities are very needed, that's why I added that very simple log on the first iteration. I think we should continue to work on this.

    These are a couple of simple ideas:
    I was thinking maybe adding a Window/Analysis/State Machine Debugger to output there so we don't spam the console. It'd output the transitions of the selected GO with a StateMachine component, or it could have a list of all the active SMs with toggles to turn debugging on/of individually.
    To add a little bit of visualization without complicating things too much, we could have a section to display something like this updating live:
    Code (CSharp):
    1. Former State ---> Current State ---> Possible State
    2.                                 ---> Possible State
    3.                                 ---> Possible State
    As for the compatibility with the changes I made, the only problem I see is that there are no longer StateTransitionSOs, but just removing the name of the transition from the log would suffice.

    Wow, that's awesome! Having GraphView would be amazing, I doubt any editor could match the simplistic yet powerful nature of it. However, we'd have to see what @cirocontinisio says about this since it's an experimental feature.
     
  25. WagnerGFX

    WagnerGFX

    Joined:
    Aug 26, 2013
    Posts:
    27
    Those are some interesting ideas. But I think we need to get some feedback from designers and gameplay programmers (even if they are us) before to understand what we are solving.

    It might be better to wait a little bit further in development until we have some enemies using FSM, more complex transition tables and some complaints :D.

    Without some direction we could end up making something that the designer will only see as a supercomputer from the 60's blinking randomly.

    Thanks for the info. Then I will make the changes you mentioned in the PR so the code is compatible with your implementation.
     
    cirocontinisio likes this.
  26. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    Totally agree :D:D
     
  27. Neonage

    Neonage

    Joined:
    May 22, 2020
    Posts:
    191
    It already looks like a supercomputer from the 60's for me. :p
    Graph is essential for such tool, not counting how many times I've said that -- but it must have solid design, otherwise everything will turn into Flying Spagetti Monster for no good reason.. I mean, @javimtib92, what is going on with that flow, I've looked at this image several times and still don't get it. =P

    Why wait more when we already can discuss overall design upon first iteration experience? :)

    Debugging and visualization should be in one window! The one with the Graph!
    For ex. when in play mode - gray out impossible transitions of selected instance and visualize data transfering.

    It's not really experimental - just not officially supported. Many-many internal tools are using it, so it's not going to be changed/removed. Unless Graph Tools Foundation and Visual Scripting comes out in 2021 and flips the industry ;)
     
    laurentlavigne likes this.
  28. javimtib92

    javimtib92

    Joined:
    Aug 15, 2020
    Posts:
    7
    @Neonage not sure really, actually I tried to recreate the transition table in a graphic way. I've never did anything like this and my objective was just to share this idea to the rest. If you already thought about it then take it like I agree with your opinion.

    I have less experience than you in this area so I'm sure you can come up with something better. I'll be happy anyway because I will learn from it and I'll know that other people think that it was a good idea to use a graph view for this project.

    Looking forward to see how this progress

    Btw this image was nothing close to a finish implementation, it's dirty and doesn't represent what I think it could look like in the future. But I don't have enough experience with this tools yet.
     
    Neonage likes this.
  29. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    I know the one I made isn't particularly neat :D but I needed to get something out that wasn't the default inspector; that was an actual eyesore.

    I know, I just thought it might fall into the same category as preview packages. We're not using them because they're not officially supported, even though some of them actually work better than some of the supported packages :D.

    I think most people here agree with that. In terms of visualization and ease of use, it's undefeated.
    So let's stop repeating ourselves and start making it :p.

    Feel free to take the lead on it, I've never worked with GraphView so I know nothing about it :confused:.
     
    Neonage likes this.
  30. kirbygc00

    kirbygc00

    Joined:
    Apr 4, 2016
    Posts:
    45
    Hi All,

    I wanted to add my 2 cents here. First of all I think DeivSky came up with a wonderful solution and ya'll have been doing great work. Very neat use of SO to decouple data from behaviour.

    Ok! Onto my hot take- I thought that more complex state transitions were difficult to read. For example- Begin Jump Descent has 4 conditions, some of which us 'And' operators and one of which uses an 'Or' operator and is conveyed as a flat list:
    upload_2020-10-31_12-21-25.png

    If I didn't have contextual knowledge from how the SO's were named and the purpose of this transition, I am not sure whether I should be reading this as
    (element 0 && element1) || (element 2 && element 3) or as
    (element 0 && element 2 && element 3) || (element 1) or as
    (element 0 && element 2 && element 3 || element 1).

    Apparently, in this system, the ordering of SO transition objects in this list matters? That seems like it could lead to errors.

    I think an improvement to this would be to group conditions together into objects which evaluate to true or false and have a list of those. e.g.
    composite condition 0 - (element 0 && element1) ||
    composite condition 1 - (element 2 && element 3)

    I wonder if this change could also make the ShouldTransition method a bit easier to read:

    Code (CSharp):
    1.         private bool ShouldTransition()
    2.         {
    3.             int count = _resultGroups.Length;
    4.             for (int i = 0, idx = 0; i < count && idx < _conditions.Length; i++)
    5.                 for (int j = 0; j < _resultGroups[i]; j++, idx++)
    6.                     _results[i] = j == 0 ?
    7.                         _conditions[idx].IsMet :
    8.                         _results[i] && _conditions[idx].IsMet;
    9.  
    10.             bool ret = false;
    11.             for (int i = 0; i < count && !ret; i++)
    12.                 ret = ret || _results[i];
    13.  
    14.             return ret;
    15.         }
    The for i of j of idx is certainly compact, but not very easy to parse (at least for me).

    Refactoring to use the composite condition object should allow us to just step through with a for loop of i and check if any are true. e.g.

    Code (CSharp):
    1.         private bool ShouldTransition()
    2.         {
    3.             int count = _resultGroups.Length;
    4.             bool ret = false;
    5.  
    6.             for (int i = 0; i < count && !ret; i++)
    7.                 ret = ret || _results[i].IsMet; // or some new method in the CC SO like _results[i].ShouldTransition();
    8.                                                             // which checks it's list of conditions and returns a bool
    9.             return ret;
    10.         }
    We would be able to split out the j / idx checks into some submethod held on the composite condition SO which helps to keep grain size down. Thoughts? Hope that was helpful, and great work so far everyone =)
     
    deivsky and cirocontinisio like this.
  31. quibit

    quibit

    Joined:
    Jan 1, 2019
    Posts:
    4
    @kirbygc00, I agree that the conditions can be hard to interpret, however creating objects for the sole purpose of containing compound conditions could lead redundancies as well.

    Consider an object that evaluates as follows:
    (condition1 || condition2) && condition3
    Then consider a separate one that evaluates as follows:
    condition4 && (condition1 || condition3)
    etc...

    Here's my take on a possible solution.

    How about eliminating the ExpectedResult from a Condition object, and having the Transition satisfy all conditions (evaluating to true) and introduce new Conditions like NOT, OR and AND which take other conditions and return the aggregate operator on the conditions passed in. These can be binary or n-ary operators.

    For example:
    HandlingConditions.PNG
    Above are 4 conditions, but 2 of the conditions take other conditions.
    (I realize the 2nd is redundant if all conditions must be satisfied, but shown for example sake)
     
    deivsky likes this.
  32. kirbygc00

    kirbygc00

    Joined:
    Apr 4, 2016
    Posts:
    45
    Hi @quibit,

    Interesting... I think we are suggesting a similar thing, but with different implementations? =)

    I think we are both suggesting:
    - Changing from a "flat list" of conditions to a "hierarchical list"
    - The transitionSO would then evaluate this list and decide to transition if any condition was met
    - There could still be non-aggregated conditions at the top level of the list which trigger the transition, e.g. condition5 or condition6 in your sheet

    It seems the difference is that you would like to keep the current condition object, but modify it. Whereas I am suggesting to introduce a new object type to meet this new requirement.

    Both implementations would introduce the ability to aggregate the results of a list of conditions and return a single true or false value.

    Is this correct? I'd like to reserve any feedback until I am sure I understand correctly =D

    Thanks!
     
    Last edited: Oct 31, 2020
  33. quibit

    quibit

    Joined:
    Jan 1, 2019
    Posts:
    4
    @kirbygc00 ,

    Perhaps we're both onto the same thing, I thought from your post you meant making objects where the condition consists of multiple conditions in the evaluation itself.

    So I was picturing something like:

    Code (CSharp):
    1. class ConditionA {
    2.   public bool Evaluate() {
    3.      return Condition1 || Condition2 && Condition3;
    4.   }
    5. }
    6.  
    7. class ConditionB {
    8.   public bool Evaluate() {
    9.      return Condition1 && Condition2 || Condition3;
    10.   }
    11. }
    My approach would also introduce new objects but not composite of conditions so much as new Condition operator, just thinking about keeping objects small and modular. Basically introduce just a few new Condition objects which can be take other conditions.

    Code (CSharp):
    1.  
    2. class NotCondition {
    3.   public bool Evaluate(Condition a) {
    4.     return !a() ;
    5.   }
    6. }
    7.  
    8. class OrCondition {
    9.   public bool Evaluate(Condition a, Condition b) {
    10.     return a.Evaluate() || b.Evaluate();
    11.   }
    12. }
    13.  
    The modification (to StateConditionSO) comes from the fact that after doing this change, there is no longer a need to have a true and false expectation (variable expectedResult) since you can accomplish the same with a different Condition operator.

    We both definitely agree on this though:
    'Changing from a "flat list" of conditions to a "hierarchical list"'
     
  34. Harsh-099

    Harsh-099

    Joined:
    May 1, 2020
    Posts:
    33
    Can someone tell me that how the current state machine system works? How could I can add new states and Trigger them via scripts?
     
  35. DSivtsov

    DSivtsov

    Joined:
    Feb 20, 2019
    Posts:
    41
    cirocontinisio and MUGIK like this.
  36. WagnerGFX

    WagnerGFX

    Joined:
    Aug 26, 2013
    Posts:
    27
    In a nutshell:
    • Each State Machine has an initial State;
    • States have Actions;
    • States have Transitions to other States;
    • Transitions have Conditions to evaluate;
    • State Machine, States and Transitions don't need custom code to work, but might need new ScriptableObject instances for new settings or new agents (Enemies and such).
    • Check ScriptableObjects / Protagonist to see how the setting are made.
    • Actions and Conditions are the only thing you might need to create new scripts if none already do what you need.
    • Each Action and Condition will need 2 classes each, one for the ScriptableObject and another for the actual logic.
    • Check Scripts / Character / State Machine to see how that is implemented.
     
  37. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    Hey, thanks!

    I'm glad you pointed conditions out. Revisiting them was one of the things I had in the back of my head :).

    I tried to address the possible confusion with AND/OR when I was making the editor, I don't know if you've seen it, it's in this PR.
    upload_2020-11-1_11-19-47.png

    With the If... Is... True/False, and underneath it the And/Or, it indicates that the And/Or affects the next condition.

    Then, my idea was to implement the default behaviour of having one IF with a bunch of conditions without parenthesis, eg:
    Code (CSharp):
    1. if (a && b || c && d || e && f && g || h)
    2.     DoSomething();
    3.  
    4. if ((a && b) || (c && d) || (e && f && g) || h)
    5.     DoSomething();
    These two conditions evaluate the same, so And conditions are grouped together.

    As for the ordering of the transitions, I see no way around it. It's either ordering, or implementing some sort of a priority system, but I thought that would just overcomplicate things. Again, in the editor I made I addressed that by making it possible to easily reorder transitions and conditions from the inspector.

    I did think about making an object for grouping conditions together, it would definitely make that part of the code more readable, but it'd add the extra bit of overhead of having to go to each ConditionGroup object and then to every Condition inside it, instead of directly to the Condition. Considering this would be the base of many other systems, I thought a simple array of indices telling me how conditions are grouped together would be better in terms of performance. Definitely open to changing it though, since the goal of the project is to learn, readability is probably better so everyone can follow. I just wanted to let you know my reasoning behind it ;).
     
    kirbygc00 likes this.
  38. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    I don't know, the good thing about the ExpectedResult is that we can use the same SO for both purposes. For instance, why have both IsGrounded and IsNotGrounded? I don't see any benefit to that.

    I think I get what you're saying with the hierarchical list, and correct me if I'm wrong. It seems like a good idea, in theory, because you could make things like this:
    ConditionA && (ConditionB || (ConditionC && ConditionD))

    For instance, by doing:
    • Transition:
      • AndCondition:
        • ConditionA
        • OrCondition:
          • ConditionB
          • AndCondition:
            • ConditionC
            • ConditionD
    However, the thing that worries me is that, these are all SOs, and having them all nested together can lead to chaos very fast. The legibility of the hierarchy would depend on whoever made it. This is the same transition:
    • Transition:
      • OrCondition:
        • AndCondition:
          • ConditionA
          • ConditionB
        • AndCondition:
          • ConditionA
          • AndCondition:
            • ConditionC
            • ConditionD
    Now, this is again the same transition:
    • Transition:
      • ConditionA And
      • ConditionB Or
      • ConditionA And
      • ConditionC And
      • ConditionD
    I think it's more understandable without all that nesting. Another advantage of a flat list is that we can use a ReorderableList and easily make changes to the conditions, adding, removing and sorting as we please without breaking the whole chain. On the other hand, I can see changing a large hierarchy turning into a big headache.
     
    laurentlavigne likes this.
  39. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    @kirbygc00 and @quibit, I believe you are talking about different things.

    @kirbygc00 From your first post I get that you're talking about the structure of transitions at runtime, having And conditions grouped together to make code more legible.
    @quibit You're talking about the visual representation of the transitions and the SO workflow.

    Correct me if I'm mistaken :p
     
  40. WagnerGFX

    WagnerGFX

    Joined:
    Aug 26, 2013
    Posts:
    27
    On a similar subject, I am investigating a way to improve the memory and processing of the Actions and Conditions.

    Currently they are controlled by a list inside the State and Transition respectively. I am considering to add two centralized lists on the state machine itself. Since they are only supposed to be unique for each StateMachine and ScriptableObject, this could remove unnecessary duplicates and allow the implementation of the second idea I am investigating.

    That is to allow the conditions to cache their results on each frame. Right now if the transitions a set to check IsGrounded a hundred times in a single frame, the condition will fully check the logic every time and give the exact same result, because nothing has changed. This could be nightmare when we implement something heavier like HasPath and that might be needed to check every time.

    With these changes we can have more complex state machines without weighting down the system more than needed. This is specially important if we keep the current flat way of checking conditions, since many will have to be repeated.
     
  41. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    I was totally thinking about per frame caching of conditions as well!

    I'm making sure that there are no duplicates, though. ActionA in StateA is the same object ActionA in StateB. Same thing with conditions, except that I allowing making multiple StateCondition structs, but the Condition object they point to is the same.

    I'm not sure how I'd do the caching, though. One of the things I wanted to avoid was to have the state machine doing type lookups, that's why I didn't centralise everything there, but I'm open to reason :D.

    One way could be to have the condition handle its own caching, and then after all the transitions have been checked, the state machine calls ClearCache on each transition, which in turn clears the cache of each condition.

    I think that would be the simplest way, what do you think?
    And can you elaborate more on how you were planning to implement it? What would you do with the centralised lists?
     
  42. WagnerGFX

    WagnerGFX

    Joined:
    Aug 26, 2013
    Posts:
    27
    That is exactly what I did, until I noticed the Conditions were duplicated in each transition and threw all the code on the air
    (╯°□°)╯︵ ┻━┻

    The idea is quite simple:
    You add a bool to StateConditionSO that will toggle the caching on and off. Because someone might actually need some weird kind of IsRandomIntEven condition that changes every time you call.

    Instead of type casting anything, the condition will have a reference to StateConditionSO that will have access to evaluate if the cache is active or not.

    Then, instead of calling Condition.Statement() directly inside StateCondition.IsMet, you call another function inside Condition that is not abstract and will handle the cache while consume the abstract Statement.

    The cache is just 2 bools. One for the result and another for either isCached or isDirty (I prefer the former, it's cleaner :D). The rest is just about evaluating if the cache is active, if is already cached or if need to actually call Statement().

    Then, after all transitions were evaluated in State.TryGetTransition() you just make a final loop on all transitions calling them to clear cache, they will do the same to their transition list. Or, since the state machine will have a centralized list you can just ask the state machine to clear at the end of the Update() instead.

    The centralized list will just be like the other lists are right now, but on the level of the state machine, instead of the state or transitions.

    Consider like the StateSO.GetState() starts with an empty dictionary. Instead of that, you can get the dictionary from the state machine. In theory, I think it should work easily with a few changes.

    The lists inside the states and transitions don't need to be removed, what they need is to lookup the main list before adding new instances.
     
  43. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    :eek: Are you certain about this? That would be an error, but I just tested it and I can't replicate it. The same Condition repeated multiple times in one transition, in multiple transitions in one state, or in multiple transitions across several states, no matter True/False/And/Or, it should only be instantiated once.
     
  44. kirbygc00

    kirbygc00

    Joined:
    Apr 4, 2016
    Posts:
    45
    @deivsky, yeah I definitely see where you are coming from now. Thanks for explaining your thought process =)

    Blew my mind, haha the latter is definitely much more compact.

    I was messing around and trying to see what the latter syntax will not allow, and what jumped out was something like the following:

    var compoundA = a && b || c && (d || e);
    var compoundA = a && b || c && (d && e || f && g);

    Just generally more complex logic using parenthesis.

    EDIT: on second thought, we could achieve this with the current system, we would just need to duplicate some calls, e.g. c && (d || e) => c && d || c && e

    But you make a very good point- it's likely YAGNI code at this point, haha. Perhaps a more friendly solution is just the revamp to the editor UI to make it more legible. Maybe some documentation about how the operators works in this context, too =)

    Keep things simple while we can! If we hit a roadblock we can always refactor then.

     
    Last edited: Nov 2, 2020
    deivsky likes this.
  45. WagnerGFX

    WagnerGFX

    Joined:
    Aug 26, 2013
    Posts:
    27
    Important Reminder: reading code and answering forums tired at night is a bad idea :D

    Re-reading the code I noticed how the new Dictionary is instantiated at the State level, so there is no duplication inside each state. But this can be improved by moving to the state machine level. If my eyes (now full of coffee ☕) are not mistaken, doing this at the StateMachine.Awake() should already solve the issue.

    It's funny how this whole idea might just be a change in a single line. Nice implementation @deivsky :cool:
     
  46. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    :D Thanks!

    Although, the dictionary is instantiated only once, on the first GetState() call, and then passed to every state, transition, action, and condition in the chain, so there still shouldn't be any duplicates.

    Actually, I had it at the state machine level at first, but I thought having a Dictionary parameter in the public GetState method would be confusing, that's why I wrapped it into an internal call, but the functionality is the same.
     
    kirbygc00 likes this.
  47. WagnerGFX

    WagnerGFX

    Joined:
    Aug 26, 2013
    Posts:
    27
    It seems I rolled a D20 for perception and got a 1. :eek:
    You are absolutely right, the initial state calls for the GetState and the dictionary goes into every state, transition, action and condition.

    I have to say that this dictionary holding everything together is a great idea, but at the same time it's quite the nomad variable, being everywhere and staying nowhere. I had a hard time grasping it's scope.

    On my defense, I think it is better for the dictionary to be created by the StateMachine. Having a public GetState means anyone can call this function and duplicate everything (which is exactly what I thought it was happening). In the way the FSM is implemented, there is no need to call GetState outside the internal assembly.

    I will change the public GetState to GetInitialState, change its access to internal and add some comments so we can avoid confusion in the future.
     
    kirbygc00 likes this.
  48. deivsky

    deivsky

    Joined:
    Nov 3, 2019
    Posts:
    57
    Hahah don't worry!

    I guess I thought maybe there's a need for a slightly different SM or whatever so maybe not limit it to internal. But then, the whole idea is that there shouldn't be the need to make a new one because this one should be generic enough. So I do agree with the change to the access.

    And yeah, it definitely needs more comments. Thanks a lot!
     
    kirbygc00 and WagnerGFX like this.
  49. WagnerGFX

    WagnerGFX

    Joined:
    Aug 26, 2013
    Posts:
    27
    I've made a branch with the changes: fsm-cached-conditions

    Since the branch was made using the debugger as base, I can't make the PR right now, but when the previous changes are merged into the main project I will send one.

    One last change I made was moving the createdInstances Dictionary to become a readonly field in the StateMachine.
    With this is easier to understand the dictionary's scope and the code gets less prone to mistakes in future changes.

    The only drawback is that the dictionary will stay in memory after the StateMachine.Awake() but I think the impact should be neglectable (something around 2Kb for each dictionary with 100 items).
     
    laurentlavigne and kirbygc00 like this.
  50. quibit

    quibit

    Joined:
    Jan 1, 2019
    Posts:
    4
    Hello, so I've been catching up, and I agree with the decision to keep current approach.
    My suggested approach would indeed introduce more visual clutter and I think that may be most important.

    However, I'd like to present my vision on its benefits as it seemed a little misunderstood.

    I agree, but there would not be a IsNotGrounded, my suggestion was to have a new Condition NOT which simply returns the ! operator of another condition, so IsGrounded would just be an argument. So the number of Conditions does not increase much except two operators, NOT and OR (can also have AND but it's not required).

    .
    First of all, you nailed it, that is exactly the type of hierarchy I had in mind.
    This is similar to parse trees generated by a compiler. Look at that hierarchy, it's a binary tree (although it doesn't necessarily have to be binary, could be n-ary tree).

    So before leaving this discussion I'd like to leave the benefits I was thinking of for approach above.
    • un-ambiguous, this goes back to kirbygc00 original point, but basically however you read it, you know how to interpret it.
    • Simple and therefore less error-prone. So take your example above, you had to spread the operator before flattening. But let's introduce just one more operator, take this example:
    ConditionA && !(ConditionB || (ConditionC && ConditionD))

    Now unless you create composite conditions, you'd have to go through the mental exercise of unpacking the operators before you can flatten it. Whereas a hierarchal tree comes intuitively.

    • Multi-threaded ready. With a tree, you can evaluate nodes at the same depth concurrently. With a flat structure, you have to move sequentially.
    • No state required. With composable operators you have a very simple data structure, no data is maintained, only operations. Again this is less error-prone and better for concurrency.
    And basically that was my brain dump on this subject, no need to reply unless I made a mistake.
     
unityunity