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) );
		}
	}
}
c++

Recommended Answers

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

Jump to Post

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, …
Jump to Post

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

Jump to Post

All 9 Replies

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

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

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.

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

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

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?

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 );
}

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?

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?

Be a part of the DaniWeb community

We're a friendly, industry-focused community of 1.21 million developers, IT pros, digital marketers, and technology enthusiasts learning and sharing knowledge.