Skipping code lines based on components

How can I make it so that parts of my code are skipped if a component is not there on the game object. While keeping a clean and efficient code?

The idea here is that I have a modular class that encompasses many others, but these smaller classes won't always be present.

So "Mover.cs" would be the main class that calls the methods of the smaller classes (Jumper.cs, Dasher.cs, WallSlider.cs, ...).

What I was previously doing was this: http://i.imgur.com/EMEZg6M.png -> A big class that has everything. I decided to change this because I often don't need most of these attributes. Now I separated them into smaller classes, which I can just drag onto the game object.

I want to have the Mover.cs class not execute Jumper.Jump() if there is no Jumper component on the game object. And if there is one, to execute the Jumper method calls.

Code (CSharp):

1.  
2. // in the Mover.cs class
3. if (Jumper) Jumper.SetPlayerGravity();
4.  

This results in: I have to do tons of "if (Jumper)" checks. It ends up making the code quite ugly. Is there a cleaner way to do this?

 

You could try splitting it up once more. You could have a mover class and a subclass that's Jumper (mover + jumper?).
That way they'd share the movement, but only the one type of class would know anything about jumping.
If your code is working already, that's okay, too.
I don't know how your code is exactly, or how many times you have to check that .. if it's really a big issue

 

You're on the right track. Keep pushing the decision making into the "smaller classes". One option is to create alternate implementations of each "smaller class", one that does what you expect and one that does nothing. Instead of the "modular class" checking for existence, it just fires the event.

I made a bunch of stuff up since I haven't seen your code and I'm not super proficient with Unity but this should give the general idea:

Code (csharp):

1. public class Mover : MonoBehaviour
2. {
3.   private IJumper Jumper { get; set; }
4.  
5.   public void Start()
6.   {
7.     Jumper = GetComponent<IJumper>() ?? new NoJumper();
8.   }
9.  
10.   public void MyMethod()
11.   {
12.     Jumper.SetPlayerGravity();
13.   }
14.  
15.   public void MyOtherMethod()
16.   {
17.     Jumper.Jump();
18.   }
19. }
20.  
21. public interface IJumper
22. {
23.   void SetPlayerGravity();
24.   void Jump();
25. }
26.  
27. public class Jumper : MonoBehaviour, IJumper
28. {
29.   private double Gravity { get; set; }
30.   private double JumpStrength { get; set; }
31.  
32.   public void SetPlayerGravity()
33.   {
34.     Gravity = 9.81d;
35.   }
36.  
37.   public void Jump()
38.   {
39.     rigidBody.AddForce(Vector3.Up * JumpStrength);
40.   }
41. }
42.  
43. public class NoJumper : MonoBehaviour, IJumper
44. {
45.   public void SetPlayerGravity() { }
46.   public void Jump() { }
47. }

 

That would be down the inheritance path rather than the adding components path (modular path? I don't know how it's called). It's not really what I'm looking for in this particular situation, thanks anyway.


@eisenpony : That seems to be smart and pretty effective. I guess instantiating the class NoJumper is really no big deal in terms of memory because it's only empty methods? Haven't really used interfaces so far but I understand the idea and that it's necessary in this case. Cool trick, thanks!

EDIT: After having used the interface in my code it's not really usable because I also would like to access certain public fields in my class. I guess I could make GetMethods for that but it's still a lot of stuff I have to write for interfaces I won't be using except for that use. Great idea though. Maybe there's something else I can do?

Code (CSharp):

1. public class NoJumper : MonoBehaviour, IJumper
2. {
3.     public void ApplyGravity()
4.     {
5.     }
6.  
7.     public void CastRayToGround()
8.     {
9.     }
10.  
11.     public int GetJumpNr()
12.     {
13.         return 0;
14.     }
15.  
16.     public int GetMaxJumpNr()
17.     {
18.         return 0;
19.     }
20.  
21.     public void Init()
22.     {
23.     }
24.  
25.     public void LimitMainVelocityY()
26.     {
27.     }
28.  
29.     public void MinimumCurrJumpNrEqualsOne()
30.     {
31.     }
32.  
33.     public void OnJumpInputDown()
34.     {
35.     }
36.  
37.     public void OnJumpInputStayPressed()
38.     {
39.     }
40.  
41.     public void OnJumpInputUp()
42.     {
43.     }
44.  
45.     public void SetPlayerGravity()
46.     {
47.     }
48.  
49.     public float TimeBeforeCharacterHitsGround()
50.     {
51.         return 0;
52.     }
53. }

 

