Is encapsulating logic into separate classes a good approach?

With Unity 2019.3, we got a new tool to play around with being SerializeReferenceAttribute. In short, one thing that this attribute allows us is to create fields with an interface as type, assigning an object into it and serialize it.

An example could be a boolean expression tree:

Code (CSharp):

1. public class Test : MonoBehaviour
2. {
3.     [SerializeReference]
4.     [SerializeReferenceButton]
5.     private BNode root;
6. }
7.  
8. public interface IValueProvider<T>
9. {
10.     T Value { get; }
11. }
12.  
13. [Serializable]
14. public abstract class BNode : IValueProvider<bool>
15. {
16.     public abstract bool Value { get; }
17. }
18.  
19. [Serializable]
20. public class ValueNode : BNode {
21.     [SerializeField]
22.     private bool value = false;
23.     public override bool Value => value;
24. }
25.  
26. [Serializable]
27. public class AndNode : BNode
28. {
29.     [SerializeField]
30.     private BNode[] children = default;
31.     public override bool Value => children.All(child => child.Value);
32. }
33.  
34. [Serializable]
35. public class OrNode : BNode
36. {
37.     [SerializeField]
38.     private BNode[] children = default;
39.     public override bool Value => children.Any(child => child.Value);
40. }

Now if you look on AndNode and OrNode, you will notice that they contain children and logic how to evaluate children. This looks valid to me, however, what if such a node would only contain logic and no other members?

Consider validators in a game. A validator checks if a certain condition is true or false. In one of the easiest of cases, it compares a number with a constant. Let's say I have a validator that checks for "If my player character has more than 50% health". Such a validator could look like this:

Code (CSharp):

1. public class ValueValidator : ScriptableObject
2. {
3.     [SerializeField]
4.     private ValueCompareOption option;
5.  
6.     [SerializeField]
7.     private float value;
8.  
9.     public bool Validate(float other)
10.     {
11.         switch (option)
12.         {
13.             case ValueCompareOption.Unknown:
14.                 Assert.IsFalse(option == ValueCompareOption.Unknown);
15.                 return false;
16.             case ValueCompareOption.LesserThan:
17.                 return other < value;
18.             case ValueCompareOption.LesserEqualThan:
19.                 return other <= value;
20.             case ValueCompareOption.NotEqualTo:
21.                 return other != value;
22.             case ValueCompareOption.GreaterEqualTo:
23.                 return other >= value;
24.             case ValueCompareOption.GreaterTo:
25.                 return other > value;
26.             case ValueCompareOption.EqualTo:
27.             default:
28.                 return other == value;
29.         }
30.     }
31. }

As you can see, the logic is hardcoded into a switch case here. What if I wrote it like this?

Code (CSharp):

1. public interface IComparator{
2.     bool Validate(float value, float other);
3. }
4.  
5. public class ValueValidator : ScriptableObject
6. {
7.     [SerializeReference]
8.     private IComparator comparator;
9.  
10.     [SerializeField]
11.     private float value;
12.  
13.     public bool Validate(float other)
14.     {
15.         return comparator.Validate(value, other);
16.     }
17. }

Using something like SerializeReferenceButton, I could now choose the class that implements the interface IComparator, allowing for great scalability and greatly reducing the complexity of the validator himself. Now the question, is this a bad design? Creating classes that do nothing but provide "dispatching some logic", if you get what I mean. They basically do nothing by themselves. Are there drawbacks that one should consider or other points to think about?

 

OMG, this is the first time I'm hearing of this new attribute. AND OMG I AM SO HAPPY UNITY FINALLY GAVE THIS TO US! I will definitely be integrating this into my future projects rather than using my wonky workarounds that I've been carrying for years now.

Personally I don't think it's an inherently bad design. It's a very plausible and useful design that I've used, and many have used, through out the years in other software. This is basically a version of the 'command pattern':
https://sourcemaking.com/design_patterns/command

It is definitely a very object-oriented approach. And the doing it via the inspector rather than through some factory pattern is a more novel way of approaching it. But the end result is very useful.

An over use of this pattern though can lead to some confusing code for those unfamiliar with the pattern. And also add layers to the testing of the code making maintenance slightly annoying. But I'd argue the switch statement would also have the same issues (I personally find it easier to unit test these IComparators than the switch case, but that's me).

 

Thank you for your response!

From personal experience, I can recommend a browse through SerializeReference Attribute?, it shows some chances and limitations as well as some insights into what it is. People still occasionally post in it, too.

Thanks for pointing me to that pattern! When I read about [SerializeReference], this was the first thing that came to my mind, being able to select the logic from an object instead of hardcoding it into something like switch-cases or ELIF-statements. Just needed to know how it is called, so thanks!

This pattern looks very easy to test to me, in comparison to a switch-case. Sure, one would have to test every single implementation of IComparator, however, this appears to be a lot more organized to me than a couple unit tests around switch-cases. It is also the scalability that I eyeball here. It is very easy to add and use new implementations for IComparator, without the need to touch existing stuff, as well as re-use these objects everywhere you need instead of having each object their own switch-case.

 

I find your use case somewhat complicated, but I'm not sure if I'm qualified to even perceive the ways people use Unity nowadays. Or I just can't figure out what's your intention with such validators and comparators. What's the point of having any of it serialized, or having deserialized stuff validated. In what context?

