Polymorphism on Scriptable Objects

Hi everyone,

let's say I wanted to create an inventory system. I would have a few scriptable objects like:
ItemBase and Consumable:ItemBase for example. I create a ConsumableItem in the Editor and drag it onto the OtherClass script in the editor.

Code (CSharp):

1. public class ItemBase : ScriptableObject{
2.     [SerializeField] private string itemName;
3.     public string ItemName{get{return itemName;}}
4. }
5.  
6. public class ConsumableItem : ItemBase{
7.     [SerializeField] private string hpToAdd;
8.     public string HpToAdd{get{return hpToAdd;}}
9. }
10.  
11. // added to a gameObject in the scene like the mainCamera so it get's executed on play
12. public class OtherClass : MonoBehaviour{
13.     public ItemBase myItem; // a consumableItem scriptable object is dragged in here in the editor
14.  
15.     void Start(){
16.         printItemName(myItem);
17.         printHpToAdd(myItem);
18.     }
19.  
20.     void printItemName(ItemBase item){
21.         print(item.ItemName); // will work because it's a prop of ItemBase
22.     }
23.  
24.     void printHpToAdd(ItemBase item){
25.         print(item.HpToAdd); // won't work because function is taking ItemBase as a param and not ConsumableItem
26.     }
27. }

Even if I used a item database I would always have the mismatch between types.
How is this done correctly?

 

That's not what polymorphism is.

Polymorphism is allowing you to treat ConsumableItem as a ItemBase... not the other way around, like you want.

'OtheClass' doesn't know if ItemBase is a ConsumableItem or not. The variable/field 'myItem' is typed as ItemBase, as a result you could have dragged anything is an ItemBase (or inherits from ItemBase) that isn't necessarily a ConsumableItem.

If 'OtherClass' expects 'myItem' to be a ConsumableItem, then type the variable as ConsumableItem.

Code (csharp):

