Search Unity

Skill systems and other data-rich structures: inheritance or database?

Discussion in 'Scripting' started by Sendatsu_Yoshimitsu, Aug 28, 2014.

  1. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    I've gotten to the point in my development cycle where I need to start looking at how skills will be tracked and organized. Right now, the simplest solution I can think of is to make a core SkillBasic.cs class that contains universal features like names and MP cost, build an empty Use() function into it, then make a new class that extends SkillBase for each new skill, overriding Use() to account for different skills having drastically different utilities. If I create a single delegate or reference for each keybind, and make that keybind's Input.GetButton simply find the referenced class and execute mySkill.Use(), I could swap things in and out and generally have an easy way of things.

    The problem I have with this is that it feels perilously close to hard-coding: many good ARPGs have upwards of 50 skills, sometimes more, and curating 50 subclasses or more as the game evolves sounds like a nightmare.

    Given that, is there an obvious, better structure I could be using here? I've briefly flirted with the idea of making a database that keeps some array SkillBasic[] which contains a variation of SkillBasic for each unique skill in-game, but that would require me to create a custom inspector so I didn't have to generate all of my skills at runtime, and it doesn't let me use overrides to change the skill's action the way subclasses would.
     
  2. GarthSmith

    GarthSmith

    Joined:
    Apr 26, 2012
    Posts:
    1,240
    Data, I load from files.

    Commands, or how to execute steps, is what I would use inheritance for.

    Something like an inventory system will definitely use both. Eg: I load from a file the costs of each item, which icon id to use, how "strong" the item is, etc. But for a health potion, the actual adding health to the player is done with inheritance.
     
  3. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    That makes a lot of sense... how do you handle managing large libraries of commands though? One subclass per interaction type (healing, attacks, buffs) simplifies things somewhat, but at the end of the day you're still making a whole bunch of classes to handle minute variations in function.
     
  4. GarthSmith

    GarthSmith

    Joined:
    Apr 26, 2012
    Posts:
    1,240
    Yup! Have a heal spell, physical attack, buffs... You can probably have one class for "spells" then extend that further for "heal spells" and "damage spells" which is probably good enough at that point. You might need to extend those further if you have like "Heal over Time" as well as "Instant Heals".

    Another thing you can do is use the command pattern. So instead of having 100 classes, you really just need 100 CastSpell delegates you can assign to a specific Spell object.

    I would do a few spells (at least 3) first, as separate classes if you have to, then start trying to figure out what's common among them and start moving code to base classes.
     
  5. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    Hmm, that makes complete and utter sense, but are delegates serializeable? The main reason I've been hesitating to use them is because I was under the impression that they were not, meaning I would have to do a bit of an end-around to save the player's skill bar arrangement between playthroughs.
     
  6. TonyLi

    TonyLi

    Joined:
    Apr 10, 2012
    Posts:
    12,694
    You might glean some ideas from plyGame's Skills system. In plyGame, a skill is just one class (derived from ScriptableObject) that has data fields that define how the skill works, such as:

    • Name
    • MP cost
    • How it's activated (when the player hits a button or performs an action, automatic in response to something, etc.)
    • How it's delivered (instant/melee, or spawn a projectile)
    • Targeting (self, player's current target, auto-select)
    • Valid classes of targets
    • Projectile to spawn
    • Damage to inflict
    • etc.
    Following this model, a skill would simply be a data file (asset) in your project. The skill class may be a little bigger than a single-skill class in order to handle everything generically, but overall it's much simpler to maintain because it's just one class, no delegates, no subclasses.

    Characters would have a list of skills (e.g., List<Skill> skills). When the character learns a new skill, assign the asset to this list.
     
    Last edited: Nov 20, 2015
  7. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    Ooh, that is a great resource... how do they actually check the flags they're setting, does it execute some kind of long conditional list every time you activate it?
     
  8. TonyLi

    TonyLi

    Joined:
    Apr 10, 2012
    Posts:
    12,694
    I think so. You could define a lot of these options as enums:
    Code (csharp):
    1. enum Targeting { Self, CurrentTarget, AutoSelect };
    and handle them in a switch:
    Code (csharp):
    1. switch (targeting) {
    2.     case Targeting.Self: HandleTargetSelf();
    3.     case Targeting.CurrentTarget: HandleTargetCurrentTarget();
    4.     case Targeting.AudoSelect: HandleAutoTarget();
    5. }
    because eventually the data has to hit the code at some point. Just try to keep it as decoupled and modular as possible to avoid interdepencies between these switch statements.
     
  9. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    That is quite awesome, and solves the problem I've been boldly smashing my face against for the last week, thank you! :)
     
  10. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    One last question occurs as I'm writing my skill class, what is the best way to organize and manage things in the project? Right now I'm just creating an empty gameobject, calling it Skill Database, and attaching each instance of Skill to it, but that seems like a fugly and slightly silly way to do it.
     
  11. TonyLi

    TonyLi

    Joined:
    Apr 10, 2012
    Posts:
    12,694
    It depends on your project, but I'd probably define a SkillDatabase class that inherits from ScriptableObject. This way you can make a freestanding asset that doesn't have to be attached to a GameObject in the scene. Because it inherits from ScriptableObject, it's serializable and you can edit it in the Inspector view. (Jacob Pennock wrote a good ScriptableObject tutorial.)

    Ultimately you'll need some reference to a SkillDatabase asset at runtime. You could attach a MonoBehaviour with a SkillDatabase variable to a singleton GameObject, or reference it in a (static?) class that isn't a MonoBehaviour, or just add it as a field in a MonoBehaviour for your character.

    To roughly sketch out what I'm talking about:

    Code (csharp):
    1. enum Activation { Button, Action, Automatic };
    2. enum Targeting { Self, CurrentTarget, AutoSelect }; // et cetera for other settings.
    3.  
    4.  
    5. //=== Individual Skills: =============================
    6. public class Skill : ScriptableObject {
    7.     public string skillName = string.Empty; // I always initialize variables.
    8.     public float mpCost = 0;
    9.     public Activation activation = Activation.Button;
    10.     public string activationButton = "Fire1";
    11.     public Targeting targeting = Targeting.CurrentTarget;
    12.     // etc.
    13.     public bool isActive = false;
    14.  
    15.     public void Update(MonoBehaviour parentMonoBehaviour) {
    16.         if (!isActive && CanActivate()) {
    17.             parentMonoBehaviour.StartCoroutine(Use());
    18.         }
    19.     }
    20.  
    21.     public bool CanActivate() {
    22.         switch (activation) {
    23.             case Activation.Button: return Input.GetButtonDown(activationButton);
    24.             // etc.
    25.         }
    26.     }
    27.  
    28.     public IEnumerator Use() { // Coroutine so it can do stuff over time.
    29.         isActive = true;
    30.         // Do stuff based on the settings above.
    31.         isActive = false;
    32.     }
    33. }
    34.  
    35.  
    36. //=== Skil Database: =============================
    37. public class SkillDatabase : ScriptableObject {
    38.     public List<Skill> skills = new List<Skill>();
    39.  
    40.     public void Awake() {
    41.         // At runtime, instantiate skills so you don't modify design-time originals.
    42.         // Assumes this SkillDatabase is itself already an instantiated copy.
    43.         for (int i = 0; i < skills.Count; i++) {
    44.             skills[i] = Instantiate(skills[i]);
    45.         }
    46.     }
    47.  
    48.     public void Update(MonoBehaviour parentMonoBehaviour) {
    49.         skills.ForEach(skill => skill.Update(parentMonoBehaviour));
    50.     }
    51. }
    52.  
    53.  
    54. //=== Character MonoBehaviour: =============================
    55. public class Character : MonoBehaviour {
    56.     public SkillDatabase skillDatabase = null;
    57.  
    58.     void Awake() {
    59.         // At runtime, instantiate a copy so you don't modify the design-time original:
    60.         skillDatabase = Instantiate(skillDatabase);
    61.         if (skillDatabase != null) skillDatabase.Awake();
    62.     }
    63.  
    64.     void Update() {
    65.         if (skillDatabase != null) skillDatabase.Update(this);
    66.     }
    67. }
    Again, this is only a rough sketch, with a couple tricky points (such as instantiating, and coroutines) thrown in. But I just typed it into the reply box. There might be typos.
     
    Marrt likes this.
  12. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    Holy cow this is exceptional, thank you so much for your help!! I'm going to have a lot of fun dissecting this to work out how to best do things... just to clarify, when you say attach it as a field in a monobehavior, you're saying that I'm better off making a SkillDatabase variable in a script on my character and setting it to the game's database, or can I get away with just dragging the script itself onto the player's object?
     
  13. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    Oh wait, I see what you did with the instantiation, ignore me- I was being dense, I forgot that you can't drag & drop things that don't inherit from Monobehavior ^^
     
  14. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    Hmm, most of this is self-explanatory, but would you mind clarifying two quick things?

    1) Where would the actual creation of new skills occur in the workflow? Would I build some kind of constructor into my Skill class, then create a line in SkillDatabase's Awake() for each unique skill I want, and pass different values to the constructor there?

    2) What is the purpose of passing the MonoBehaviour argument in Update and Awake? It looks like they're supposed to point at the gameobject the database is instantiated on (to wit, the player), but I don't see where they're actually given their reference, or why you wouldn't do something like
    Code (CSharp):
    1.  
    2. void Update(){
    3. parentMonoBehaviour = gameObject;
    4. }
    5.  
     
    Last edited: Aug 29, 2014
  15. TonyLi

    TonyLi

    Joined:
    Apr 10, 2012
    Posts:
    12,694
    That's a good question because I omitted that part. It's in Jacob Pennock's tutorial, but you'd create a menu item that calls the static method ScriptableObject.CreateInstance(). Here are shorter, more up-to-date instructions: Grab the ScriptableObjectUtility.cs from the wiki. Then create a script in a folder named "Editor". For example, you could call it SkillMenu.cs:

    Code (csharp):
    1. using UnityEngine;
    2. using UnityEditor;
    3.  
    4. public class SkillMenu {
    5.  
    6.     [MenuItem("Assets/Create/Skill")]
    7.     public static void CreateAsset () {
    8.         ScriptableObjectUtility.CreateAsset<Skill>();
    9.     }
    10.  
    11.     [MenuItem("Assets/Create/Skill Database")]
    12.     public static void CreateAsset () {
    13.         ScriptableObjectUtility.CreateAsset<SkillDatabase>();
    14.     }
    15. }
    This will add a couple menu items to the Unity editor. When you select them, they'll create Skill and SkillDatabase asset files.

    Skill and SkillDatabase inherit from ScriptableObject, not MonoBehaviour. The StartCoroutine() method is only available for MonoBehaviours. So I passed along the MonoBehaviour to give skills access to the StartCoroutine() method. This was just an embellishment. If the skill's Use() method can execute and finish right away (for example, to simply spawn a prefab such as a fireball), you can get rid of the coroutine stuff.
     
  16. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    So thank you very much for your feedback! In several concise and well-informed posts you've cleared up about a hundred questions for me, I really appreciate it! ^^
     
  17. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    Hmm this is odd, unity doesn't like me using CreateAsset() twice, is there something I'm missing that would let me use the member for each menu item I make for ScriptableObjectUtility?
     
    Last edited: Aug 29, 2014
  18. TonyLi

    TonyLi

    Joined:
    Apr 10, 2012
    Posts:
    12,694
    Sorry, I've never actually used ScriptableObjectUtility. I've always directly called ScriptableObject.CreateInstance<>() and AssetDatabase.CreateAsset().

    I assume you're referring to ScriptableObjectUtility.CreateAsset<>(). But if it's complaining about AssetDatabase.CreateAsset() instead, make sure you're providing new assets and unique paths for each call.

    If that doesn't help, please post your code here and any errors it's reporting.
     
  19. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    I get the error just using your example, to wit:

    Code (CSharp):
    1.     [MenuItem("Assets/Create/Skill")]
    2.     public static void CreateAsset () {
    3.         ScriptableObjectUtility.CreateAsset<Skill>();
    4.     }
    5.    
    6.     [MenuItem("Assets/Create/Skill Database")]
    7.     public static void CreateAsset () {
    8.         ScriptableObjectUtility.CreateAsset<SkillDatabase>();
    9.     }
    What's giving it fits is having two public static void CreateAsset() functions- the error goes away and the menu creation option remains if I just rename one of them, but I'm not sure if I'm supposed to do that.
     
  20. TonyLi

    TonyLi

    Joined:
    Apr 10, 2012
    Posts:
    12,694
    Sorry, pitfalls of copy-and-paste. Rename one CreateSkillAsset() and the other CreateSkillDatabaseAsset():
    Code (csharp):
    1. [MenuItem("Assets/Create/Skill")]
    2. public static void CreateSkillAsset () {
    3.     ScriptableObjectUtility.CreateAsset<Skill>();
    4. }
    5.    
    6. [MenuItem("Assets/Create/Skill Database")]
    7. public static void CreateSkillDatabaseAsset () {
    8.     ScriptableObjectUtility.CreateAsset<SkillDatabase>();
    9. }
     
  21. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    Oooh, perfect! I see... I wasn't sure if CreateAsset was referring to a specific method in ScriptableObjectUtility, thank you! ^_^
     
  22. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    By the way, I'm consistently seeing a script error claiming that Update() can't take parameters, but it compiles and operates fine, and a bit of internet sleuthing reveals that this is apparently a Unity bug related to ScriptableObjectUtility and can safely be ignored, is that your experience as well?
     
  23. TonyLi

    TonyLi

    Joined:
    Apr 10, 2012
    Posts:
    12,694
    You can just rename it to something different, like OnUpdate() or UpdateSkill(). This was just a rough sketch of the concept.
     
  24. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    Oh that makes sense, thank you! I promise this is the very last thing, since everything works swimmingly and I'm getting a good enough sense of what everything does to build on it, but I notice that if I drag Skills into the database asset, close unity, and re-open it, every slot in the database shows a type mismatch. Do you have any idea where in the code I should be looking to start debugging?
     
  25. TonyLi

    TonyLi

    Joined:
    Apr 10, 2012
    Posts:
    12,694
    Nesting ScriptableObjects is a bit tricky; sorry, I glossed over that. You might find it more convenient to put everything in the SkillDatabase rather than creating separate Skill assets and assigning them into SkillDatabase:
    Code (csharp):
    1. enum Activation { Button, Action, Automatic };
    2. enum Targeting { Self, CurrentTarget, AutoSelect }; // et cetera for other settings.
    3.  
    4.  
    5. //=== Individual Skills: =============================
    6. [Serializable]
    7. public class Skill {
    8.     public string skillName = string.Empty; // I always initialize variables.
    9.     public float mpCost = 0;
    10.     public Activation activation = Activation.Button;
    11.     public string activationButton = "Fire1";
    12.     public Targeting targeting = Targeting.CurrentTarget;
    13.     // etc.
    14.     public bool isActive = false;
    15.  
    16.     public void Update(MonoBehaviour parentMonoBehaviour) {
    17.         if (!isActive && CanActivate()) {
    18.             parentMonoBehaviour.StartCoroutine(Use());
    19.         }
    20.     }
    21.  
    22.     public bool CanActivate() {
    23.         switch (activation) {
    24.             case Activation.Button: return Input.GetButtonDown(activationButton);
    25.             // etc.
    26.         }
    27.     }
    28.  
    29.     public IEnumerator Use() { // Coroutine so it can do stuff over time.
    30.         isActive = true;
    31.         // Do stuff based on the settings above.
    32.         isActive = false;
    33.     }
    34. }
    35.  
    36.  
    37. //=== Skil Database: =============================
    38. public class SkillDatabase : ScriptableObject {
    39.     public List<Skill> skills = new List<Skill>();
    40.  
    41.     public void UpdateSkillDatabase(MonoBehaviour parentMonoBehaviour) {
    42.         skills.ForEach(skill => skill.Update(parentMonoBehaviour));
    43.     }
    44. }
    45.  
    46.  
    47. //=== Character MonoBehaviour: =============================
    48. public class Character : MonoBehaviour {
    49.     public SkillDatabase skillDatabase = null;
    50.  
    51.     void Awake() {
    52.         // At runtime, instantiate a copy so you don't modify the design-time original:
    53.         skillDatabase = Instantiate(skillDatabase);
    54.     }
    55.  
    56.     void Update() {
    57.         if (skillDatabase != null) skillDatabase.UpdateSkillDatabase(this);
    58.     }
    59. }
    60.  
    61.  
    62. //===========================================================
    63. // The editor script:
    64.  
    65. using UnityEngine;
    66. using UnityEditor;
    67.  
    68. public class SkillMenu {
    69.  
    70.     [MenuItem("Assets/Create/Skill Database")]
    71.     public static void CreateSkillDatabaseAsset() {
    72.         ScriptableObjectUtility.CreateAsset<SkillDatabase>();
    73.     }
    74. }
    Notes:
    • You no longer need to instantiate Skills, so there's no need for SkillDatabase.Awake(). I removed that.
    • You only need one menu item now.
    • You only have one asset file to keep track of now (SkillDatabase) rather than an asset that points to other assets.
     
    Deleted User and Tien-Tran like this.
  26. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    Holy cow, that works perfectly and is massively easier to curate, thank you!! Without your incredibly detailed help and feedback, I would still be working with dozens of subclasses living on a gameobject, thank you so much :)
     
  27. TonyLi

    TonyLi

    Joined:
    Apr 10, 2012
    Posts:
    12,694
    Glad I could help!
     
    Pharaoh_ likes this.
  28. TonyLi

    TonyLi

    Joined:
    Apr 10, 2012
    Posts:
    12,694
  29. Sendatsu_Yoshimitsu

    Sendatsu_Yoshimitsu

    Joined:
    May 19, 2014
    Posts:
    691
    This is really cool, I never considered using a component-based approach. I'm definitely going to take a look at this... off the top of my head my one concern would be error trapping, as doing effects with components instead of switch statements could lead to multiple effects fighting for control in a way that causes weird behavior but doesn't produce errors, but even that is reasonably easy to handle- thank you for remembering this thread and updating me! :)
     
  30. Dorscherl

    Dorscherl

    Joined:
    May 29, 2017
    Posts:
    26

    Know this is old but I'm curious to whether there is a neat way i can utilize this for AI and players alike. Where as you specify the button and get input directly from the Input manager, I want to specify the value directly from an input script on the gameobject. Need to handle activations that way and when conditions on the character are met (auto) etc
     
  31. TonyLi

    TonyLi

    Joined:
    Apr 10, 2012
    Posts:
    12,694