My class has lots and lots of state variables. Am I designing poorly?

From what I understand, having lots of variables that keep track of state in your classes is generally frowned upon, I guess because it makes the code complicated and hard to figure out how it works (call a function and a million little states update under the hood), but I'm finding that my classes end up accumulating a lot of state variables. e.g. I have a script which represents an electrical arc that can bend and flicker on and off, turn into a strong straight beam, etc., and I end up with just a ton of little floats and bools representing flags of whether it's flickering, not flickering, how long ago it flickered on last(because it needs to flash brighter for the first .125 second after it flickers on), whether the arc was just turned on or off, and how long ago it happened (again, to drive shader/audio), and on and on and on.

I have another system which does dynamic running animation for my robots, and that thing has a billion variables keeping track of feet positions and velocities, where the feet are going, which legs are IK and which are FK, which leg is springing off and which leg is coming in for landing, spline interpolation targets, and dozens more.

Is there a design pattern I'm missing, or is this just more common in game programming, to have to keep track of a whole bunch of stuff like this?

 

For abilities/magic/powers I'd definitely not keep a bunch of variables directly on the character. I think at most that a character class should keep track of basic attributes and passing on movement commands (input) to animation and movement components.

Make it heavily component-based - have each ability in its own self-contained class. Everything could derive from a common Ability state machine class, which lets you reduce complexity to just methods for starting and and animating without having to worry about which ability it is in the core character code. The character only needs to know if an ability is available and how to execute it, while the ability takes care of its own state. Some common methods for handling energy/stamina/mana costs etc. would also be required. Make it so you don't need to change any code or properties in the character just to add a new ability.

See this official video on AI FSMs for some inspiration. It has bonus ScriptableObject use.

As for robot animations, it's possible you could similarly disconnect subsystems from each other. There are real-life robots where segments of legs/wheels have their own AI just doing its best to do one thing, and it automatically sorts itself out relative to sibling components. I'm not sure how it should be implemented, but it smells wrong if you just fill the character component with arbitrary variables for animation, IK etc.

 

There is some state that is just inherent to the task being done. And in general games tend to have a lot of state. There is no harm in having state variables when you actually need them.

That said one of the primary culprits for excessive state variables in Unity is the fact that you can't keep a locally scoped variable between Update calls. Which means you have push any state that needs to hang around for more then a frame to the class level.

In some cases you can use Coroutines to mitigate this. Coroutines can hold local scope variables across multiple frames. That way you don't have to clutter up your class with state that's not relevant to the class as a whole. (Under the hood coroutines generate their own class to keep local state. But that's not code you have to deal with, so its fine.)

Then of course you can keep breaking things down to smaller and smaller components. This doesn't reduce the total number of state variables, but it does help keep them more manageable.

 

I'm not really qualified to weigh in on this but .. sounds to me like you're describing animations. I think there is a reason those have been pushed into other tools like sprite sheet editors for 2D and Blender for 3D. If you are drawing legs dynamically based on runtime values entirely with code, no doubt you will have a lot of local state!

Without seeing actual code, I think we'll all be guessing. My guess is that you should just describe your animations with a tool and transition to them with code.

 

Yeah, if it's ONLY for animation there's already an animation system with state machines to hook into.

 

Since you mentioned, there is an official State design pattern, but it won't cut them down - just pushes around the logic. If you have: enum weaponStatus {charging, firing, recoiling}; you're supposed to write a virtual superclass for weaponStatus. Then empty subclasses for each of the three possible values, then add and overload virtual functions so that ws.doStateBasedAction1(); replaces the IF you were going to use. To change states, assign ws=new firing();. Obviously, it only works for discrete values - not floats where certain ranges represent a state.

It's kind of a neat exercise, abusing dynamic dispatch to get a feel for what it really does. Plus the idea of classes that just implement logic and don't really represent anything, which you see a lot of in the "OOP or die" crowd.

 

Yes, this is the primary reason I need so many variables. Was this thing on or off last frame? Boop, new variable. How long has it been since X happened? Boop, new variable. Where's this thing headed? New variable. Where was it coming from? New variable.

I had thought about coroutines but I need these to run in a specific order in my Update/LateUpdate calls. And more components makes sense if they are actually components conceptually, but if they really are just state needed by this one class, I guess like you say, games tend to have a lot of state, so maybe it's OK to just have a lot of state variables.

It would be really, really nice to have variables that persisted over multiple calls but were scoped just to a function (like how coroutines look like they work from the outside point of view). My code be so much cleaner, as simply having all these state variables floating around makes me more prone to 1) forget what they were for and 2) access them or modify them in a method where I really shouldn't be. i.e. they really should be "private to a single method" but because any method in the class can read them, it's tempting to peek at them from elsewhere, and then if I change my method it can break things unexpectedly.

Well, one of the examples I listed was indeed for animation, though it seems to be a general pattern for me. In this case, I have a custom animation system to let my character run around dynamically and lean into turns and plant its feet nicely - if you're interested here's a test I recorded awhile ago:

