Deep copy problem

[Johnny666]

Im trying to make a game. Theres a player and he shoots bullets, and i was told whats wrong is it is making a shallow copy instead of a deep copy even though i researched this i cannot completely understand it. This is my attempt however i know it is wrong in some way because whenever i call the fire() method, the program crashes. I am VERY desperate i have been trying to solve this for a very long time and would appreciate any help!

Bullet Class:

#include "Bullet.h"

Bullet::Bullet()
{
	ptr = NULL;
}

Bullet::~Bullet() {
  delete ptr;
  ptr = NULL;
}

Bullet& Bullet::operator= (Bullet const& f) {
  if ( this != &f ) {
    delete ptr;
    ptr=new char[strlen(f.ptr)+1];
    strcpy(ptr,f.ptr);
  }
  return *this;
}

Bullet::Bullet(const char *p) {
  ptr=new char[strlen(p)+1];
  strcpy(ptr,p);
}

Bullet::Bullet ( Bullet const& f) {
  ptr=new char[strlen(f.ptr)+1];
  strcpy(ptr,f.ptr);
}

Bullet::Bullet(int startX, int startY, int moveX, int moveY)
{
	startingX = startX;
	startingY = startY;
	x = 0;
	y = 0;
	dx = moveX;
	dy = moveY;
	Extra ex;
}

int Bullet::getX()
{
	return x;
}

int Bullet::getY()
{
	return y;
}

int Bullet::getDx()
{
	return dx;
}

int Bullet::getDy()
{
	return dy;
}

int Bullet::getStartingX()
{
	return startingX;
}

int Bullet::getStartingY()
{
	return startingY;
}

void Bullet::move()
{
	x += dx;
	y += dy;
}

void Bullet::show(SDL_Surface *bullet, SDL_Surface *screen, SDL_Rect camera)
{
	
	//Show the bullet
    ex.apply_surface( (startingX  - camera.x) + x, (startingY - camera.y) + y, bullet, screen );
}

fire() method:

#include "Dot.h"

using namespace std;

Dot::Dot()
{
    //Initialize the offsets
	mousex = 0;
	mousey = 0;
	angle = 0;
	imageOffsetX = 15;
	imageOffsetY = 15;
	dx = 1;
	dy = 1;
	bullets = 20;
    velocity = 3;
	box.x = 32;
    box.y = 32;
    box.w = 20;
    box.h = 20;
    TYPE_WOOD = 0;
    TYPE_ROCK = 1;
    TOTAL_TILES = 2304;

	//Bullets
	vector<Bullet> playerBullets;

    //Path to extra function
    Extra ex;
	
    //Initialize the velocity
    xVel = 0;
    yVel = 0;
}

void Dot::fire()
{

	if( bullets > 0 )
	{

		if (angle == 0){
			playerBullets.push_back( Bullet(box.x, box.y, 5, 0) );
		}else if(angle == 90){
			playerBullets.push_back( Bullet(box.x, box.y, 0, -5) );
		}else if(angle == 180){
			playerBullets.push_back( Bullet(box.x, box.y, -5, 0) );
		}else if(angle == 270){
			playerBullets.push_back( Bullet(box.x, box.y, 0, 5) );
		}
	}
}

[tkud]

This should help in your understanding of shallow and deep copying.

[Johnny666]

I have seen that before and many other tutorial but i still dont understand it. What am i doing wrong in my code?

[Schol-R-LEA]

Would you mind posting the class declaration (in the header file), as well? It would clarify a number of things about the code.

From what I am gathering, though, the Bullet class has a number of instance variables, something like

int x, y, startX, startY, dx, dy;
char* ptr;

There may be more, but these are the ones which I could infer from the c'tors.

The main constructor seems to initialize the six integer instance vars, and declares (but does not create or initialize) an Extra object, ex . It does not initialize ptr , however.

The copy c'tor is the opposite of this, in that it copies the value of ptr (as a string) but does not copy or even mention any of the other instance vars. This is the 'shallow copy' they were referring to, and it isn't even a complete copy from appearances.

A 'deep copy' would need to copy the individual instance variables, one for one, and if any of them are themselves objects (or c-strings), they would need to be deep-copied as well.

[Johnny666]

Sure thing. When i attempted to add the deep copy, i really 'blindly' went about it and no doubt forgot to do something in the process.

Also the ex variable is just a pathway to functions for use with SDL.

Bullet.cpp

#include "Bullet.h"

Bullet::Bullet()
{
	ptr = NULL;
}

Bullet::~Bullet() {
  delete[] ptr;
  ptr = NULL;
}

Bullet& Bullet::operator= (Bullet const& f) {
  if ( this != &f ) {
    delete ptr;
    ptr=new char[strlen(f.ptr)+1];
    strcpy(ptr,f.ptr);
  }
  return *this;
}

Bullet::Bullet(const char *p) {
  ptr=new char[strlen(p)+1];
  strcpy(ptr,p);
}

Bullet::Bullet ( Bullet const& f) {
  ptr=new char[strlen(f.ptr)+1];
  strcpy(ptr,f.ptr);
}

Bullet::Bullet(int startX, int startY, int moveX, int moveY)
{
	startingX = startX;
	startingY = startY;
	x = 0;
	y = 0;
	dx = moveX;
	dy = moveY;
	ptr = NULL;
	Extra ex;
}

int Bullet::getX()
{
	return x;
}

int Bullet::getY()
{
	return y;
}

int Bullet::getDx()
{
	return dx;
}

int Bullet::getDy()
{
	return dy;
}