Interfaces support properties. So you can use them to access public fields in a class. You'll also need to implement the same properties in your NoJumper class.

The concept is sound, but your implementation won't work verbatim in Unity.

You can't create a MonoBehaviour with new. Unity will throw you an error. Instead you can use AddComponent.

The null coalescing operator might also cause you trouble. I have a vague feeling that the current version of mono unity uses doesn't support it. It can also bite with Unity's fake null implementation, which means a UnityEngine.Object can sometimes be null and not null at the same time. Either way you can get around it by writing it out long. (I'm not 100% sure it will be a problem in this case. But fake null can bite when you cast a MonoBehaviour to an Interface, then destroy the original MonoBehaviour and keep a reference to the interface).

 

A small addition: I wouldn't use doubles for gravity or jump force; just a habit to type floats.

 

Isn't there a faster way somehow? Right now calling a method for an object that's null throws an error maybe there's a way to make it not throw an error and skip it instead?

 

It really sounds like you haven't properly separated things yet. If jump and move really belong on separate scripts, they shouldn't need to share very much data at all.

So either cut the apron strings completely, and make these two scripts independent. Or jam them back together where they belong.

 

I need to specify the order in which each class's methods are called.
i.e.:
1)Jumper.Jump()
2)Dash.Dash()
3)Jumper.SomethingElse()

So that means the only way left is to make it in one script right? If I do that it's excess code for most entities in my game. Maybe that's not so bad but it's too bad that there's no way to do this properly. It feels like Unity wants you to code in smaller classes and add behaviours like what I previously explained.

 

There are always more ways of completing a task, it simply takes time to find the right level of abstraction. We want to break our components down so that they are easier to understand, extend and replace. However, we don't want to break things down into meaningless atoms so that it's impossible to get a view of the big picture. As Einstein said, everything should be made as simple as possible, but no simpler.

The trick is to find the correct balance of general vs specific implementation. Personally, I lean heavily towards preferring general solutions because, while they are more difficult to create initially, they tend to be more flexible and extensible in the future. Others lean towards specific solutions because that allows them to move quickly without worrying about problems that might never show up. With practice, you will get faster at creating generic solutions, so you can have the best of both worlds.

---------------------------------​

Right now, you call each method explicitly but another option to ordering your method calls is to add them to an ordered collection and execute them in the same order they are added.

Maybe it makes sense to implement a two-layer-event system. Each interface is given two opportunities to execute some event. For every component, Event1 is called in the order the components were added. Then for every component, Event2 is called in the order the components were added. An example can be seen in the way Unity implements their MonoBehaviour lifecycle. First, each component has Awake() called, then Start(), then for every frame, Update and LateUpdate. Of course, the actual implementation is quite complex but the idea is there.

Or maybe you need finer control over the method calls, then a sorted collection of method calls could work.

On the other hand, if the order and parameters are complex, a manager with implicit knowledge of the required information and order of events making each call explicitly isn't so bad. However, we want to separate the concerns here as much as possible. It's okay for the manager to have knowledge of the order of event calls but let' say the manager also holds the data needed to execute those events. As much as possible, we want to encapsulate information into the classes that need that information.

Consider this pseudo code.

Code (csharp):

1. class Manager
2. {
3.   public Jumper Jumper;
4.   public DasherAbility DasherAbility;
5.  
6.   public double jumpStrength;
7.   public double dashStrength;
8.  
9.   public void Execute()
10.   {
11.     Jumper.Jump(jumpStrength);
12.     DasherAbility.Dash(dashStrength);
13.     Jumper.DoubleJump(jumpStrength/2);
14.   }
15. }

If we move the relevant information into the components that actually need to do the work, our manager gets simpler.

Code (csharp):

