1

I am trying to create a 2D cave generation system. When I run the program I get "System.StackOverflowException" Exception, after I try to create new object from its own class.

My cave generator works like this:

I create a map that contains ID’s (integers) of the different types of cells(like wall, water or empty Space).

First off all my "Map" class creates a map filled with walls and after that in the centre of the map, it creates a "Miner" object. The Miner digs the map and makes caves. The problem is I want to create more miners. So, my Miner that is digging the map creates another Miner. However, when I do this, I get a "System.StackOverflowException" Exception.

How do I go about tracking down the cause of the StackOverflow in my program. Here is my miner code:

Miner.cs

public class Miner
{
    Random rand = new Random();

    public string state { get; set; }
    public int x { get; set; }
    public int y { get; set; }
    public Map map { get; set; }
    public int minersCount;

    public Miner(Map map, string state, int x, int y)
    {
        this.map = map;
        this.state = state;
        this.x = x;
        this.y = y;
        minersCount++;

        if (state == "Active")
        {
            StartDigging();
        }
    }

    bool IsOutOfBounds(int x, int y)
    {
        if (x == 0 || y == 0)
        {
            return true;
        }
        else if (x > map.mapWidth - 2 || y > map.mapHeight - 2)
        {
            return true;
        }
        return false;
    }

    bool IsLastMiner()
    {
        if (minersCount == 1)
        {
            return true;
        }
        else
        {
            return false;
        }
    }

    public void StartDigging()
    {
        if (state == "Active")
        {
            int dir = 0;
            bool needStop = false;
            int ID = -1;

            while (!needStop && !IsOutOfBounds(x, y))
            {
                while (dir == 0)
                {
                    dir = ChooseDirection();
                }

                if (!AroundIsNothing())
                {
                    while (ID == -1)
                    {
                        ID = GetIDFromDirection(dir);
                    }
                }
                else
                {
                    if (!IsLastMiner())
                    {
                        needStop = true;
                    }
                }

                if (ID == 1)
                {
                    DigToDirection(dir);
                    dir = 0;
                }

                if (ID == 0 && IsLastMiner())
                {
                    MoveToDirection(dir);
                    dir = 0;
                }

                TryToCreateNewMiner();
            }

            if (needStop)
            {
                state = "Deactive";
            }
        }
    }

    public void TryToCreateNewMiner()
    {
        if (RandomPercent(8))
        {
            Miner newMiner = new Miner(map, "Active", x, y);
        }
        else
        {
            return;
        }
    }

    bool AroundIsNothing()
    {
        if (map.map[x + 1, y] == 0 && map.map[x, y + 1] == 0 &&
            map.map[x - 1, y] == 0 && map.map[x, y - 1] == 0)
        {
            return true;
        }
        else
        {
            return false;
        }
    }

    void MoveToDirection(int dir)
    {
        if (dir == 1)
        {
            x = x + 1;
        }
        else if (dir == 2)
        {
            y = y + 1;
        }
        else if (dir == 3)
        {
            x = x - 1;
        }
        else if (dir == 4)
        {
            y = y - 1;
        }
    }

    void DigToDirection(int dir)
    {
        if (dir == 1)
        {
            map.map[x + 1, y] = 0;
            x = x + 1;
        }
        else if (dir == 2)
        {
            map.map[x, y + 1] = 0;
            y = y + 1;
        }
        else if (dir == 3)
        {
            map.map[x - 1, y] = 0;
            x = x - 1;
        }
        else if (dir == 4)
        {
            map.map[x, y - 1] = 0;
            y = y - 1;
        }
    }

    int GetIDFromDirection(int dir)
    {
        if (dir == 1)
        {
            return map.map[x + 1, y];
        }
        else if (dir == 2)
        {
            return map.map[x, y + 1];
        }
        else if (dir == 3)
        {
            return map.map[x - 1, y];
        }
        else if (dir == 4)
        {
            return map.map[x, y - 1];
        }
        else
        {
            return -1;
        }
    }

    int ChooseDirection()
    {
        return rand.Next(1, 5);
    }

