- Home /
I'm getting a StackOverFlowException
So from what I've seen a SOE is linked to things that recur, like referencing a method: MethodA from within MethodA.
I haven't seen anything like that in my script and would like some help. I also don't have any errors from building the script.
It should be noted that the error occurs when my units are close to the enemy. However since that relates to most of the script, it's not a lot to go on.
So the error:
UnityEngine.Transform.get_position () (at C:/buildslave/unity/build/artifacts/generated/common/runtime/UnityEngineTransformBindings.gen.cs: 26)
It then lists lines in my script:
HopliteCombat.CheckEnemies () (at Assets/Scripts/Gameplay/HopliteCombat.cs: 67)
Which is this line:
Collider[] enemyColliders = Physics.OverlapSphere(gameObject.transform.position, combatRadius);
And line 109:
if (CheckEnemies() == "Hoplite")
And there are a bunch of other references.
Here is the whole script, since multiple methods are referenced I don't think it would be wise to show only snippets:
using System;
using UnityEngine;
using UnityEngine.UI;
using System.Collections;
using System.Collections.Generic;
public class HopliteCombat : MonoBehaviour {
//EXT SCRIPTS
public Movement move;
private Hoplite hoplite;
//HEALTH
private HealthBar health;
public GameObject canvas;
private bool healthBarUp = false;
//STATS
public float attackSpeed = 2.0f;
public float combatRadius = 20.0f;
//UNITS
public GameObject enemy;
public NavMeshAgent leaderAgent;
void Update()
{
if (CheckDistance() < 4.0f && move.enemySelected == true)
{
canvas.SetActive(true);
if (healthBarUp)
{
canvas.transform.position += new Vector3(0, 2, 0);
healthBarUp = true;
}
Debug.Log("engaging is true");
//stop moving
leaderAgent.Stop();
Attack();
}
else {canvas.SetActive(false);}
}
private float CheckDistance()
{
float distance = Vector3.Distance(transform.position, enemy.transform.position);
return distance;
}
private string CheckEnemies()
{
//create a sphere that detects units in a 20.0f radius. Loop through and find the closest collider, return the tag of the go.
float bestDistance = 9000.0f;
Collider bestCollider = null;
Collider[] enemyColliders = Physics.OverlapSphere(gameObject.transform.position, combatRadius);
foreach (Collider opponent in enemyColliders)
{
float distance = Vector3.Distance(gameObject.transform.position, opponent.transform.position);
if (distance < bestDistance)
{
bestDistance = distance;
bestCollider = opponent;
}
}
return bestCollider.gameObject.tag;
}
private GameObject ReturnLeader()
{
Collider[] enemyColliders = Physics.OverlapSphere(gameObject.transform.position, combatRadius);
foreach (Collider opponent in enemyColliders)
{
if (opponent.tag == "Leader")
{
return opponent.gameObject;
}
}
return null;
}
private float Attack()
{
float finalAttackDamage = default(float);
//Check tag of nearest enemy, factor in counter damage(s).
if (CheckEnemies() == "Hoplite")
{
hoplite = GetComponent<Hoplite>();
finalAttackDamage = hoplite.baseDamage;
}
if (CheckEnemies() == "Archer")
{
hoplite = GetComponent<Hoplite>();
finalAttackDamage = hoplite.baseDamage + hoplite.counterArcher;
return finalAttackDamage;
}
if (CheckEnemies() == "Peltast")
{
hoplite = GetComponent<Hoplite>();
finalAttackDamage = hoplite.baseDamage + hoplite.counterPeltast;
return finalAttackDamage;
}
if (CheckEnemies() == "Slinger")
{
hoplite = GetComponent<Hoplite>();
finalAttackDamage = hoplite.baseDamage + hoplite.counterSlinger;
return finalAttackDamage;
}
if (CheckEnemies() == "Cavalry")
{
hoplite = GetComponent<Hoplite>();
finalAttackDamage = hoplite.baseDamage;
return finalAttackDamage;
}
//After two seconds apply damage.
attackSpeed -= Time.deltaTime;
if (attackSpeed <= 0)
{
GiveDamage();
}
return 0.0f;
}
public void GiveDamage()
{
health = ReturnLeader().GetComponent<HealthBar>();
health.TakeDamage(Attack());
}
}
Answer by Bunny83 · Mar 06, 2017 at 08:08 PM
Attack method calls GiveDamage in line 153 and GiveDamage calls the Attack method in line 165. So you have a recursion that's bouncing between the two methods until you run out of stack.
Apart from your logical error (your infinite recursion), your "CheckEnemies" method is a quite demanding method that uses "Physics.OverlapSphere" which allocates an array each time it's called and you iterate over that array each time the method is called. That's actually not the problem. If you can't avoid it the method is fine. However calling the method five times inside Attack is not a good idea, especially when the result is the same for each of the five calls. So you should call it once and save the returned string in a local variable inside Attack. Then you can compare that variable to your 5 different strings.
Another thing that is strange is that the "hoplite" variable is a class variable but you reassign it inside every of your 5 conditionals. If the script is on the same gameobject from the very beginning you should get the component once in Start(). If the component might be missing first and might be added later (though i don't think that's the case here) you might want to make that "hoplite" variable a local variable since it has no use outside the Attack method.
After all it seems you packed too many different aspects into your Attack method. First it is used to calculate the attack damage. Second as a side effect it updates an attack timer and calls GiveDamage. The structure is a bit confusing as it is now. If the closes enemy is any of those five cases you never get to the point where you count down your timer or where you call GiveDamage since you early-exit in each case.
The return value of Attack isn't used when you call it in Update.
Hi, thanks for the response. Quick question, how would I get the value stored within my Attack() for:
health.TakeDamage(thefloatvariable);
Without incurring a StackOverflowException. I have a number of if statements
I could move this code into each if statement but that strikes me as incredibly inefficient:
health = Return$$anonymous$$er().GetComponent<HealthBar>();
health.TakeDamage(finalAttackDamage);
Why would you move that code into each case? Just place it at the point where you currently call "GiveDamage". Of course you have to remove all your "return" statements.
Also even when you place it in each case, that's not inefficient, just unclean code. It runs just as efficient. What is in efficient is calling "CheckEnemies" 5 times.
Though i would suggest a general restructure of your Attack method. There's no point in checking for an enemy every frame if you only attack once in a certain time period. Btw: you never reset your "attackSpeed" variable (which doesn't represent any speed but a time / timer).
If you want more control over the sequence of actions you might want to use a coroutine. Your current code is not complete and has many potential problems (like "Return$$anonymous$$er" can return null but you never check that case). So it's actually difficult to derive from your code what exact behaviour you want.