1. class Jumper
2. {
3.   public double jumpStrength;
4.  
5.   // Implement Jump and DoubleJump..
6. }
7.  
8. class DashAbility
9. {
10.   public double dashStrength;
11.  
12.   // Implement Dash..
13. }
14.  
15. class Manager
16. {
17.   public Jumper Jumper;
18.   public DasherAbility DasherAbility;
19.   public void Execute()
20.   {
21.     Jumper.Jump();
22.     DasherAbility.Dash();
23.     Jumper.DoubleJump();
24.   }
25. }

Now, you can see the manager is just responsible for calling things in the correct order, the parameter information was moved to the subcomponents. The manager is simpler because instead of controlling the order and the required data, it only knows about the order of events.

Required data and execution order are examples of the different kinds of concerns that we should take into account when designing a class's "Single Responsibility". As much as it makes sense, identify and divvy these concerns up so that each class can be responsible for only one thing.

At this point, we've actually reached a pretty good balance, so I'm wondering why you say this

Where do you see excess code? What doesn't feel "proper" to you?

---------------------------------​

Even though it's a gimmicky idea, let's explore one step further with the two-layer-event interface idea just to see how it changes the Manager.

Code (csharp):

1. interface IComponent
2. {
3.   void Event1();
4.   void Event2();
5. }
6.  
7. class Jumper : IComponent
8. {
9.   public double jumpStrength { get; }
10.  
11.   public void Awake()
12.   {
13.     Manager.Register(this);
14.   }
15.  
16.   public void Event1()
17.   {
18.     Jump(jumpStrength);
19.   }
20.  
21.   public void Event2()
22.   {
23.     DoubleJump(jumpStrength/2);
24.   }
25.  
26.   // ...
27. }
28.  
29. class DashAbility : IComponent
30. {
31.   public double dashStrength { get; }
32.  
33.   public void Awake()
34.   {
35.     Manager.Register(this);
36.   }
37.  
38.   public void Event1()
39.   {
40.     Dash(dashStrength );
41.   }
42.  
43.   public void Event2()
44.   { }
45.  
46.   // ...
47. }
48.  
49. class Manager
50. {
51.   public Queue<IComponent> components;
52.   public Queue<IComponent> Components
53.   {
54.     get
55.     {
56.       if (components== null)
57.         components= new Queue<IComponent>();
58.       return components;
59.     }
60.   }
61.  
62.   private Manager singleton;
63.   private Manager Singleton
64.   {
65.     get
66.     {
67.       if (singleton == null)
68.         singleton = new Manager();
69.       return singleton;
70.     }
71.   }
72.  
73.   public static Register(IComponent component)
74.   {
75.     Singleton.Components.Enqueue(component);
76.   }
77.  
78.   public void Execute()
79.   {
80.     foreach (var component in Components)
81.       component.Event1();
82.     foreach (var component in Components)
83.       component.Event2();
84.   }
85. }

Now the Manager class isn't even responsible for knowing what components exist -- there is no explicit reference to Jumper or DashAbility -- just that some exist and implement the IComponent interface. This has made the Manager really simple, though it has made the lifetime management of the components a little more complex. This is the balance I'm talking about.

 

It's supported (an early mention of it is from 2011). It just won't work for UnityEngine.Object for the reason you mentioned.

 

Thanks for the help!
When I mentioned the excess code I meant that if I wrote everything in a single script, seeing as most of my game characters wouldn't need dash or certain abilities, it wouldn't be optimal to put the whole script on them. That wasn't about your answer but for Kiwasi.

The 1st idea with the Manager class seems ideal, it's the one I'm currently using however the problem I had with this one is that since I will add different components on every different character I must check on every line where I use that component if it's not null.

So in your example I would have to do this:

Code (CSharp):

1. class Jumper
2. {
3.   public double jumpStrength;
4.   // Implement Jump and DoubleJump..
5. }
6. class DashAbility
7. {
8.   public double dashStrength;
9.   // Implement Dash..
10. }
11. class Manager
12. {
13.   public Jumper Jumper;
14.   public DasherAbility DasherAbility;
15.  
16. void Start()
17. {
18.      Jumper = GetComponent<Jumper>();
19.      DasherAbility= GetComponent<DasherAbility>();
20. }
21.   public void Update()
22.   {
23.     if (Jumper) Jumper.Jump();
24.     if (DasherAbility) DasherAbility.Dash();
25.     if (Jumper) Jumper.DoubleJump();
26.  
27. // Obviously, in this example it's quite easy to add 3 if's
28. // In my scripts, it's the same idea but I have to do it a lot more, around 5-10 times per class in the Manager.
29.   }
30. }

