Sign in to follow this  
goochie

Stack around variable being corrupted

Recommended Posts

goochie    122
I've looked at a couple other threads about this here, but nothing seemed to apply to this case. The "stack around variable 'tank1' has been corrupted" error only pops up when I call on tank1's Fire() function, if it is commented out then the program runs and terminates just fine. Program.cpp:
#include "Tank.h"
#include <iostream>

//define Tank constructors
Tank::Tank()
{
	itsX = 0;
	itsY = 0;
	itsZ = 0;
	itsAngle = 0;
	itsHealth = 100;
	itsTarget = NULL;
}

Tank::Tank(float x, float y, float z, float angle)
{
	itsX = x;
	itsY = y;
	itsZ = z;
	itsAngle = angle;
	itsHealth = 100;
	itsTarget = NULL;
}

Tank::~Tank()
{
}

//define Tank functions
void Tank::Fire()
{
	if (itsTarget != NULL)
	{
		//if tank has a target, deal damage
		int thp = itsTarget->GetHealth();
		thp -= 20;
		itsTarget->SetHealth(thp);
	}
	else
	{/**or else do nothing, for now**/}
}

int main()
{
	//make 2 tanks
	Tank tank1;
	Tank tank2;

	//target eachother
	tank1.SetCurrentTarget(tank2);
	tank2.SetCurrentTarget(tank1);

	//std::cout << "Tank1 health: " << tank1.GetHealth() << " Tank2 health: " << tank2.GetHealth() << "\n";
	tank1.Fire();
	//std::cout << "Tank1 fires on Tank2! Tank2 health: " << tank2.GetHealth() << "\n";

	return 0;
}
Tank.h:
#include <iostream>

class Tank
{
private:

	//status data
	
	float itsX;
	float itsY;
	float itsZ;
	float itsAngle;
	int itsHealth;
	Tank * itsTarget;
public:

	//constructors
	Tank();
	Tank(float x, float y, float z, float angle);

	//destructor
	~Tank();

	//action methods
	void Fire();	//tank fires, do damage to itsTarget
	
	//get-set methods for all member variables
	float GetX() const {return itsX;}
	float GetY() const {return itsY;}
	float GetZ() const {return itsZ;}
	float GetAngle() const {return itsAngle;}
	int GetHealth() const {return itsHealth;}
	Tank* GetCurrentTarget() const {return itsTarget;}
	void SetX(float x) {itsX = x;}
	void SetY(float y) {itsY = y;}
	void SetZ(float z) {itsZ = z;}
	void SetAngle(float angle) {itsAngle = angle;}
	void SetHealth(int health) {itsHealth = health;}
	void SetCurrentTarget(Tank target) {itsTarget = ⌖}

};
I'm still a bit of a noob with C++. Most of my experience is with Java/C#, so I'm sure its something stupid that I'm just missing.

Share this post


Link to post
Share on other sites
SiCrane    11839
You probably want SetCurrentTarget() to look something like:

void SetCurrentTarget(Tank * target) {itsTarget = target;}

Share this post


Link to post
Share on other sites
janta    345
SetCurrentTarget passes by copy, therefore you're storing a pointer to a temporary stack object, which soon after causes a stack corruption

-ja

(Edit:beaten. damn you SiCrane...)

Share this post


Link to post
Share on other sites
janta    345
Could a compiler have warned about storing the address of a temporary variable? It does warn when returning such an address but...

Share this post


Link to post
Share on other sites
Zahlman    1682
I don't know of any compilers that do, but it's an interesting suggestion.

Anyway. To the OP: this isn't how OO programming works. The point of this whole "encapsulation" thing isn't just making functions to modify each member variable. That just makes more work for you and doesn't really *hide* anything. The point is that outside code that uses a Tank shouldn't *care*, to the extent that that's possible, what data is in it. It also means that the Tank takes responsibility for figuring out what happens to itself, in response to the outside world.

Thus, instead of reading a Tank's hp, decreasing the value and setting it back, you tell the tank "hey, I'm doing X amount of damage to you". Then the Tank adjusts its hp accordingly (which also gives it a chance to apply any special armouring that it has, etc.)


//int GetHealth() const {return itsHealth;}
//void SetHealth(int health) {itsHealth = health;}
void TakeDamage(int damage) {
// For now...
itsHealth = std::max(0, itsHealth - damage);
if (!itsHealth) { die(); }
}



That kind of thing.

If you're coming from a Java/C# background, you're supposed to know this kind of stuff. Unfortunately, it doesn't really get taught very well.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this