Official State Machine

@deivsky and others, I tried to create new states, actions, conditions and others for enemy, as @cirocontinisio said me to do for my Simple Enemy System, to adapt the currently used statemachine. I did so today, but at last ended up with some errors.

When I finished added some states (Idle, Move and Attack), actions (Moving, Attacking), Conditions and Transitions and added respective scripts to it, there were no errors in console.

I played the game, went near the enemy but there were no expected actions. When I looked at the console there was one error: (Image below)



And the State Machine script on enemy was disabled (but It was enabled before playing the game)
When I manually enabled the script, the console was showing another error, NullObjectReference repeatedly again and again (Image Below):



Please Help...

 

Hey @Harsh-099,

from the photo you attached it seems like you added the same transition twice (enemy to moving) which is throwing the error. i would make sure you don't have duplicates and see how it goes.

If you haven't gotten any help by tonight I'll boot up the project and troubleshoot this for you.

 

Hey!

I managed to replicate the error, and I'm not sure how it didn't come up earlier! .

It happens during initialization, when a Transition that's being created chains up to a State that hasn't been created holding the same Transition.

Examples:
StateA => TransitionA => StateB => TransitionA => Exception. (why would you do this?)
StateA => TransitionA => StateB => TransitionB => StateC => TransitionA => Exception. (this seems more likely)

Where all the States and Transitions are being instantiated for the first time.

This is because the Transition is added to the Dictionary after getting its target State:


The solution would be doing the same as what I did with States where I add an empty State to the Dictionary and then fill it:


@Harsh-099 from your stack trace it seems like the transition that's causing you trouble is EnemyToMoving. For a quick solution, make copies of it for each state that uses it.

@cirocontinisio I'm gonna make a PR fixing this, but this wouldn't be happening if we had TransitionTables, any idea if/when that'll get merged?

Edit: Issue and PR submitted.

 

It's okay, I know you've been busy, a lot of merges lately! And I know this isn't the easiest thread to follow, soon we'll catch up in posts with the Choose the game title thread .

So to recap:

• The current state of the StateMachine is that the whole workflow of state/transition/condition/action is so convoluted that it makes you not want to use the StateMachine at all.
• The TransitionTable PR, which comes with a custom editor, is currently the only proposal that addresses this issue. The one thing I'm afraid is I'm not much of an Editor scripting expert so, even though I've made my own testing and everything seems to work fine, more testing might be needed. But what better testing than merging and having people use it?
• There was a talk about GraphView but I'm not sure whether anyone is actually moving forward with it.
• Besides that we have the improved StateMachine debugger, by @WagnerGFX, which provides better logging of the transitions between states, including what condition is making the transition happen, proving really useful for spotting possible errors. Further iteration on this is still needed but I think it should get merged, and we can continue to add on top later.
• There's another draft PR by @WagnerGFX that would mainly improve performance on conditions that are repeated in one State by caching the result. It's draft because it depends on the debugger PR, but the change is minimal so I think it can be merged as well.

And about the fix:
Applying the fix would just be a temporary solution if the TransitionTable PR gets merged, because TransitionSOs (which are the cause of the issue) are entirely removed in it. Seeing that there are no other proposals to improve the workflow, I'd push the TransitionTable PR.

I don't think I'm missing anything, we had a couple of discussions going on but I think they came to proper conclusions. Anyone can correct me if I missed something and/or disagrees .

 

My suggestion is to also start merging with the TransitionTable, then go to the debugger and finally the condition caching.
Specially since the TransitionTable is a great improvement and the other 2 PRs will need some small changes due to merging conflicts.

There was a talk about improving the condition logic system, but we still don't have any conclusion and the condition caching is independent of that.

 

Solid plan, sounds good to me.

RE condition system logic:
IMO this is the lowest priority. these editor improvements may do the same job (from a ux PoV). I think we're mostly in agreement that we should hold off on condition system changes until we know we need them!

 

It should've been in a separate branch from the start

 

Oh, I thought it was happening because of something mine mistake. I will try it after the pull request gets merged.

 

It's a good idea, but we should also put a timer or another type of limit on the changes since this is a core system and could impact the rest of the development if we keep changing too much and for too long.

