Is creating a Scriptable Object for each child class a good practice ?

Hey everyone !
I have come across a post regarding having a single instance of a SO, by addind the CreateAssetMenu attribute then commenting it out.

I wanted to ask someone about something I’m doing, because I don’t know if it’s a good or bad practice.

Basically I have :

  • an abstract CardData class that derives from ScriptableObject
  • an CardHolder class that derives from MonoBehaviour, that reads CardData

And I’m planning to have all my cards deriving from CardData, and for each of them adding the CreateAssetMenu attribute then removing it once I created the .asset.

On the paper, it works, but it represents so much pain in Unity that I can’t help thinking it’s a bad practice. And on the other hand I don’t see a better way of doing it.

Is this a workflow you would recommand/find good ?

Thank you for replying !

One thing to consider is do you actually need a scriptable object for each CardData if you only have one of each kind. If you are not creating multiple variations of the same card type that only differ in data or you don’t need to store references to other assets (like prefabs or sprites) maybe you can just use regular classes with no assets for your card system.
Even if you need references to other assets that could be solved using string based asset lookup, downside is that it’s much easier to make mistakes since it will be less obvious when things break due to typo or renaming the asset. You will also not be able to leverage unity automatically including required assets based on reference chain. First problem can be mitigated by validation script or test which goes through all your cards and check if string based asset references have corresponding asset. These tradeoffs are somewhat similar to programing language with strict typing versus scripting language with no or much weaker type checking. It’s up to you to decide whether reducing friction for some steps are worth the increased risk of some mistake types.

Different way that could potentially improve your workflow of creating a SO asset and then removing context menu action is having a single script which creates SO assets for all classes inheriting from your CardData base class unless there is one already. That way you would avoid adding/removing context menu button each time. You could even write a bunch of scripts an then activate command once, instead creating SO assets one by one. Structure of such script would consist of these parts:

  • Create generic menu/context menu button using ContextMenu attribute
  • Gather all subclasses of CardData class by iterating over all types in assembly and checking if they inherit from your carda data baseclass
  • generate path of asset based on class name and check if you already have previously created one for this type
  • Create SO object from the type using ScriptableObject.CreateInstance(Type type) method and then save it as asset using AssetDatabase.CreateAsset

Depending on your needs step 3 could also be done based on type instead of assuming predefined path for each type, thus allowing you to rename created assets.
In cases you decide to make multiple variants of some CardData type you can just copy existing SO asset (asset copying feature is added in Unity 2021). Still no need to temporary create context menu item.

Thank you for your detailed answer. I perfectly understand it.

For a long time I have been using MonoBehaviours for Cards, and prefabs. But since the cards are 3D, dealing with UI was quite a pain, so I decided to also have a flat texture to put in the menus, and I thought that having it as a property of a SO would be cleaner than using an non-instantiated prefab’s property.

This being said, I also use a SO as a database. It stores a list of CardData, UnitData, etc, and when saving and loading I iterate through the corresponding list to save data based on strings or ints. I think having SO for cards would also be easier for me to deal with, so I might stick to adding the [CreateAssetMenu] attribute then commenting it out.

From what I understand from your reply, it is inconveniant for sure, but absolutely not wrong, am I correct summarizing it this way ?

I would definitely not do it the way you’re looking to do it, as that would become a maintenance nightmare. Remember: composition over inheritance.

I can suggest two approaches. One will work out of the box, the latter will require either plugins, or custom inspector work.

The first would be to socket behaviour using other scriptable objects. You can define certain card abilities using scriptable objects, and use these in a list of card effects. Each ability you’d keep small in scope, such as draw x cards or deal x damage, of which you can make instances of and reference those in the cards themselves. Though as the abilities rack up these can fill up your Right-Click menu (though smart nesting can mitigate this).