1.  
2. public class OtherClass : MonoBehaviour {
3.     public ConsumableItem myItem;
4. ...
5.  

If 'OtherClass' expects 'myItem' to be a ItemBase, BUT has optional functionality if it just happens to be a ConsumeableItem do something like this:

Code (csharp):

1.  
2. void printHpToAdd(ItemBase item){
3.     if(item is ConsumableItem) print((item as ConsumableItem).HpToAdd);
4. }
5.  

... mind you I don't get why you're printItemName and printHpToAdd methods are private and take in an ItemBase. Those methods are private and local to 'OtherClass'... they could just access the local field/variable. Anyways, that's a semi-related tangent.

 

Can also read about just regular polymorphism: inheritance, overloading, abstract base classes. It's a general topic, so you're not stuck reading only Unity sites.

That type of confusion is common -- the first time someone sees something is in Unity, so they check Unity sources for it; not knowing it's in common use and Unity is using it the normal way.

 

While that works, I would generally caution against checking for specific subclasses except as a last resort. Ideally, the class (e.g. OtherClass) that's making use of the base class (e.g. ItemBase) shouldn't need to know about any of the derived classes (e.g. ConsumableItem) in order to function; rather, the derived classes should have overrides that handle the different functionality. This will help keep your code organized.

The provided example code is not a good example of this, which is to say, OtherClass shouldn't have a "printHpToAdd" method at all. If this is part of some supplementary data that different derived classes might have, then you'd want ItemBase to have a PrintSupplementaryData() function (probably empty), and then override that in any derived classes to output whatever supplementary data is appropriate to that class.

Code (csharp):

1.  
2. public class ItemBase : ScriptableObject{
3.     [SerializeField] private string itemName;
4.     public string ItemName{get{return itemName;}}
5. public virtual void PrintSupplementaryData() { }
6. }
7.  
8. public class ConsumableItem : ItemBase{
9.     [SerializeField] private string hpToAdd;
10.     public string HpToAdd{get{return hpToAdd;}}
11. public override void PrintSupplementaryData() {
12. print(HpToAdd);
13. }
14. }
15.  
16. // added to a gameObject in the scene like the mainCamera so it get's executed on play
17. public class OtherClass : MonoBehaviour{
18.     public ItemBase myItem; // a consumableItem scriptable object is dragged in here in the editor
19.  
20.     void Start(){
21.         printItemName(myItem);
22.         printSupplementaryData(myItem);
23.     }
24.  
25.     void printItemName(ItemBase item){
26.         print(item.ItemName); // will work because it's a prop of ItemBase
27.     }
28.  
29.     void printSupplementaryData(ItemBase item){
30.         item.PrintSupplementaryData(); //will print nothing for BaseItem, but will print the hpToAdd for ConsumableItem
31.     }
32. }

Exceptions to this guideline might be if you don't have control over the classes (e.g. they came from a package you don't want to or can't modify), or if you need some data from the current scope that the classes don't have (though even then a virtual method with a parameter is usually preferable.)

 

I agree @StarManta, it's definitely smelly for OtherClass to do that. It's why I offered it up as the secondary "but" scenario.

I would have liked to go into a lengthy diatribe as to why it's smelly, but alas, at work today. So my posts are shorter.

 

Why do you think mine are so long?

 

In the spirit of learning, especially about things that are not Unity specific...

One technique for dealing with the specific problem you identified is Double Dispatch. You can read about it here: https://lostechies.com/derekgreer/2010/04/19/double-dispatch-is-a-code-smell/

If you even got as far as the url, you'll notice that double dispatch is considered bad practice. The reason is it causes an explosion of modifications whenever you go about adding new types.


StarManta's advice is better; rather than requiring your "controller" class to know what it's dealing with, think about the common tasks all items will be able to do. For items, I think of "using", or "apply the effect".

It might be confusing how to adapt the simple example to a real implementation, so let me take it one step further.

Very likely, you won't be interested in "printing supplementary data", but actually doing something. Doing things in Unity typically requires GameObjects, so let's define our interface to require some.

Code (CSharp):

1. public abstract class ItemBase : ScriptableObject{
2.     [SerializeField] private string itemName;
3.     public string ItemName{get{return itemName;}}
4.     public abstract void Apply(GameObject user, GameObject target);
5. }
6.  
7. public class ConsumableItem : ItemBase{
8.     [SerializeField] private string hpToAdd;
9.     public string HpToAdd{get{return hpToAdd;}}
10.  
11.     public override void Apply(GameObject user, GameObject target)
12.     {
13.       var hp = target.GetComponent<IHPSystem>();
14.       if (hp != null)
15.         hp.Heal(HpToAdd);
16.     }
17. }
18.  
19. // added to a gameObject in the scene like the mainCamera so it get's executed on play
20. public class OtherClass : MonoBehaviour{
21.     public ItemBase myItem; // a consumableItem scriptable object is dragged in here in the editor
22.  
23.     void Start(){
24.       myItem.Apply(this.gameObject, this.gameObject);
25.     }
26. }

This of course assumes another component: the HPSystem. A simple implementation might look like this:

Code (csharp):

1. interface IHPSystem
2. {
3.   void Heal(int amount);
4.   void Damage(int amount);
5. }
6. class HPSystem : MonoBehaviour, IHPSystem
7. {
8.   [SerializeField] private int hp;
9.   public int HP => hp;
10.  
11.   [SerializeField] private int maxHp;
12.   public int MaxHP => maxHp;
13.  
14.   public void Heal(int amount)
15.   {
16.     hp = Mathf.Max(hp + amount, maxHp);
17.   }
18.  
19.   public void Damage(int amount)
20.   {
21.     hp = hp - amount;
22.     // Here is a great place to test if hp <= 0 and trigger an event
23.   }
24. }

If you're paying close attention, you might realize it would be useful to disallow the use of an item when targeting something without an HPSystem. Or even when the target doesn't need the item's effect. You could use an exception or error code in the Apply method, but there is a better option to avoid this vexing problem.

Let's modify our item interface slightly:

Code (CSharp):

1. public abstract class ItemBase : ScriptableObject{
2.     [SerializeField] private string itemName;
3.     public string ItemName{get{return itemName;}}
4.     public abstract bool TryApply(GameObject user, GameObject target, out string failureReason);
5. }

Now we can do the following:

Code (csharp):

1. public class ConsumableItem : ItemBase{
2.    [SerializeField] private string hpToAdd;
3.     public string HpToAdd{get{return hpToAdd;}}
4.  
5.     public override bool TryApply(GameObject user, GameObject target, out string failureReason)
6.     {
7.       var hp = target.GetComponent<IHPSystem>();
8.       if (hp == null)
9.       {
10.         failureReason = "The target cannot be healed because it cannot be damaged!";
11.         return false;
12.       }
13.       else
14.       {
15.         hp.Heal(HpToAdd);
16.         failureReason = "";
17.         return true;
18.       }
19.     }
20. }
21.  
22. // added to a gameObject in the scene like the mainCamera so it get's executed on play
23. public class OtherClass : MonoBehaviour{
24.     public ItemBase myItem; // a consumableItem scriptable object is dragged in here in the editor
25.  
26.     void Start(){
27.       if (!myItem.TryApply(this.gameObject, this.gameObject, out var error)
28.         print(error);
29.     }
30. }

Just the addition of a few common pieces of data, and a simple method will open up a lot of new use cases. I hope that shows how rethinking the design to prefer calling methods on the base class, passing in the potentially useful information, and hiding the implementation in the derived classes keeps things much simpler than trying to know what concrete type your "controller" class is holding.

 

Hey @eisenpony @lordofduct @Owen-Reynolds @StarManta ,
honestly I'm surprised about how fast and detailed your replies came in. I've seen much less supportive forums. So thanks so much!

I admit the example I provided is not ideal. I see that now. It was meant to make a point and to give repliers at least something to work with, without posting the whole inventory system in the forum.

I will read through your replies soon.

 

This can introduce quite some logic to the items though.

And unfortunately, there's the problem that "using an item" does not necessarily imply one certain kind of effect / action, i.e. there's not just "it does work" and "it does not work", and even for an action, it'd likely be possible to play out differently depending on the target. If you wanted to cover all that in item classes, it'd clutter the items really quickly.

Sometimes these problems are not noticeable from the beginning, and become a problem later down the road.

 

Hi Suddoha, how are things?

It's been a while since we could debate. Okay, I'll bite!

Quite right, and for the short term, I think that's precisely where that logic should be. Once I have a database of dozens of items, I may notice some common patterns and refactor to use a common "targeting system" or something like that. If I do though, I'm more likely to inject it into those items that need it, or use the decorator pattern to wrap the items with "AllyTargetableItem" or something like that. I'd prefer to stay away from moving that responsibility to the "controller" class, especially if it's not 100% common to all possible items. To mee this is the best pragmatic choice.

I'm not 100% sure I know what you're getting at, but a binary "worked or didn't work" seems pretty good for any item's I can think of. Even if an item was only partially used, such as using an item which heals hp and mp on a character that only requires hp. Besides that, my suggestion actually provides more information in the case of "didn't work", so it's not really just binary.

Again, I think I need more context to understand your point. Also, injecting components that contain that logic into the items that need it seems like a simple way around your proposed issue.

Quite right. However, I think my suggestion strikes a good balance of forethought and pragmatism. I don't have the same experience you guys do building games, but this seems like a good solution to the LOB App developer in me. Still, I'd love to hear any hard-earned nuggets of wisdom others have from building their own item systems!

 

That's certainly a way to implement it. There's no wrong or right.

However, now the items need to know how to to resolve the components. What if the health wasn't implemented as a component? I'm aware that's just an example, but at the moment it sort of dictates the requirements for the consumers.

Also, given that the consumer is likely the one who "uses an item", it seems a little odd to pass itself as "some target" of which the actual component has to be resolved. I mean, why attempting to apply an item when the consumer may already have the information whether or not it can use that item in the first place?

So, if we suppose that the consumer is the one using the item (directly), it'd make more sense to pass at least IHPSystem as the argument:
- no knowledge of how to resolve the target component
- no knowledge of whether or not the system is a component
- no "target cannot use it" cases (I'd argue an item should be doing that)
- no calls to items that do not support it, unless it accepts such argument and still "doesn't support" it

Of course, this might require more work on the callers side in order to manage different item types, as you cannot have that one method signature for everything.

But at this point, we could already do the following

Code (csharp):

1. private int _hpRecovery = 20;
2. // ...
3. public void TryApply(IHPSystem system)
4. {
5.      if (system == null)
6.      {
7.          // throw due to wrong usage, or report "false" using the try-pattern
8.      }
9.    
10.      system.Heal(_hpRecovery);
11. }
12.  

Now when you look at that, it's almost like we're running an extra-mile for nothing. Instead, we could get the HP through a property and apply it in our own specific way.

The following circumstances may support that decision:
- entity has full hp
- entity is dead
- entity cannot heal at the moment

These cases have to be handled somewhere and that's likely not an item's responsibility. So we could move the entire "apply health" process to something that has all the information. There's much more to applying HP than just "add X HP", which the item cannot satisfy without all the extra information that it shouldn't know about.

Consider a game with debuffs. A specific item's effect might be weakened, useless, inverted... That might no longer be a trivial example, but usually games start out very simple, but as the time goes by new features are introduced. So now the calling instance needs to evaluate which item it's dealing with, and with that in mind, it needs to intercept and modify the values before they are applied.

And the case you proposed attempts to avoid a vexing exception using the try-pattern, but even the try-pattern is sort of fixing a problem that shouldn't occur in the first place, IMO. Reason being that it yields information about whether the argument is capable of providing the type-based functionality, whereas the arguments can be constrained using a specific type.

It's very common in Unity to pass GameObjects around, and let other types use GetComponent to resolve the components it needs. That tends to be very flexible in many cases, but it also moves more responsibilities to those components and systems that they wouldn't need to care about when they just receive an argument of the required type.

 

Hi again. Sorry for the length of silence, I've been really busy at work

I've been eager to get back to you though. I think you made some interesting points but I want to defend my line of reasoning. I'm eager to see if you have any more counterpoints!

Touché. You've caught me using a service locator .. Normally I steer well clear of these and prefer to inject the dependency needed. Just the solution you suggested .. However, I'm stuck on this point. Let me explain.

Items seem like one of the kinds of things I want to be able to easily extend. I don't want to imagine all possible types of items right now but prefer to put that decision off. I need adding items to be easy, which means I need adding item dependencies -- subsystems like IHPSystem -- to be easy.

I also need to deal with late-runtime-binding. In a game, I don't know who the target of my item / skill / attack is going to be in advance. The targeting must be done at runtime as a requirement.

The pattern that I think best fulfills these two requirements is service locator. Obviously, there is a cost.. For service locator, it's something I call "implicit dependencies" -- you can't see the classes dependencies by inspecting its constructor. You have to read the rest of the code. BLECH.

However, I think we can mitigate that problem by following a convention of grabbing all dependencies in guard clauses at the top of our item's activation code.

To defend against your other comments:

this is the implicit dependency problem. I'm okay with taking that hit if it means my items are more extensible. If your complaint is specifically about GetComponent, because you want to avoid knowledge of Unity technology, that can be remedied with an adapter that does the "service locating".

I don't think this is a problem. At least not a problem on its own. Its like a combination of your previous point and your next point

I still think this is an excellent use case. I took advantage of that all the time back in my days of playing Final Fantasy. Just spam potions on your party from the menu until the game prevents you from using them because the target is at full health. Mashing buttons instead of paying attention is a staple of my game playing.

This is really just your first point again. Yes, I admit my items must take on the responsibility of "getting their dependency". It's not ideal, but it saves me from either having complicated caller code or a dependency mapping subsystem.

This bit I really have to take issue with. Giving my HPSystem an HP property locks down my design and bleeds all the details into the calling code.

For instance, maybe I want a system like this.

Code (csharp):

1. public class Invulnerable : IHPSystem
2. {
3.   public void Heal(int amount) { }
4.   public void Damage(int amount) { }
5. }

Or like this

Code (csharp):

1. public class Mechanical : IHPSystem // Machines can't be healed
2. {
3.   private int Hp { get; set; }
4.   private int MaxHp {get; }
5.  
6.   public void Heal(int amount) { }
7.   public void Damage(int amount)
8.   {
9.     Hp -= amount;
10.     // check for Hp < 0 and call relevant events
11.   }
12. }

Precisely why we can't expose an Hp property, the calling code shouldn't know about these details either. The HPSystem should be the only one that knows that.

In this case, there would be additional systems which the items act on. Maybe an IDefenseSystem or an IStatsSystem, etc... The point is that the responsibility for the additional effects should lie in a component of its own. Not in the item, granted, but certainly not in the calling code either.

I gave only one example why an item might not be usable. There could be others, in which case the subsystems would need to provide ways to check those. For instance, we might add a bool WillAcceptHeal() method to the IHPSystem.

Quite right. It does spread the service locator pattern quite a bit and I'm in favour of avoiding that where possible. I'm just not sure how to achieve the late-runtime-binding feature without either the locator pattern or a sophisticated, reflection based framework that can identify the late dependencies and resolve them (using a service locator anyways .. just insulating other components from it)

None-the-less, it is a common pattern, and I think for good reason: it is the pragmatic choice. The consistency of the Unity component system, plus a little coding convention, can keep the implicit dependency problem under control while providing the benefit of late-runtime-binding.