Is anyone cooking any other changes for the FSM?
At least I'm out of ideas.

 

It would be good to have a separate branch to continue development, but I think, after the livestream, we should merge what we currently have to main, that way we can start using it for the development of other systems. I believe most of what might be missing is editor stuff, so there shouldn't be any breaking changes that require rebuilding every state machine we have (like replacing StateTransitionSO with TransitionTableSO), and it should be safe to move forward with these other systems.

 

Since the other two PRs have been merged I will now be able to review the conflicts and make it a better PR.

@deivsky, have you seen this recent video about the Editor? The side list example at the end might actually be very useful to make a single window where every Transition Table is listed and able to edit.

 

That means I can work on enemy state machine from this 'states machines improvements' branch. But I have created my feature-enemy branch from main branch, how will I add the new branch changes to it?

 

Issue time!

When using an Assembly Definition, any Editor sub-folder loses its powers and everything is set to the same assembly.
That means any attempt to compile will give an error due to the use of Editor scripts.

The default solution is to add another assembly definition for that folder:
- The FSM and editor scripts will stay in the same place (good, minimal changes)
- The editor scripts will be isolated from the rest of the code (good practice)
- Another project inside the solution on the visual studio hierarchy (bothersome, but good)
- Seem to be the default recommendation on the Unity Manual (good)

There are other possible changes, but they have their own issues with good practices:
- Move the scripts to the existing Assets>Scripts>Editor
- Have a single UOP1.Editor assembly definition and add multiple references when needed.

Should I go with the default or there are any better suggestions?

 

Ahh I'd go with the default one. Usually I do but I saw everyone was using Editor folders, didn't know they don't work with assemblies.

 

I didn't either, but I opened Visual Studio and saw the editor code inside UOP1.StateMachine instead of Assembly-CSharp-Editor.

At that moment my Programmer Senses™ issued a warning

And now we have two issues, since there is another script using editor references. But that one is also in the main branch so I opened an issue. For this one in the State Machine I opened a PR with the fix.

 

Can I not pull the 'state-machine-changes' branch into my 'feature-enemy' branch and from there continue working?

And change the pull request pointing 'main' branch to 'state-machine-improvements' branch because i can't seem to edit my side branch in the pull request.

 

If you try to merge the changes from "state-machine-improvements" into your "feature-enemy" then the commits will fall out of sync. The best thing to do is just create a new updated branch and move manually the changes to it. I had to do the same with my changes for Condition Caching.

You can't change your side of the PR. I actually just had this issue with my changes and had to make a new PR with the new branch.

On another note.
@cirocontinisio, should we change the namespace of the FSM from UOP1.StateMachine to ChopChop.StateMachine?

 

So should i delete the "feature-enemy" branch (after moving changes from it) or not?

 

If the branch is unused and any PRs have been merged or cancelled, then I think there should be no problem. I usually follow this logic when deleting my branches.

 

Yes, please!!!

 

@Harsh-099 Assuming you've been commiting the changes into your branch you could create a new branch from the new state machine branch and then cherry pick the commits from yours to this new up-to-date branch and this way you wouldn't have to move any changes manually, but you would still need to check that your cherry picked changes doesn't change or use any non-existent files or code that isn't there anymore. And for those who didn't commit the changes yet they can stash their work and apply the stash into the updated branch.

 

What is meant by "cherry pick"?

 

Basically picking your commits one-by-one from your original branch and putting them into the up-to-date branch. Check out these examples:

• https://git-scm.com/docs/git-cherry-pick
  
• https://devconnected.com/how-to-cherry-pick-git-commits/
  
• https://www.git-tower.com/learn/git/faq/cherry-pick
  
• https://mattstauffer.com/blog/how-to-merge-only-specific-commits-from-a-pull-request/

 

This is not superfluous at all. Reading the conditions was hard even when knowing how it works.

@deivsky is still working in a full editor window for the transitions, but I don't know if the changes are close to finish.

 

(Butting in ) The full editor window also has a list of every transition table in the project to select and edit faster instead of relying in finding them in the inspector. But its true that it is creating two places to edit the same thing.

If speed is important, it might be better to add later (since there are no changes in the core) or review the idea so it won't be a duplicated window. An alternative could be to make the window a dedicated listing (with no editing) of every SO related to the FSM so it can be selected or dragged to the inspector.

