abstract classes with MonoBehavior

Unity Engine
1

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?

using UnityEngine;
using System.Collections;

public abstract class CardInformation : MonoBehaviour {
    protected float Width {  // width in world units
        get{
            return Width;
        }

        private set{
            Width = value;
        }
    }

    protected float Height {  // height in world units
        get{
            return Height;
        }
       
        private set{
            Height = value;
        }
    }

    [Range(1,50)] protected int Probability; // 50 is 100% of the deck
    
    // Use this for initialization
    void Start () {
        Width = (606f/1024f) * transform.localScale.x;
        Height = (1024f/1024f) * transform.localScale.y;
        SetProbability();
    }

    public bool Inside(Vector3 pos){ // Returns true if the pos is inside this card
        bool retBool = false;
        if (pos.x <= transform.position.x - Width/2 && pos.x >= transform.position.x - Width/2
            && pos.y <= transform.position.y - Height/2 && pos.y >= transform.position.y - Height/2) {
            retBool = true;
        }
        return retBool;
    }

    public int GetProbability(){
        return Probability;
    }

    abstract public void Action(GameObject targetTile,  GameObject[] tiles); // This function will be overriden by each card for its specific action
    abstract public void SetProbability();
}

Derived class

using UnityEngine;
using System.Collections;

public class PlusOne : CardInformation { // Inherits from the CardInformation, must over ride Action   

    override public void Action(GameObject targetTile,  GameObject[] tiles){
        targetTile.GetComponent<TileInformation>().Strength += 1;
    }

    override public void SetProbability(){
        Probability = 50;
    }
}
2

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

I notice other quarks about your code that just seem messy. So I rewrote it a bit to and added comments to the changes.

using UnityEngine;
using System.Collections;

public abstract class CardInformation : MonoBehaviour
{

    //lets organize with some fields

    #region Fields

    //unless you're using default properties, implement your fields
    [System.NonSerialized()] //flag them as nonserialized, I've noticed a bug where unity serializes some private fields when not flagged as such
    private float _width;
    [System.NonSerialized()]
    private float _height;

    [SerializeField()] //this field is private, but needs to be serialized
    [Range(1, 50)]
    private int _probability;

    #endregion

    #region CONSTRUCTOR

    // Use this for initialization
    //make it protected virtual incase the child class needs to access Start as well
    protected virtual void Start()
    {
        Width = (606f / 1024f) * transform.localScale.x;
        Height = (1024f / 1024f) * transform.localScale.y;
        ResetProbablity();
    }

    #endregion

    #region Properties

    //I couldn't see why you made the getters of this protected. So I made them public, but the setters protected.
    //I especially didn't know why you did it with Probability since you later have a 'GetProbability()' method mirroring the Java style of getters.

    public float Width
    {
        get { return _width; }
        protected set { _width = value; }
    }

    public float Height
    {
        get { return _height; }
        protected set { _height = value; }
    }

    //a public getter for Probability
    public int Probability
    {
        get { return _probability; }
        protected set
        {
            //use the setter to also clamp, the Range attribute only works in the inspector
            _probability = Mathf.Clamp(value, 1, 50);
        }
    }

    #endregion

    #region Methods

    public bool Inside(Vector3 pos)
    { // Returns true if the pos is inside this card
        bool retBool = false;
        if (pos.x <= transform.position.x - Width / 2 && pos.x >= transform.position.x - Width / 2
            && pos.y <= transform.position.y - Height / 2 && pos.y >= transform.position.y - Height / 2)
        {
            retBool = true;
        }
        return retBool;
    }

    #endregion

    #region Abstract Interface

    abstract public void Action(GameObject targetTile, GameObject[] tiles); // This function will be overriden by each card for its specific action

    //I renamed this to better represent what the function is doing
    abstract public void ResetProbablity();

    #endregion

}

  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:

    #region Fields

    [SerializeField()] //this field is private, but needs to be serialized
    [Range(1, 50)]
    private int _probability;

    #endregion
   
    #region Properties

    public float Width
    {
        get;
        protected set;
    }

    public float Height
    {
        get;
        protected set;
    }
   
    #endregion

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.

  1. 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.

  2. 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.

  3. 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.

2 Likes
4

@ 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

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

As for how it fixed the stack overflow.

In your previous version you said:

    protected float Width {  // width in world units
        get{
            return Width;
        }

        private set{
            Width = value;
        }
    }

Then later you say:

Width=(606f/1024f)* transform.localScale.x;

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:

        private set{
            Width = value;
        }

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:

        private set{
            Width = value;
        }

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).

3 Likes
7

@lordofduct I see. So if Width = runs the properties’ set code, is there a way to set it without using a field?

8

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:

protected float Width{ get; private set; }

and you can consider this shorthand for:

private float _width;

protected float Width
{
   get{ return this._width; }
   private set{ this._width = value; }
}

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.

1 Like
9

Cool, thanks.

10

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!

3 Likes