Anyway, maybe that's just me (it's probably just me so nvm), in the meanwhile I'm definitely happy that SO's and MB's can finally serialize reference topologies. That's really huge (with a great potential to be misused).

 

Well for starters it can allow easier compositing in ScriptableObjects. Imagine how you can extend functionality by adding components to a GameObject. What if you could easily do that in a ScriptableObject without having your SO have to reference another SO. Instead you create an "Item" SO and then add extensible attributes to that Item via the inspector.

Next where the 'command' pattern really comes into play is it allows adding new logical extensions to something down the line. Lets say you created a system for release on the app store and you had gone with the "enum/switch" method and you released it. But now someone using your product wanted to add a new comparison operation... well just by implementing the IComparator interface they have a new comparison available to them without having to go and modify your code (editing 3rd party code is cumbersome if you had to get an update, or what if the code was distributed pre-compiled).

Basically it facilitates a composition based approach to design over an inheritance approach. And a huge benefit of composition is getting to swap out functionality without having to rebuild the object. A class can maintain state while the composited logical commands and modify behaviour.

I'll give you a real world example.. and this is actually something I plan to use this for because I hated having to use Components for it. So first an image:


This here is a boss in our game called the "Hankenfisch/Frankenfisch" (it's a frankstein like monster of our main character Hank. It's basically a bunch of Hanks swen together into a huge giant beast monster).

It's AI is a basic state machine made up of a root "Brain" which acts as the state for the entity (note the variables on it).

It has a few states like:

Idle:


It has some simple logic scripts that dictate how this state behaves. It basically idle's around and sense's for something tagged "Player" via its "Eyes" set that to the "Target" variable and enters the "Pursuit" state.

Pursuit:


This one uses Pursuit logic that pursues the "Target" variable with a few other parameters. But also has an "Logic Selector"... this is a sub-selector that selects from a collection of attacks, Far-Mid, Near-Mid, Near-Left, etc etc etc etc.

Lastly there is the "Dead" state:


Not a whole lot going on here.

...

Note this boss has a very basic AI brain to it. It's why I selected it, there's not a whole lot to this big lumbering beast as the encounter is more narrative structured than difficulty structure (it's suppose to be the moment in the game you realize your character is actually a clone, and every time you died and come back, it's really just another clone of yourself. This is when it clicks that all the zombies you fought up to now were decomposing versions of you and your partner Cass and you've been stuck on this island for 30 years. A lets-play video if you're interested). I didn't want to cloud up this post with an AI tree that's super complicated when that's not really the point of the post.

Anyways....

This design actually requires a lot of nested GameObjects.

NOW, I could have written a super complicated custom system that allowed for customly serialized AI trees just like many third party systems we've found out there. But we didn't necessarily have the time/resources to do all that. And so instead this was the approach we took with the tools we had available to us via Unity.

...

With this new attribute, and using the "Command Pattern". Imagine if instead we had this.

A component:

Code (csharp):

1. public class Brain : MonoBehaviour
2. {
3.     public LogicState[] States;
4. }

A ScriptableObject:

Code (csharp):

1. public class LogicState : ScriptableObject
2. {
3.     [SerializeReference]
4.     public ILogic[] Logic;
5. }

And the same exact ILogic interface I used in the Hankenfisch:

Code (csharp):

1.    public interface ILogic
2.     {
3.  
4.         //bool enabled { get; }
5.         bool isActiveAndEnabled { get; }
6.  
7.         void OnEnterState(IAIController ai, IAIStateMachine machine, IAIState lastState);
8.         void OnExitState(IAIController ai, IAIStateMachine machine, IAIState nextState);
9.  
10.         /// <summary>
11.         /// Tick logic update cycle.
12.         /// </summary>
13.         /// <param name="ai"></param>
14.         /// <returns></returns>
15.         ActionResult Tick(IAIController ai);
16.  
17.     }

(this is a copy paste of the ILogic interface from that project)

Now instead of this complex GameObject hierarchy. I could create the very same hierarchy as a single component and a list of ScriptableObjects on that component.

I could recycle ScriptableObjects.

Some references might move around (like the animator references), but that's mostly just data orientation and not super complicated to deal with as a refactor goes.

 

Thanks for that, I got the example, especially in the second half where you summarized the approach. Yes you couldn't do this without references to already live systems that are integral to your project.

But this is really the crux of where we differ in our methodology. I'd never ever make a Hankenfisch of my project, pardon the pun, and would realize this behavior on some other grounds. And I can't tell if your approach is better or not, so that serves as an indicator to me that I'm not sufficiently knowledgeable about this. Or maybe I just didn't have enough practice with the whole shebang in a live project -- I like to code gritty down-to-earth stuff anyway -- so who knows, maybe this will also grow with me in time (like Linq grew on me eventually, though I still don't use it lol I mostly love it because of the features that got implemented because of it).

I just find this really adorable because you can preserve some complicated cycling graphs, cross-referencing, and basically save a lot of space without compromising the data structure (think serializing deep inventory systems without having to ID the items, and then not having to recreate the actual references in deserialization; this is massive).

I like the lo-fi Alone in the Dark / Resident Evil vibe of that game btw. Nice.

 

It's just visual scripting, right? The same as Scratch or what UnReal has long had. It's nice when you need some level-design logic done by a non-programmer, or simply tweaked by one.

 

Pretty much. I create basic parts, designer can wire them together to get different behaviour.