int Bullet::getStartingX()
{
	return startingX;
}

int Bullet::getStartingY()
{
	return startingY;
}

void Bullet::move()
{
	x += dx;
	y += dy;
}

void Bullet::show(SDL_Surface *bullet, SDL_Surface *screen, SDL_Rect camera)
{
	//Show the bullet
    ex.apply_surface( (startingX  - camera.x) + x, (startingY - camera.y) + y, bullet, screen );
}

Bullet.h

class Bullet{

private:
	int x, y;
	int dx, dy;
	char *ptr;

	Extra ex;

public:
	Bullet();
	~Bullet();
	Bullet& operator= (Bullet const& f);
	Bullet ( Bullet const& f);
	Bullet(const char *p);
	
	Bullet(int,int,int,int);
	
	int startingX, startingY;
	int getX();
	int getY();
	int getDx();
	int getDy();
	int getStartingX();
	int getStartingY();

	void move();
	void show(SDL_Surface*, SDL_Surface*, SDL_Rect);

};
#endif

[hag++]

When you do a deep copy you need to copy ALL instance variables as well. In this case you are copying the char* but none of the int's

[Schol-R-LEA]

If you don't mind me asking, what is the ptr variable for in the first place? You seem to have two completely different non-copying constructor, one which initializes the int s and one which initializes ptr , and they appear to be mutually exclusive.

Oh, and none of the c'tors initializes the ex variable, which I was also curious about. What is that one for?

[mike_2000_17]

Your deep-copying is essentially correct. But you are really complicating things by not using the std::string class instead of char-arrays. Always prefer using RAII classes when they are available.

I also recommend you read my tutorial on building a RAII class.

As for your code:

Line 10 is useless.

Line 15 is incorrect. It should be delete[] ptr; because the ptr points to an array.

Line 41 is useless. This line just creates a temporary local variable that is called "ex" and that is never used. Your compiler should complain with a warning about an unused variable, if not, I remind you that you should always compile with the highest possible level of warnings (-Wall on GCC).

You have a lot of constructors, and most of them don't initialize all the data members. Even if you wish to default-construct a data member, you should make that obvious by putting it in the initialization list anyways.

With all those changes in mind, you should get this:

class Bullet {
  private:
    int x, y;
    int dx, dy;
    int startingX, startingY;
    std::string name;

    Extra ex;

  public:
    Bullet();
    //~Bullet(); //destructor not needed because all members are RAII.
    Bullet(const std::string& aName);
    Bullet(int startX, int startY, int moveX, int moveY); //

    //copy-constructor and assignment are not needed because all data members are RAII.
	
    int getX();
    int getY();
    int getDx();
    int getDy();
    int getStartingX();
    int getStartingY();

    void move();
    void show(SDL_Surface*, SDL_Surface*, SDL_Rect);

};

Bullet::Bullet() : x(0), y(0), 
                   dx(0), dy(0), 
                   startingX(0), startingY(0), 
                   name(), ex() { };

Bullet::Bullet(const std::string& aName) :
               x(0), y(0), dx(0), dy(0),
               startingX(0), startingY(0),
               name(aName), ex() { };

Bullet::Bullet(int startX, int startY, int moveX, int moveY) :
               x(0), y(0), dx(moveX), dy(moveY),
               startingX(startX), startingY(startY),
               name(), ex() { };

int Bullet::getX()
{
  return x;
}

int Bullet::getY()
{
  return y;
}

int Bullet::getDx()
{
  return dx;
}

int Bullet::getDy()
{
  return dy;
}

int Bullet::getStartingX()
{
  return startingX;
}

int Bullet::getStartingY()
{
  return startingY;
}

void Bullet::move()
{
  x += dx;
  y += dy;
}

void Bullet::show(SDL_Surface *bullet, SDL_Surface *screen, SDL_Rect camera)
{
  //Show the bullet
  ex.apply_surface( (startingX  - camera.x) + x, (startingY - camera.y) + y, bullet, screen );
}

[Johnny666]

It looks like that worked! Im shocked because i'v ben trying to solve this problem for a very long time, it no longer crashes :) I can't thank you enough!

One small issue though, for moving the bullets i incorporated the .move() in my Bullet class. 'x' and 'y' variables are supposed to accelerate my Bullet because in my .show() method my x and y variables are added to the x and y of the image blitted to the screen ->

ex.apply_surface( (startingX  - camera.x) + x, (startingY - camera.y) + y, bullet, screen );

Disregard the (startingX - camera.x) parts of it, they work fine.

Anyways so you'd think that everytime i call .move() i would accelerate the bullet by adding dx and dy to coords, but it doesnt i dont know why.

I start by calling the .move() in my main in my SDL loop.

for(int i = 0; i < myDot.getPlayerBullets().size() ; i++){
     myDot.getPlayerBullets().at( i ).move();
}

The above code is called every 2 seconds by my SDL timer just so debug purposes and what not.

Following that, i have the .show() method called every frame from my main SDL loop.

for(int i = 0; i < myDot.getPlayerBullets().size() ; i++){
     myDot.getPlayerBullets().at( i ).show( bulletImages[0], screen, camera, blah );
}

Any ideas why my when i call my .move() method i it doesn't change the 'x' and 'y' variables in my Bullet class as it should?

[raptr_dflo]

Your .move() and .show() look ok to me. Have you verified that x and y aren't changing (using getX() and getY())? Have you verified that dx and dy are indeed non-zero for each bullet? Also, do you remember to call SDL_Flip() (or whatever it's actually called, I don't have an SDL reference open at the moment) after drawing everything for that frame?