From what I can see there are no more changes to the core of the FSM unless some big issue appear. Performance and flexibility are good and the implementation is still simple enough.

 

Hi, sorry I haven't been so active, I'm moving out this week so it's been super busy lately .

These are great additions!
If by layout issue you mean with the add transition box, I've already addressed that in my branch with the editor window. I refactored it to use EditorGUI instead of EditorGUILayout. I can move it to a separate branch if we decide we don't want the window.

@WagnerGFX already made the point on why it's useful. The editor window is basically just for easier accessibility to get to the transition tables, then it simply draws the inspector of the selected one.

I'll be pushing a PR soon!

 

Okay, I've opened a new PR.

@cirocontinisio I fixed the layout issue you actually meant (found the TODO in the code).
@WagnerGFX I changed the add transition button to make it clearer, and moved the menu item as suggested.

 

Definitely makes sense. I didn't know why we had two MB that had basically the same purpose (control the character).

It's a little weird that the condition is the one consuming it, but I guess an action just to set it to false is also redundant. What you're doing is the equivalent of:

Code (CSharp):

1. if (boolean)
2. {
3.     boolean = false;
4.     // Do stuff...
5. }

Which is rather common, so I'd say it's fine.

 

Hi ChopChop's brains!

I am starting to look at using the state machine for enemies like the plant critter and I was wondering what was the targeted folder structure for state machine SO?

First, is it too early? Should I wait for the merge of state-machine-improvements branch?

Next, what is the targeted folder structure for all these SO?
There is this folder: ScriptableObjects/Protagonist, that seems quite generic, but I have the feeling that if we start to add SO for each NPC/Enemy here SO will soon be undiscoverable...

Also, the same state will not have the same conditions/actions depending of the protagonist. For instance Idle state for the playable character is not transitioning the same as an enemy.

Maybe the response is obvious, sorry I am not fully familiar with the big picture around the usage of our SM yet.

 

Hey, totally, I think it's time that we start making more use of the state machine. Main is already up to date with our branch, except for some editor stuff, so I'd say it's safe to move forward.

As for the folder structure, there just isn't one for enemies yet, but you can add it yourself. Following the same structure of the protagonist, you could make it ScriptableObjects/Enemies.

 

I thought it was enough with the onChangedCallback!

Code (CSharp):

1. reorderableList.onChangedCallback += list => list.serializedProperty.serializedObject.ApplyModifiedProperties();

 

Hi there,
we had a quick usage review of the state machine yesterday with a new comer and we found that it is difficult to understand that the current state rows in the transition table are expandable sections, despite the note at the top, that seems to be just ignored by users
Actually, my understanding is that the little arrow before the label has the same color as the background so it is completely invisible.

In term of possible solutions, the following could solve this issue:
- Expand the states by default when the table is opened
- Change the background color to make the arrow visible
- Change the arrow color (I did not find how to achieve this)

What do you think about it?

 

Hey, nice feedback!

I actually don't remember how I got rid of the arrow, I don't think it's hiding with the background, I think it's just not there because we're using a custom style. Anyway, I removed it because I didn't like it, but I do see it being a little confusing without it.

States are currently only expandable one at a time. I did this because I thought it would look very convoluted and become harder to navigate if we could have all open, and there's usually no need to look at more than one state at a time. We could have the initial state expanded by default though, that should make a good enough hint that you can expand any of the rest.

 

It's never enough!

I get, though, that it's also a bit more confusing because at the same time there's a toolbar right on the foldout header. Maybe that whole thing is something that could be reviewed.

 

The initial idea of separating SOs from runtime data was about shared vs. non-shared data.

For ex. Enemy SO can have a lot of shared properties, but only a few of them would be unique per-instance at runtime.

I agree that these "Authoring" SOs created a lot of boilerplate code, and all they do is just create runtime class for the instance, passing the same values..

So I'm wondering - why StateAction/StateCondition is not an interface for the struct?

That way, we can just paste struct data modified in the inspector, without boilerplate constructors.

Edit: in the example, parameters should actually be in the SO as a shared data, instead of being unique for every instance, wasting memory and "playmode editability". You get the idea