The 2nd idea with IComponent is cool indeed, but the problem is that it puts a constrain on how I'll have to code, it could work but it's not as clear as the order with Manager.

If I could check once in the code that there isn't a Jumper or a DasherAbility it'd be sweet.
I like my code as is except for the lots of if's that arose when I separated the single class into multiple ones with a Manager. Here's what I would like to achieve..
THIS:

Code (CSharp):

1.  public void Update()
2.   {
3.    if (Jumper == null) SkipJumperLinesOfCode();
4.    if (DasherAbility== null) SkipDasherLinesOfCode();
5.  
6.     Jumper.Jump();
7.     DasherAbility.Dash();
8.     Jumper.DoubleJump();
9.   }
10. }

OR THIS (in the Jumper class):

Code (CSharp):

1.   // in the jumper class, Jump() is a method
2. public void Jump()
3.   {
4.    if (the calling object is null) return;
5.  
6.    // Jump code implementation here...
7.   }
8. }

Surely that 2nd option I suggested cannot work seeing as a null object won't have access to the Jump() method in the first place.

 

I think this will work.

Code (csharp):

1. public class Manager : MonoBehaviour
2. {
3.   private IJumper Jumper { get; set; }
4.   private IDasher Dasher { get; set; }
5.  
6.   public void Start()
7.   {
8.     Jumper = GetComponent<IJumper>() ?? new DisabledJumper();
9.     Dasher = GetComponent<IDasher>() ?? new DisabledDasher();
10.   }
11.  
12.   public void MyMethod()
13.   {
14.     Jumper.Jump();
15.     Dasher.Dash();
16.     Jumper.DoubleJump();
17.   }
18. }
19.  
20. public interface IJumper
21. {
22.   void Jump();
23.   void DoubleJump();
24. }
25.  
26. public class Jumper : MonoBehaviour, IJumper
27. {
28.   private double JumpStrength { get; set; }
29.  
30.   // Implement Jump and DoubleJump ...
31. }
32.  
33. public class DisabledJumper : IJumper
34. {
35.   public void Jump() { }
36.   public void DoubleJump() { }
37. }
38.  
39. // Similar implementation for IDasher

My reasoning: GetComponent<T>() returns null when the GameObject does not have a component of this type so the null coalescing operator will work fine.

I changed the Disabled implementation to not extend MonoBehaviour since it doesn't need to work on the GameObject anyways. I think this allows me to new up an instance.

If a GameObject doesn't have one of the components used by this Manager script, the Manager will simply new up a disabled version of the component.

 

Feast, I went back and read your second post -- the one with your implementation of IJumper. Just judging by the names of your methods, it sounds like you are exposing too much capability on this class.

When I hear the component name "Jumper", I think of a component which will make my character jump. I expect it to have pretty simple commands like Jump, DoubleJump, etc.. I'd be really surprised to get this component and have to call all these crazy things like CastRayToGround and OnJumpInputStayPressed just to make things jump around..

I'm starting to echo Kiwasi's original concerns. Your component is exposing too much of its inner workings.

 

Ok, I'm going to use the interface method, thanks for all the help eisenpony!