Secondly, and my personal preference, would be to use SerializeReference to socket behaviour into your Scriptable Object cards. This is similar to the above bit without the need to make SO’s for each effect, using plain Serializable classes instead. While Unity supports the serialisation, it doesn’t support the drawing of the fields, so you’ll either need to do that yourself, or use plugins to take care of that for you.

1 Like

Thank you for your answer !
I definitely understand your point, and I think it would fit a lot of card games, but in my case I feel like it would be a nightmare.

The game I’m working on is not a deckbuilding game like Slay the Spire : cards cover movement on a map, and many other aspects, so in the end I would need to have multiple specific SOs as well.

For the second proposition, I am not 100% sure to understand the benefit of it to be honest. Currently, I have an abstract CardData class that holds many protected functions, such as Draw(X), Discard(X), Move(X), Dash(X) and so on. What I usually do is something like the following :

public class DummyCard : CardData

public override void OnPlay()
{
      Draw(2);
      Heal(1);
}

And I think once it’s set up it’s actually conveniant to use, so I am not really sure about how [SerializeReference] would not overcomplicate this ?

I guess what I could do is having a CardData that inherits from ScriptableObject, storing data such as name, description, artwork, and having a class for each card as I currently do, that reads the CardData.

Something like this :

public class CardData : ScriptableObject
{
   [SerializeField] private string _name;
   [SerializeField] private Sprite _artworkd

