Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

abstract classes with MonoBehavior

Discussion in 'Scripting' started by James-Sullivan, Aug 4, 2015.

  1. James-Sullivan

    James-Sullivan

    Joined:
    Jun 15, 2015
    Posts:
    128
    I have this abstract class that inherits from MonoBehavior. In its Start method, it calls on of the classes abstract methods, SetProbability(). For some reason, this causes Unity to crash.

    I don't know much about abstract classes, and even less about how MonoBehavior's methods tie into this. Can someone please take a look at this to see what I'm doing wrong? Can an abstract class's Start() call a abstract method so that each derived class has that method called for it on Start?
    Code (CSharp):
    1. using UnityEngine;
    2. using System.Collections;
    3.  
    4. public abstract class CardInformation : MonoBehaviour {
    5.     protected float Width {  // width in world units
    6.         get{
    7.             return Width;
    8.         }
    9.  
    10.         private set{
    11.             Width = value;
    12.         }
    13.     }
    14.  
    15.     protected float Height {  // height in world units
    16.         get{
    17.             return Height;
    18.         }
    19.        
    20.         private set{
    21.             Height = value;
    22.         }
    23.     }
    24.  
    25.     [Range(1,50)] protected int Probability; // 50 is 100% of the deck
    26.    
    27.     // Use this for initialization
    28.     void Start () {
    29.         Width = (606f/1024f) * transform.localScale.x;
    30.         Height = (1024f/1024f) * transform.localScale.y;
    31.         SetProbability();
    32.     }
    33.  
    34.     public bool Inside(Vector3 pos){ // Returns true if the pos is inside this card
    35.         bool retBool = false;
    36.         if (pos.x <= transform.position.x - Width/2 && pos.x >= transform.position.x - Width/2
    37.             && pos.y <= transform.position.y - Height/2 && pos.y >= transform.position.y - Height/2) {
    38.             retBool = true;
    39.         }
    40.         return retBool;
    41.     }
    42.  
    43.     public int GetProbability(){
    44.         return Probability;
    45.     }
    46.  
    47.     abstract public void Action(GameObject targetTile,  GameObject[] tiles); // This function will be overriden by each card for its specific action
    48.     abstract public void SetProbability();
    49. }
    50.  
    Derived class
    Code (CSharp):
    1. using UnityEngine;
    2. using System.Collections;
    3.  
    4. public class PlusOne : CardInformation { // Inherits from the CardInformation, must over ride Action  
    5.  
    6.     override public void Action(GameObject targetTile,  GameObject[] tiles){
    7.         targetTile.GetComponent<TileInformation>().Strength += 1;
    8.     }
    9.  
    10.     override public void SetProbability(){
    11.         Probability = 50;
    12.     }
    13. }
    14.  
     
  2. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,513
    Check out your implementations for Width and Height... your setters set the value to Width and Height respectively.

    This will keep calling the setter recursively, because when setting itself you're telling it to set itself, which tells it to set itself, which tells it to set itself, which... and on and on.

    Creating a stackoverflow.

    Unity in this case on your system crashes.
     
  3. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,513
    I notice other quarks about your code that just seem messy. So I rewrote it a bit to and added comments to the changes.

    Code (csharp):
    1.  
    2. using UnityEngine;
    3. using System.Collections;
    4.  
    5. public abstract class CardInformation : MonoBehaviour
    6. {
    7.  
    8.     //lets organize with some fields
    9.  
    10.     #region Fields
    11.  
    12.     //unless you're using default properties, implement your fields
    13.     [System.NonSerialized()] //flag them as nonserialized, I've noticed a bug where unity serializes some private fields when not flagged as such
    14.     private float _width;
    15.     [System.NonSerialized()]
    16.     private float _height;
    17.  
    18.     [SerializeField()] //this field is private, but needs to be serialized
    19.     [Range(1, 50)]
    20.     private int _probability;
    21.  
    22.     #endregion
    23.  
    24.     #region CONSTRUCTOR
    25.  
    26.     // Use this for initialization
    27.     //make it protected virtual incase the child class needs to access Start as well
    28.     protected virtual void Start()
    29.     {
    30.         Width = (606f / 1024f) * transform.localScale.x;
    31.         Height = (1024f / 1024f) * transform.localScale.y;
    32.         ResetProbablity();
    33.     }
    34.  
    35.     #endregion
    36.  
    37.     #region Properties
    38.  
    39.     //I couldn't see why you made the getters of this protected. So I made them public, but the setters protected.
    40.     //I especially didn't know why you did it with Probability since you later have a 'GetProbability()' method mirroring the Java style of getters.
    41.  
    42.     public float Width
    43.     {
    44.         get { return _width; }
    45.         protected set { _width = value; }
    46.     }
    47.  
    48.     public float Height
    49.     {
    50.         get { return _height; }
    51.         protected set { _height = value; }
    52.     }
    53.  
    54.     //a public getter for Probability
    55.     public int Probability
    56.     {
    57.         get { return _probability; }
    58.         protected set
    59.         {
    60.             //use the setter to also clamp, the Range attribute only works in the inspector
    61.             _probability = Mathf.Clamp(value, 1, 50);
    62.         }
    63.     }
    64.  
    65.     #endregion
    66.  
    67.     #region Methods
    68.  
    69.     public bool Inside(Vector3 pos)
    70.     { // Returns true if the pos is inside this card
    71.         bool retBool = false;
    72.         if (pos.x <= transform.position.x - Width / 2 && pos.x >= transform.position.x - Width / 2
    73.             && pos.y <= transform.position.y - Height / 2 && pos.y >= transform.position.y - Height / 2)
    74.         {
    75.             retBool = true;
    76.         }
    77.         return retBool;
    78.     }
    79.  
    80.     #endregion
    81.  
    82.     #region Abstract Interface
    83.  
    84.     abstract public void Action(GameObject targetTile, GameObject[] tiles); // This function will be overriden by each card for its specific action
    85.  
    86.     //I renamed this to better represent what the function is doing
    87.     abstract public void ResetProbablity();
    88.  
    89.     #endregion
    90.  
    91. }
    92.  
    1) Note I organized code into regions.

    2) I declared fields for all your properties (this is why you had your stackoverflow...). Not you could ALSO have not declared fields and just declared default properties, which the compiler injects fields in its place for you. You'd do this like so:

    Code (csharp):
    1.  
    2.     #region Fields
    3.  
    4.     [SerializeField()] //this field is private, but needs to be serialized
    5.     [Range(1, 50)]
    6.     private int _probability;
    7.  
    8.     #endregion
    9.    
    10.     #region Properties
    11.  
    12.     public float Width
    13.     {
    14.         get;
    15.         protected set;
    16.     }
    17.  
    18.     public float Height
    19.     {
    20.         get;
    21.         protected set;
    22.     }
    23.    
    24.     #endregion
    25.  
    Personally I avoid the default properties so that I can look in my 'fields' region and know exactly what members actually exist when reading my code. They both compile into the same thing.

    3) I made your Start method protected virtual. This way if PlusOne or any other child class needs to use 'Start' it can. If you don't override, and leave it private like you did, then if you right a private Start in a child class... technically there are 2 distinct 'Start' methods. Unity will in turn use the first 'Start' it finds and only call that.

    4) In the probability property, I actively clamp the value. This is because Range doesn't actually clamp when setting at runtime. It only clamps the range in the inspector. Now if this class is attempted to be set to 51 by a child class... it will clamp it.

    5) I renamed SetProbability to ResetProbability. This is because it's not a setter, the name 'SetProbability' implies that we get to set the value. Instead what we're doing is telling the class to reset it to its default value.
     
    CastleIsGreat and BenZed like this.
  4. James-Sullivan

    James-Sullivan

    Joined:
    Jun 15, 2015
    Posts:
    128
    @ lordofduct Wow, thanks for the notes on how to improve my code. I learned a lot from reading it.

    What is Serialization and why not serialize width and height, but sterilize probability?
    2. How does declaring fields for my properties fix the stack overflow problem?
     
  5. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,513
    serialization is how unity stores values for loading when you load a prefab or scene.

    Basically, the values you see in the inspector are serialized. Unity turns them into bits/strings and stores those values on disk for future loading (if you set your project settings to text mode serialization, then right click on a prefab in windows explorer, and select to edit in notepad... you'll see the serialized version of your objects).

    If you don't want them in the inspector, don't serialize them. With the exception of one case... that being if you don't want a value seen in the inspector, but you still want it serialized. In that case use HideInInspector (this being a rare case).

    I assumed you didn't want Width or Height serialized since you had them defined as properties and only properties. And properties don't get serialized. Since they weren't showing up in your inspector before, I maintained that fact.
     
  6. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,513
    As for how it fixed the stack overflow.

    In your previous version you said:

    Code (csharp):
    1.  
    2.     protected float Width {  // width in world units
    3.         get{
    4.             return Width;
    5.         }
    6.  
    7.         private set{
    8.             Width = value;
    9.         }
    10.     }
    11.  
    Then later you say:

    Code (csharp):
    1.  
    2. Width=(606f/1024f)* transform.localScale.x;
    3.  
    Here's the thing... when you say 'Width =', what you're actually doing is calling the function declared in the 'set' part of that property named Width. That being:

    Code (csharp):
    1.  
    2.         private set{
    3.             Width = value;
    4.         }
    5.  
    But what does the set function do?

    It says to set 'Width = value'.

    But when we say 'Width = ...' we're calling the set function of the property Width.

    So we just called:

    Code (csharp):
    1.  
    2.         private set{
    3.             Width = value;
    4.         }
    5.  
    We called the function that we're already in.

    Which in turn calls itself.

    Which in turn calls itself.

    Which in turn...

    ...

    ...

    on for infinity.



    What does this have to do with the stack you might ask.

    Well a function takes up memory on the stack, and that memory isn't returned until the function completes. If that function calls another function, that follow on function will take up space until its done, and the function that called it can't complete until that function finishes. This is why it's called a stack, each function gets pushed onto the stack, and popped off when it completes. A function can't pop until any functions it calls pop.

    It's a Last In First Out, or LIFO collection. Otherwise known as a 'stack'. Hence why we call this the stack memory.

    Think of it like a stack of plates. As you place plates on, to get to the plates towards the bottom, you have to remove the plates on top.


    Anyways, you're pushing infinite number of plates onto the stack, never removing them. And Unity will only allow you to have a stack a certain height. When it reaches that height you "overflow" the stack. As a result the program runs out of memory to run the program in... it could ask the operating system for more, but why? If you overflow the stack, it's likely you're in an infinite loop (in this case you are), so allocating new memory for the stack would be futile. (nevermind that moving the stack is a difficult task to do anyways).
     
    passerbycmc, CastleIsGreat and BenZed like this.
  7. James-Sullivan

    James-Sullivan

    Joined:
    Jun 15, 2015
    Posts:
    128
    @lordofduct I see. So if Width = runs the properties' set code, is there a way to set it without using a field?
     
  8. DonLoquacious

    DonLoquacious

    Joined:
    Feb 24, 2013
    Posts:
    1,667
    Properties are not data containers like fields are- they don't "hold the value" of anything, they just hold implementation (a getter and setter). Therefore, all properties either having backing fields or run internal calculations to output a result from other data (like a method). The one semi-exception to this is called "Auto-Properties", which is where you specify the getters/setters it has without actually implementing them yourself.

    Important note: auto-properties still have backing fields, they're just created by the compiler instead. In this case it would be like:
    Code (csharp):
    1. protected float Width{ get; private set; }
    and you can consider this shorthand for:
    Code (csharp):
    1. private float _width;
    2.  
    3. protected float Width
    4. {
    5.    get{ return this._width; }
    6.    private set{ this._width = value; }
    7. }
    While auto-properties are generally (in C# programming) considered fantastic stand-ins because they're quick and easy, and you can change the implementation later without changing any of the references to it, in Unity they suffer the problem of not being exposed to the inspector unless you write a custom editor script for it. You CAN use the [SerializeField] attribute for the backing field, if you make an explicit backing field and don't use an auto-property, to expose that to the inspector instead though.
     
    Last edited: Aug 5, 2015
    lmraddmb likes this.
  9. James-Sullivan

    James-Sullivan

    Joined:
    Jun 15, 2015
    Posts:
    128
    Cool, thanks.
     
  10. CastleIsGreat

    CastleIsGreat

    Joined:
    Nov 10, 2014
    Posts:
    176
    See Posts like this are why I should spend more time just reading random posts in this section. Do I get annoyed when I see overly simplified questions that people could learn by watching a tutorial video provided by unity? a little... Are reading those just so that I could find ones like this worth it? hell yes...

    I read this post and could literally feel my brain learning things. Keep up the good work lordofduct!
     
    lmraddmb, passerbycmc and lordofduct like this.