    bool RandomPercent(int percent)
    {
        if (percent >= rand.Next(1, 101))
        {
            return true;
        }
        return false;
    }
}
7
  • 4
    remove all the code that is not relevant to reproduce the problem. Commented Nov 6, 2014 at 17:06
  • Where you see not relevant code?? Commented Nov 6, 2014 at 17:12
  • console IO? Is it necessary? Commented Nov 6, 2014 at 18:58
  • Now I removed not relevant code. Commented Nov 6, 2014 at 19:48
  • 1
    StackOverflow exception on StackOverflow... Just pointing it out (I can't believe it's taken this long to notice one!) Do I win anything? Commented Nov 19, 2014 at 0:18

1 Answer 1

5

Whilst you can get StackOverflowExceptions by creating too many really large objects on the stack, it usually happens because your code has got into a state where it is calling the same chain of functions over and over again. So, to track down the cause in your code, the best starting point is to determine where your code calls itself.

Your code consists of several functions that are called by the Miner class itself, most of which are trivial

Trivial functions that don't call anything else in the class. Whilst these functions may contribute to the state that triggers the problem, they aren't part of the terminal function loop:

IsOutOfBounds(int x, int y)
bool IsLastMiner()
bool AroundIsNothing()
void MoveToDirection(int dir)
void DigToDirection(int dir)
int GetIDFromDirection(int dir)
int ChooseDirection()
bool RandomPercent(int percent)

This leaves your remaining three functions

public Miner(Map map, string state, int x, int y) // Called by TryToCreateNewMiner
public void StartDigging()                        // Called by constructor
                                                  // Contains main digging loop
public void TryToCreateNewMiner()                 // Called by StartDigging

These three functions form a calling loop, so if the branching logic in the functions is incorrect it could cause a non-terminating loop and hence a stack overflow.

So, looking at the branching logic in the functions

Miner

The constructor only has one branch, based on if the state is "Active". It is always active, since that's the way the object is always being created, so the constructor will always call StartDigging. This feels like the state isn't being handled correctly, although it's possible that you're going to use it for something else in the future...

As an aside, it's generally considered to be bad practice to do a lot of processing, not required to create the object in an objects constructor. All of your processing happens in the constructor which feels wrong.

TryToCreateNewMiner

This has one branch, 8% of the time, it will create a new miner and call the constructor. So for every 10 times TryToCreateNewMiner is called, we stand a good chance that it will have succeeded at least once. The new miner is initially started in the same position as the parent object (x and y aren't changed).

StartDigging

There's a fair bit of branching in this method. The main bit we are interested in are the conditions around calls to TryToCreateNewMiner. Lets look at the branches:

if(state=="Active")

This is currently a redundant check (it's always active).

while (!needStop && !IsOutOfBounds(x, y)) {

The first part of this termination clause is never triggered. needStop is only ever set to true if(!IsLastMiner). Since minersCount is always 1, it's always the last miner, so needStop is never triggered. The way you are using minersCount suggests that you think it is shared between instances of Miner, which it isn't. If that is your intention you may want to read up on static variables.

The second part of the termination clause is the only way out of the loop and that is triggered if either x or y reaches the edge of the map.

while(dir==0)

This is a pointless check, dir can only be a number between 1 and 5, since that's what is returned by ChooseDirection.

if(!AroundIsNothing())

This is checking if the positions that the Miner can move into are all set to 0. If they are not, then GetIDFromDirection is called. This is key. If the Miner is currently surrounded by 0, ID will not be set, it will remain at it's previous value. In a situation where a Miner has just been created, this will be -1 (we know this could happen because all Miners are created at the location of the Miner creating it).

The last two checksif(ID==1) and if(ID==0 && IsLastMiner()) guard the code that moves the Miner (either by calling dig, or move). So, if ID is not 0, or 1 at this point the Miner will not move. This could cause a problem because it is immediately before the call to TryToCreateNewMiner, so if the program ever gets into this situation it will be stuck in a loop where the Miner isn't moving and it's constantly trying to create new Miners in the same position. 8% of the time this will work, creating a new miner in the same position, which will perform the same checks and get into the same loop, again not moving and trying to create a new miner and so it goes until the stack runs out of space and the program crashes.

You need to take a look at your termination clauses and the way you're handling ID, you probably don't want the Miner to just stop doing anything if it gets completely surround by 0.

Sign up to request clarification or add additional context in comments.

1 Comment

Oh thanks for soo good answer, now I understand what I doing wrong :)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.