   public string Name
   { get { return _name; }

   public Sprite Artwork
   { get { return _artwork; }
}
public class Card : MonoBehaviour
{
   [SerializeField] private CardData _cardData

   public CardData CardData
   { get { return _cardData; }

   public virtual void OnPlay()
   {
      Debug.Log($"You just played {CardData.Name} !");
   }
}

Would’nt it be good ?

I was just about to mention the first approach spiney mentioned as alternative. One aditional benefit of it that wasn’t mentioned is that makes much easier to generate in game UI describing effect of card or making predictions of card’s effect ahead of time. It can avoid the need of having manually written card description and keeping description text in sync with actual behavior. Different benefit is that it also allows more easily support user generated content and mods in a relatively robust and safe way.

I prefer avoiding dealing with absolute rules and calling something always wrong or always correct. More often it’s question of complexity and scale of things you are operating at, your skill level and willingness to deal manual work, willingness to learn new things and develop tooling. Especially in programming there can be huge difference in magnitude of size for the problems people are dealing with, so one size rule rarely fits all.

This reminds me typical example of spending two days to write a script for automating work that would have taken 30 minutes to do manually. If you have to do it once or twice a year maybe the automation wasn’t worth it. But if it needs to be done every day for next year by team of multiple people even few minutes wasted on cumbersome workflow can add up.

It’s also easy to fall into pitfall of effectively developing your own scripting language.

I personally would prefer separating gameplay rule logic from MonBehaviors that are used to visualize the game. That way I can easily execute tests independently of any UI stuff and core Unity execution loop. Downside is that it either: limits your ability to use custom visualizations of the effects, pushes you into reinventing bicycle and creating custom animation system or makes it more difficult to synchronize timing of animations with functional aspects of the card effects.

But most importantly don’t give up and keep pushing forward with creating your game. Especially at indie developer scale it’s possible to make good game even with terrible code structure and mediocre programming skills as long as you have great idea, persistence, and mindset of working within your limitations. At some point in future it may prevent you to implement some features (trickier stuff like multiplayer, replays, saves at arbitrary point, preview) forcing you to either not have them or do a huge rewrite.
Bad architecture is more critical for medium and large teams, where people need to effectively split and combine their work, process inefficiencies accumulate and multiply, redoing large portions of the work is prohibitively expensive. Problems can’t be overcome with just persistence, resulting in progress completely halting, project being canceled or extremely buggy product.

I recommend to look up the Unity talk about Game Architecture with Scriptable Objects. It shows you how you can use them. but there is actually no right or wrong. If you can use them for your adventage, do it.

Thank you for your detailed and precious answers. I will absolutely take note of everything you said.

Currently, I am rushing to meet deadlines, and even though I now, thanks to you, see how to achieve a better architecture, I think I will stick to the one I have at the moment to finish the first slice of the game, before I start rewriting this big chunk.

As a wise friend told me “one of your product’s core features is also to be delivered on time” :smile:

But I definitely see how I can improve my architecture in the future, so once I’m done with this playable slice, I will organize things better, as you advised !

All of these functions could easily be, and probably should be, shunted into static helper classes.

The inspector work would take the most time if you don’t use a plugin. Otherwise I would wager it would save you a lot of coding, allowing you to reuse effects and add new effects with little effort.

Random example:

//Base class for card effects
[System.Serializable]
public abstract CardEffectBase
{
    public abstract void ActivateEffect();
}

//example effect
[System.Serializable]
public DrawCardEffect : CardEffectBase
{
    [field: SerializeField]
    public int CardDrawAmount { get; private set; } = 1;

    public override void ActivateEffect()
    {
        CardHelperMethods.DrawCards(cardDrawAmount: CardDrawAmount); //drawing cards via helper class
    }
}

//how the effects would be implemented
public Card : ScriptableObject
{
    [SerializeReference]
    private List<CardEffectBase> _cardEffects = new List<CardEffectBase>(); //polymorphic list of effects
   
    public void PlayCard()
    {
        foreach (CardEffectBase cardEffect in _cardEffects)
        {
            cardEffect.ActivateEffect();
        }
    }
}

Super rough example of course. But with this method you’d A: Only need only one class for the cards, and B: free to add new effects with little cost. You just make new card assets, and add the effects to the list.

1 Like

Bumping this topic real quick to share what I’ve done this afternoon, that can be quite useful for people coming on this topic.

I’ve been working on the Traps System today, which is basically traps that the player can set on the map to kill monsters. But bare with me, it is appliable to what we discussed earlier on this topic.

I have a DefenseData class, like the following (removed some variables so it’s more readable here) :

public class DefenseData : ScriptableObject
{
    [SerializeField] private string _name;
    [SerializeField, TextArea(order = 3)] private string _description;

    [SerializeField, Range(0, 10)] private int _health;

    [SerializeField] private List<DefensePosCondSO> _positionConditions = new List<DefensePosCondSO>();

    public string Name
    {
        get { return _name; }
    }

    public string Description
    {
        get { return _description; }
    }

    public int Health
    {
        get { return _health; }
    }

    public List<DefensePosCondSO> PositionConditions
    {
        get { return _positionConditions; }
    }

When the player sets traps, I want to check if he can actually set them here. For example, the Fence can only be set on Dirt.
So I created an abstract class with Position Conditions, that inherits from Scriptable Object, like the following :

public abstract class DefensePosCondSO : ScriptableObject
{
    public abstract bool CheckCondition(Vector2Int position);
}

I then created a child class, that checks for a specific type of tile where the player wants to set the trap, like so :

[CreateAssetMenu(menuName = ("Defenses/On Specific Tile"))]
public class OnSpecificTile : DefensePosCondSO
{
    [SerializeField] private TileData checkedTile;

    public override bool CheckCondition(Vector2Int position)
    {
        if (!CollisionManager.Tiles.ContainsKey(position))
            return false;

        if (CollisionManager.Tiles[position] == checkedTile)
            return true;

        return false;
    }
}

And finally, on the DefenseBuilder script, when the player is setting the trap, we iterate through all conditions to check wether or not he can set the trap here :

    private bool CheckConditions(DefenseData data, Vector2Int position)
    {
        foreach(var condition in data.PositionConditions)
        {
            if (!condition.CheckCondition(position))
                return false;
        }

        return true;
    }

And voilà !

I think this logic is really close from what I have been told earlier on this topic, it works like a charm and seems quite scalable for a small team, so I wanted to share it !

1 Like