Right now, I am pretty much exposing every method of the Jumper class since they have to be called from the Manager class. If I put more than 1 functionality into an exposed method it starts to be obscure what the method does exactly.
I wanted to be able to look simply at my Manager class and understand everything from that script only (so that I don't have to look into the Jumper class for example).

That's why I didn't want to use an Update() method for casting rays in the Jumper class. So that I always know for sure that there is no Update() method within all the smaller classes.
The smaller classes are only providing methods, they're not running anything. The manager does that.

My jumper class has practically no inner working because of that, maybe a private method or 2 (that are used in a public method).My smaller classes are libraries. Is that a bad way to proceed?

Now that I'm using the interfaces I would like to expose the properties in the editor. However I don't believe Unity lets you do that. https://docs.unity3d.com/ScriptReference/ISerializationCallbackReceiver.html That's an overly convoluted way of doing it. Is there an easier way to expose properties?

Code (CSharp):

1. public interface IJumper
2. {
3.     float Gravity { get; }
4.     int CurrJumpNr { get; }
5.     bool PostApex { get; }
6.     float DistanceToGround { get;}
7.     float TimeBeforeHitTheGround { get;}
8.     float PreApexMoveXMult { get;set; }
9.     float PostApexMoveXMult { get;set; }
10. }
11.  
12. public Jumper : MonoBehavour, IJumper
13. {
14.     // I want to expose this variable to the Editor.
15.     // This doesn't work
16.     [SerializeField] public float PreApexMoveXMult { get; set; }
17.     // I'm forced to use { get; set; } because of the interface IJumper
18. }
19.  

 

I just realized that if I use interfaces with fields it's not very optimal anymore since having fields will take up memory for nothing. Does using empty methods take up memory as well?

Maybe it's not a big deal however?
I could have 50 fields that are not useful on a simple entity in my game. If there's a lot of those entities doesn't it end up being a problem?

Code (CSharp):

1. public class DisabledJumper : IJumper
2. {
3. // Takes up memory!
4.     public float Gravity { get; private set; }
5.     public int CurrJumpNr { get; private set; }
6.     public bool PostApex { get; private set; }
7.     public float DistanceToGround { get; private set; }
8.     public float TimeBeforeHitTheGround { get; private set; }
9.     public float PreApexMoveXMult { get; set; }
10.     public float PostApexMoveXMult { get; set; }
11.  
12. // Does this take up memory?
13.     public void ApplyGravity()
14.     {
15.     }
16.  
17.     public void CastRayToGround()
18.     {
19.     }
20.  
21.     public int GetJumpNr()
22.     {
23.         return 0;
24.     }
25.  
26.     public int GetMaxJumpNr()
27.     {
28.         return 0;
29.     }
30.  
31.     public void Init()
32.     {
33.     }
34.  
35.     public void LimitMainVelocityY()
36.     {
37.     }
38.  
39.     public void MinimumCurrJumpNrEqualsOne()
40.     {
41.     }
42.  
43.     public void OnJumpInputDown()
44.     {
45.     }
46.  
47.     public void OnJumpInputStayPressed()
48.     {
49.     }
50.  
51.     public void OnJumpInputUp()
52.     {
53.     }
54.  
55.  
56.     public void CurrJumpEquals0()
57.     {
58.     }
59.  
60.     public void SetPlayerGravity()
61.     {
62.     }
63.  
64.     public float TimeBeforeCharacterHitsGround()
65.     {
66.         return 0;
67.     }
68.  
69.     public void HandleInput()
70.     {
71.     }
72. }

 

Even though the interface requires you to implement the get and set properties, there's no reason you can't also use fields to gain the automatic editor experience. The properties can be implemented with custom code, so you can simply wrap the fields.

Code (csharp):

1. class Jumper : MonoBehaviour, IJumper
2. {
3.   public float preApexMoveXMult;
4.   public float PreApexMoveXMult
5.  {
6.     get { return preApexMoveXMult; }
7.     set { preApexMoveXMult = value; }
8.   }
9. }

Before continuing, I'd like to point out that all of this work has more to do with your decision to expose all the inner workings of Jumper. I'd like to return to one of my original code samples.

Code (csharp):

1. class Manager
2. {
3.   public Jumper Jumper;
4.   public DasherAbility DasherAbility;
5.  
6.   public double jumpStrength;
7.   public double dashStrength;
8.  
9.   public void Execute()
10.   {
11.     Jumper.Jump(jumpStrength);
12.     DasherAbility.Dash(dashStrength);
13.     Jumper.DoubleJump(jumpStrength/2);
14.   }
15. }

See that the manager has to know all about the data required by Jumper. This is an example of coupled code, which is considered bad. The second example reduced the coupling by encapsulating the required data into the submodule.

Code (csharp):

1. class Jumper
2. {
3.   public double jumpStrength;
4.  
5.   // Implement Jump and DoubleJump..
6. }
7.  
8. class DashAbility
9. {
10.   public double dashStrength;
11.  
12.   // Implement Dash..
13. }
14.  
15. class Manager
16. {
17.   public Jumper Jumper;
18.   public DasherAbility DasherAbility;
19.   public void Execute()
20.   {
21.     Jumper.Jump();
22.     DasherAbility.Dash();
23.     Jumper.DoubleJump();
24.   }
25. }

However, if you are removing the parameters of the function calls and replacing them with getters and setters on Jumper, you haven't actually reduced the coupling at all. These two classes are still tightly interwoven and this is precisely what object oriented programming seeks to reduce. When you say

I think you are missing the main benefit of object oriented programming and componentization.

 

You're right. Implementing the properties that way will use up memory since they create backing fields automatically. However, since you are not using them, you could implement DisabledJumper like so:

Code (csharp):

1.     public float Gravity { get { return 0f; } }
2.     public int CurrJumpNr { get {return 0; } }
3.     public bool PostApex { get { return false; } }
4.     public float DistanceToGround { get { return 0f; } }
5.     public float TimeBeforeHitTheGround { get { return 0f; } }
6.     public float PreApexMoveXMult { get { return 0f; } set { } }
7.     public float PostApexMoveXMult { get { return 0f; } set { } }

Technically, yes. You require enough memory to hold the methods and their instructions in memory once. The instructions are stored in a common area and all objects of this type use the same set of instructions.

 

I understand what you mean, it seems to me that there are codes where you need easy iterations over them like when I'm doing custom physics, I need to be able to quickly reorder the various methods around to understand what might be causing the problem. I've been programming for 2 years maybe that's the issue.
When the execution order is simple, using componentization works well. If it's not, it seems to me that you're making your life harder by separating stuff.

Good for the backing fields, maybe it's even better that I also use a static class. This way, it wouldn't matter how many entities use my Manager class, something like this:

Code (CSharp):

1. Jumper = GetComponent<IJumper>() ?? static DisabledJumper();

But apparently static classes cannot implement interfaces. So I would need to create an instance and then point to that instance for every character using the Manager class right? I'll try and figure that out when I'm done eating.

EDIT: With static classes:

Code (CSharp):

1. public class StaticClasses : MonoBehaviour
2. {
3.     public static DisabledDasher NoDasher;
4.     public static DisabledJumper NoJumper;
5.  
6.     public static void Init()
7.     {
8.         NoDasher = new DisabledDasher();
9.         NoJumper = new DisabledJumper();
10.     }
11. }
12.  

Code (CSharp):

1.  private void Awake()
2.     {
3.         StaticClasses.Init();
4.  
5.         Jumper = GetComponent<IJumper>() ?? StaticClasses.NoJumper;
6.         Jumper.Init();
7.         Jumper.SetPlayerGravity();
8.  
9.         Dasher = GetComponent<IDasher>() ?? StaticClasses.NoDasher;
10.         Dasher.Init();
11.     }

 

Hmm I want to be careful here. I would strongly disagree with this conclusion and I hope this thread hasn't contributed to that feeling.

It definitely sounds like your current work would benefit from componentization because of the complexity you are experiencing. So far we have just been making small steps towards this goal but I think at this point we won't make much more progress without significant refactoring of your smaller components.

If you want to continue with componentization, I encourage you to try what Kiwasi hinted at. If you can describe your Jumper interface in much simpler terms, it will really alleviate the complexity you are dealing with in Manager. I think in time you will come to embrace the splitting of classes rather than preferring one large code base from which you can, theoretically, "see how everything works". While splitting code up does hide certain details that might seem important, done right it really frees up our high level classes to deal just with the problem domain.

 

I think I understand the idea behind componentization.
I simplified it up a bit by saying I was doing that just to see how things work. Sometimes I also need that information in other new classes.

I made a tree to explain the idea of my code:
I would like it to be as efficient as can be, but what's the better way here?
(Outlined in blue are the classes that all my entities will/might have)