So it's not an animation I could have done in my 3D package - it's a complicated script on the lower body that just has to keep track of a ton of stuff - where the last step was, where the next one is, what state it's in, IK spline positions, FK joint angle velocities, target end positions and velocities, current and rest positions/rotations of important joints, and dozens more. I've broken out components where it made sense (each individual leg has it's own leg script which just does IK/FK positioning, and there are little joint rotation spline interpolator scripts on all the joints that keep track of angles and rotational velocity so I can switch back and forth between FK and IK, etc. that the main script can set or get information to and from), but in the end, there is one master lower-leg-running-system script that just needs to know lots and lots of stuff, so it has lots of state variables...

But even a simpler case like my electrical arc script seems to end up accumulating lots of state variables eventually.

 

You've probably heard the OOP mantra "loose coupling, high cohesion".

When you say

This sounds a lot like low cohesion to me. I understand it is tempting to break scripts apart where it fits the analogy of the physical object you are creating. Like you said, you have separate scripts for each leg. However, you need to remember programming is not real life and you should not restrict yourself to the same physical analogy.

One way to improve your cohesion is to split out the methods which use independent local variables. Let's look at an example.

Code (csharp):

1. class Varrick
2. {
3.   private int a;
4.   private int b;
5.   private int c;
6.  
7.   public void HeyZhuLi()
8.   {
9.     // Use local variables a and b to do something.
10.   }
11.  
12.   public void DoTheThing()
13.   {
14.     // Use local variables b and c to do something.
15.   }
16. }

This could be refactored like so

Code (csharp):

1. class Varrick
2. {
3.   private int b;
4.   private ZhuLi ZhuLi;
5.   private Thing Thing;
6.  
7.   public void HeyZhuLi()
8.   {
9.     ZhuLi.Execute(b);
10.   }
11.  
12.   public void DoTheThing()
13.   {
14.     Thing.Execute(b);
15.   }
16. }
17.  
18. class ZhuLi
19. {
20.   private int a;
21.  
22.   public Execute(b)
23.   {
24.     // Use local variable a and parameter b to do something
25.   }
26. }
27.  
28. class Thing{
29.   private int c;
30.  
31.   public Execute(b)
32.   {
33.     // Use local variable c and parameter b to do something
34.   }
35. }

This vastly improves our cohesion and keeps our variables a and c protected from accidental nefarious use.

 

To rephrase eisenpony's example, if you have lots of variables crammed into your script that are only used by a few functions, it's often nice to split it all off into a new class. If you have:

float timer1, timer2;
int stateA, stateB;
void doThingWithTheseVars() {...}

it can be turned into a class holding all that stuff:

class ManageLazer { float timer1, timer2; ... public void doThing(); }

You have to new() a ManageLazer variable, but not a huge deal. And sometimes you need a backpointer from the smaller class (to your main script,) but that's no big deal either (Components in Unity all have them.)

 

A slight variation of this, which I often use, is to create a dictionary associating each enum value with a delegate pointing to a method. A dispatcher method simply looks up the delegate based on the current state and calls it. This doesn't solve the "lots of state variables" thing, but it does clean up a lot of control logic, and not quite as intrusive as creating a bunch of classes. It's also fairly simple to encapsulate this logic into a simple class.

 

and

Awesome, that's all great information and makes sense.

I know premature optimization is the bane of all blah blah blah but just so I know, is there any kind of overhead with making a new class and calling a function on it like that? My intuition says since you only make the new object once, the extra overhead is 0.

The right way to do this I assume is to make this new class inside the original class (make a nested class), so that the entire implementation (that there is a separate class at all) is completely hidden from everything else, correct?

 

A nice thing about splitting data off into a new class is you can put it into a different file (or keep it as an inner class.) Put all of your "state classes" into the same file, if you like. That might make it easier to find things. Or you could make these small classes Monobehaviours (with no Start or Update) just so you can have them in the Inspector, filled in and pre-hooked up.

 

There are almost always performance tradeoffs when adding more abstraction to your design. Even splitting your code into multiple methods creates a lookup into a virtual method table. Moving instructions and data into other classes probably won't make a big difference in terms of memory use, but I suspect your refactoring will lead to some additional initialization and execution method calls. This means your total extra overhead will be bigger than 0.

The good news is that these performance penalties are minuscule compared to the benefit we receive in terms of organization and clarity. I'd argue that, used intelligently, simple refactoring to introduce smaller and more cohesive classes will, in the end, lead to huge performance gains through better developer understanding and insight. Algorithm changes almost always create larger improvements than "micro-optimizations" like writing all your code in a single method.

As far as the "correct way", this is a very subjective topic.. I will say I rarely find a practical use for a nested class. I think I'd prefer to see the classes in their own namespace but not nested. Nested classes can be a strong way to protect implementation but I think it's usually overkill. I also find "loose classes" easier to interface in the future if I want a second implementation. Also, a well split class will define an intuitive interface so I don't typically need details, having the class nested in the same file is just distracting.

 

You can always roll your own coroutine caller. It's actually pretty trivial to do, simple store the IEnumerator and call MoveNext at the appropriate time.

Under the hood the compiler will make its own little class to deal with it, but you don't need to worry about that. It effectively lets you scope variables local to a persistent method.