# Missile Command - Part 2 - Independent Missiles

## Recommended Posts

Hey everybody,

A little over a month ago, I made a post asking for help with my Missile Command game that I was making with regards to getting the Cannon to angle properly, and I was very pleased with all of the support.  It really lit a fire under me (even moreso before I worked up the courage to ask the first time).

Well, the game is not finished yet, but I have made a lot of progress since then ( it's very motivating  ) and I've been able to work through the code and logic for shooting missiles from the cannon.

The missile logic is almost ready, but there is one small bug that I discovered last week that has been wracking my brain non-stop, so I'm posting a new topic here.

Here is an animated GIF of the missile behavior so far, in this shot, it's working as expected when you click the Left mouse button to fire one missile at a time:

But what I didn't expect to happen, was here, when you click the Left button to fire a new missile when the other missile is still in transit:

When the next missile is fired, the original missile changes course, re-angles itself and flies towards the next clicked spot, abandoning it's first clicked target, which is not what I wanted lol.

It's like the missiles turn into smart missiles all of a sudden.  The good news is that they're being destroyed when they hit the spot anyways, so that's something.

So, for anyone reading this post, how can I make the previous missiles stay on their original target, even if I click the mouse to summon a new missile?

Each missile's path should be independent of each other.

This will also play into the logic for the upcoming falling Asteroid class I'll be adding, so solving this one for me will also allow me to make independent asteroids and I'm almost done the game! lol

Here's the code:

Game.h

#ifndef GAME_H_
#define GAME_H_

// Preprocessors
#include "SDL.h"
#include "SDL_image.h"
#include "SDL_ttf.h"
#include "SDL_mixer.h"
#include <iostream>
#include <sstream>
#include <stdio.h>
#include <math.h>
#include <cmath>
#include <vector>

// Inclusions for game objects
#include "Cannon.h"
#include "Missile.h"
#include "Ground.h"
#include "Silo.h"

#define FPS_DELAY           500
#define SCREEN_WIDTH        800
#define SCREEN_HEIGHT       600
#define MAX_ROTATION        0
#define MIN_ROTATION        180
#define DEG_TO_RAD(deg)     degrees * ( 3.141592653589793238 / 180)
#define OVER_PI             ( 180 / 3.141592653589793238 )

class Game
{
public:
Game();
~Game();

bool Init();
void Run();

private:
SDL_Window* window;
SDL_Renderer* renderer;

SDL_Texture* texture;

// Timing
unsigned int lastTick, fpsTick, fps, frameCount;

// Test
float testX, testY;

/*
Fonts for testing
*/
TTF_Font* fontMouseX;
TTF_Font* fontMouseY;
TTF_Font* fontCannonAngle;
TTF_Font* fontFire;
TTF_Font* fontAmmo;

std::stringstream xValue;
std::stringstream yValue;
std::stringstream angleValue;
std::stringstream fireValue;
std::stringstream ammoValue;

SDL_Texture* xTexture;
SDL_Texture* yTexture;
SDL_Texture* aTexture;
SDL_Texture* fTexture;
SDL_Texture* ammoTexture;

int xWidth, xHeight, yWidth, yHeight, aWidth, aHeight;
int fWidth, fHeight, ammoWidth, ammoHeight;

/*
END
*/

// Game objects
Cannon* cannon;                                 // The cannon, both the pipe and the base

std::vector<Missile*> missileVec;                // A vector of missiles

Silo* silo;
Ground* ground;                                 // The ground

SDL_Cursor* cursor;                             // The crosshair cursor

// Flag to check if the cannon
// is currently firing a missile
bool isFiring;
int clickedX;
int clickedY;

void Clean();                                   // Cleanup function
void Update(float delta);                       // Update game elements
void Render(float delta);                       // Render game elements

void NewGame();                                 // Start a new game

void SetCannonAngle( float vecX, float vecY);   // Set the angle of cannon pipe
double SetMissileAngle( float vecX, float vecY);  // Set the angle of missile
void ExplodeMissile();                          // Explode the missile once it
// has reached the target

void CheckAsteroidCollisions();                 // See if missile hits asteroid
void CheckSiloCollisions();                     // See if asteroid hits silo
int GetSiloCount();                             // Check if there are silos remaining

};

#endif // GAME_H_

Game.cpp

#include "Game.h"

Game::Game()
{
window = 0;
renderer = 0;
}

Game::~Game()
{

}

bool Game::Init()
{
// Initialize the SDL video and audio subsystems
SDL_Init(SDL_INIT_VIDEO | SDL_INIT_AUDIO);

// Create window
window = SDL_CreateWindow("Missile Command v1.0", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED,
800, 600, SDL_WINDOW_SHOWN | SDL_WINDOW_OPENGL);

if(!window)
{
std::cout << "Error creating window: " << SDL_GetError() << std::endl;
return false;
}

// Create renderer
renderer = SDL_CreateRenderer(window, -1, SDL_RENDERER_ACCELERATED);
if(!renderer)
{
std::cout << "Error creating renderer: " << SDL_GetError() << std::endl;
return false;
}

TTF_Init();

fontMouseX = TTF_OpenFont("lato.ttf", 14);
fontMouseY = TTF_OpenFont("lato.ttf", 14);
fontCannonAngle = TTF_OpenFont("lato.ttf", 14);
fontFire = TTF_OpenFont("lato.ttf", 14);
fontAmmo = TTF_OpenFont("lato.ttf", 14);

// Initialize resources
texture = SDL_CreateTextureFromSurface(renderer,surface);
SDL_FreeSurface(surface);

// Set mouse cursor to crosshair
cursor = SDL_CreateSystemCursor(SDL_SYSTEM_CURSOR_CROSSHAIR);
SDL_SetCursor(cursor);

// Initialize timing
lastTick = SDL_GetTicks();
fpsTick = lastTick;
fps = 0;                // Set starting FPS value
frameCount = 0;         // Set starting frame count

testX = 0;
testY = 0;

return true;
}

void Game::Clean()
{
// Clean resources
SDL_DestroyTexture(texture);
SDL_FreeCursor(cursor);
SDL_DestroyRenderer(renderer);
SDL_DestroyWindow(window);

// Clean test data
SDL_DestroyTexture(xTexture);
SDL_DestroyTexture(yTexture);
SDL_DestroyTexture(aTexture);
SDL_DestroyTexture(fTexture);
SDL_DestroyTexture(ammoTexture);
}

void Game::Run()
{
// Create game objects
cannon = new Cannon(renderer);
ground = new Ground(renderer);
silo = new Silo(renderer);

// Start a new game
NewGame();

// Main loop
while(1)
{
// Event handler
SDL_Event e;

// If event is a QUIT event, stop the program
if(SDL_PollEvent(&e))
{
if(e.type == SDL_QUIT)
{
break;
}

// If a mouse button was pressed...
else if(e.type == SDL_MOUSEBUTTONDOWN)
{
// If the left mouse button was pressed,
// set the "fire" flag to TRUE and
// record x,y position of click
if(e.button.button == SDL_BUTTON_LEFT)
{
clickedX = e.button.x;
clickedY = e.button.y;
isFiring = true;
}
}
}

// Calculate delta and fps
unsigned int curTick = SDL_GetTicks();
float delta = (curTick - lastTick) / 1000.0f;

// Cap FPS delay to specific amount
if(curTick - fpsTick >= FPS_DELAY)
{
fps = frameCount * (1000.0f / (curTick - fpsTick));
fpsTick = curTick;
frameCount = 0;
//std::cout << "FPS: " << fps << std::endl;
char buf[100];
snprintf(buf,100,"Missile Command v1.0 (fps: %u)", fps);
SDL_SetWindowTitle(window,buf);
}
else
{
frameCount++;
}

lastTick = curTick;

// Update and render the game
Update(delta);
Render(delta);
}

delete cannon;
//missileVec.clear();
delete ground;
delete silo;

Clean();

// Close the fonts
TTF_CloseFont(fontMouseX);
TTF_CloseFont(fontMouseY);
TTF_CloseFont(fontCannonAngle);
TTF_CloseFont(fontFire);
TTF_CloseFont(fontAmmo);

TTF_Quit();

SDL_Quit();
}

void Game::NewGame()
{

}

void Game::Update(float delta)
{
// Game logic

// Input, get mouse position to determine
// the cannon pipe's angle for rendering
int mX, mY;
SDL_GetMouseState(&mX,&mY);

// Assign the vector to variables
float vectorX = mX - cannon->x;
float vectorY = mY - cannon->y;

// "Snap" the cannon angle to the angle between
// cannon and mouse
SetCannonAngle( vectorX, vectorY );

// If cannon is firing, create a missile and add
// it to the missile vector container, and set
// "isFiring" to false again
if( isFiring == true )
{
Missile* newMissile = new Missile(renderer, cannon->x, cannon->y);
missileVec.push_back(newMissile);
isFiring = false;
}

// If there are missiles in the vector container, set their direction
// angle, fire them and update the missile positions
for( int i = 0; i < missileVec.size(); i++ )
{
// Update the missile's trajectory upon travel
missileVec[i]->endX = clickedX;
missileVec[i]->endY = clickedY;

// Set a vector between the clicked location
// and the missile
float targetX = clickedX - missileVec[i]->x;
float targetY = clickedY - missileVec[i]->y;

// Set facing angle for missile
missileVec[i]->angle = SetMissileAngle(targetX,targetY);

// Update the missile's position
missileVec[i]->Update(delta);

// If the missile is triggered to be destroyed,
// destroy it
if( missileVec[i]->destroyNow == true )
{
// Remove the destroyed missile
// from the vector
missileVec.pop_back();
}
}

/*// If there are no more silos,
// start a new game
if(GetSiloCount() == 0)
{
NewGame();
}*/

// Check if the cannon has fired a missile
//cannon->missile->Update(delta, mX - cannon->x, mY - cannon->y);

/*
Test Data
*/

// Stream the X, Y, Angle and isFired values
xValue.str("");     // Clear the stream before piping the xValue
xValue << "X: " << mX;

yValue.str("");     // Clear the stream before piping the yValue
yValue << "Y: " << mY;

angleValue.str(""); // Clear the stream before piping the angleValue
angleValue << "Angle: " << cannon->angle;

fireValue.str(""); // Clear the stream before piping the fireValue
fireValue << "isFiring: " << std::boolalpha << isFiring;

ammoValue.str(""); // Clear the stream before piping the ammoValue
ammoValue << "Missiles in Vector: " << missileVec.size();

// Set font color to WHITE
SDL_Color textColor = {255,255,255};

//***************************************
// DEBUG - Prepare the fonts for testing
//***************************************

// Render the X-coordinate text
SDL_Surface* temp = TTF_RenderText_Solid( fontMouseX, xValue.str().c_str(), textColor );
xTexture = SDL_CreateTextureFromSurface( renderer, temp );
xWidth = temp->w;
xHeight = temp->h;
SDL_FreeSurface(temp);

// Render the Y-coordinate text
temp = TTF_RenderText_Solid( fontMouseY, yValue.str().c_str(), textColor );
yTexture = SDL_CreateTextureFromSurface( renderer, temp );
yWidth = temp->w;
yHeight = temp->h;
SDL_FreeSurface(temp);

// Render the angle text
temp = TTF_RenderText_Solid( fontCannonAngle, angleValue.str().c_str(), textColor );
aTexture = SDL_CreateTextureFromSurface( renderer, temp );
aWidth = temp->w;
aHeight = temp->h;
SDL_FreeSurface(temp);

// Render the isFiring text
temp = TTF_RenderText_Solid( fontFire, fireValue.str().c_str(), textColor );
fTexture = SDL_CreateTextureFromSurface( renderer, temp );
fWidth = temp->w;
fHeight = temp->h;
SDL_FreeSurface(temp);

// Render the missileVec.size() text
temp = TTF_RenderText_Solid( fontAmmo, ammoValue.str().c_str(), textColor );
ammoTexture = SDL_CreateTextureFromSurface( renderer, temp );
ammoWidth = temp->w;
ammoHeight = temp->h;
SDL_FreeSurface(temp);
}

void Game::SetCannonAngle(float vecX, float vecY)
{
// "Snap" cannon angle to angle where mouse
// cursor is located in order to track it
double theAngle = atan2(vecY, vecX);

// Pass radian angle to convert to degrees
theAngle = ConvertDegrees(theAngle);

// Convert degrees format from -180 to 180
// to 0 to 360
if( theAngle < 0 )
{
theAngle = 360 - (-theAngle);
}

if(theAngle > MAX_ROTATION && theAngle < 90)
{
cannon->angle = MAX_ROTATION;
}
else if(theAngle < MIN_ROTATION && theAngle > 90)
{
cannon->angle = MIN_ROTATION;
}
else
{
cannon->angle = theAngle;
}
}

double Game::SetMissileAngle(float vecX, float vecY)
{
// "Snap" missile angle to angle where mouse
// cursor is located in order to track it
double theAngle = atan2(vecY,vecX);

// Pass radian angle to convert to degrees
theAngle = ConvertDegrees(theAngle);

// Convert degrees format from -180 to 180
// to 0 to 360
if( theAngle < 0 )
{
theAngle = 360 - (-theAngle);
}

return theAngle;
}

{
// Convert the angle from radians to
// degrees and return the value
}

{
// Convert the angle from degrees to
// radians and return the value
}

int Game::GetSiloCount()
{
int siloCount = 0;

// If the current silo is still alive,
// the game is not over
if(silo->state)
{
siloCount = 1;
}

return siloCount;
}

void Game::Render(float delta)
{
// Clear the renderer
SDL_RenderClear(renderer);

// Render the game objects
cannon->Render(delta);
ground->Render(delta);
silo->Render(delta);

// If there is a missile in the vector,
// render it
for( int i = 0; i < missileVec.size(); i++ )
{
missileVec[i]->Render(delta);
}

/*
Setting source rectangles and then
rendering the x, y and angle fonts
*/

// For x
SDL_Rect rect;
rect.x = 20;
rect.y = 20;
rect.w = xWidth;
rect.h = xHeight;
SDL_RenderCopy(renderer, xTexture, 0, &rect);

// For y
rect.x = 20;
rect.y = 40;
rect.w = yWidth;
rect.h = yHeight;
SDL_RenderCopy(renderer, yTexture, 0, &rect);

// For angle
rect.x = 20;
rect.y = 60;
rect.w = aWidth;
rect.h = aHeight;
SDL_RenderCopy(renderer, aTexture, 0, &rect);

// For isFired
rect.x = 20;
rect.y = 80;
rect.w = fWidth;
rect.h = fHeight;
SDL_RenderCopy(renderer, fTexture, 0, &rect);

// For isFired
rect.x = 20;
rect.y = 100;
rect.w = ammoWidth;
rect.h = ammoHeight;
SDL_RenderCopy(renderer, ammoTexture, 0, &rect);

// Present the renderer to display
SDL_RenderPresent(renderer);
}

Missile.h

#ifndef MISSILE_H_INCLUDED
#define MISSILE_H_INCLUDED

#include "Entity.h"

#include <math.h>

class Missile : public Entity
{
public:
Missile(SDL_Renderer* renderer, float cX, float cY);
~Missile();

void Update(float delta);
void Render(float delta);

void Fire();

void Move();

// Starting and ending positions for missile
float startX, startY, endX, endY;
float directionX, directionY;
float distance, speed;

// The missile's angle
double angle;

// Flag to have missile destroyed
// when certain conditions are met
bool destroyNow;

private:
SDL_Texture* texture;           // Texture for missile
SDL_Point center;
};

#endif // MISSILE_H_INCLUDED

Missile.cpp

#include "Missile.h"

Missile::Missile(SDL_Renderer* renderer, float cX, float cY) : Entity(renderer)
{
// Create missile texture
texture = SDL_CreateTextureFromSurface(renderer,surface);
SDL_FreeSurface(surface);

// Set angle variable default,
// width, height
angle = 0.0f;
width = 34;
height = 13;

// Set missile offset for positioning relative to cannon
float xOffset = 8;

// Set starting x,y position
x = cX + xOffset;
y = cY;

// Set the missile's starting x and y position
// and speed
startX = x;
startY = y;
speed = 150;

// Set the missile's center for rotation
center.x = ( width / 2 );
center.y = ( height / 2 );

// Set destruction flag to false
destroyNow = false;
}

Missile::~Missile()
{
// Clean resources
SDL_DestroyTexture(texture);
}

void Missile::Move()
{
// Determine the distance from the starting
// position to the ending position
distance = sqrt( pow(endX - startX, 2) + pow(endY - startY, 2));

// Determine direction of movement
directionX = (endX - startX) / distance;
directionY = (endY - startY) / distance;
}

void Missile::Update(float delta)
{
// Call move function
Move();

// If the missile is moving,
// update its position
this->x += directionX * speed * delta;
this->y += directionY * speed * delta;

// If the missile goes BEYOND the
// destination x and y (endX and endY),
// set the missile's position to endX
// and endY and stop movement
if( sqrt( pow( this->x - startX, 2) + pow( this->y - startY, 2) ) >= distance )
{
this->x = endX;
this->y = endY;
this->destroyNow = true;
}
}

void Missile::Render(float delta)
{
// Render the missile
SDL_Rect rect;
rect.x = (int)(x + 0.5f);
rect.y = (int)(y + 0.5f);
rect.w = width;
rect.h = height;

// Render the missile at given angle
SDL_RenderCopyEx(renderer,texture,0,&rect,angle,&center,SDL_FLIP_NONE);
}


I appreciate any feedback, guys.  Thanks in advance.

Edited by Glydion
Changing syntax styling

##### Share on other sites

Hi Glydion, one small suggestion, you should edit the post above, double click on the code and choose the C++ display option, it makes it easier to read

##### Share on other sites

In general, try to post only the relevant bits of code if you can, instead of all code.

The bug lies in Game::Update. When you loop over all missiles, you set all missiles to head towards clickedX and clickedY (you set the end variables for all missiles).

Instead, only set the end variable when you create a new missile. Maybe add it to the constructor and pass it in there.

Doing this will uncover a new bug. All missiles should now fly to where they are intended to, but they will still keep on facing where you clicked. This is a similar bug to the one above -- you are using clickedX and clickedY instead of endX and endY to calculate targetX and targetY.

As a last thing, I would suggest changing the way you comment stuff. Currently, you are commenting what the code does, in a lot of detail. Instead, try commenting only the cases where it isn't obvious why the code is doing what it does.

Stuff like "Ground *ground; //The ground" is unnecessary, and it forces the reader (in most cases, the reader is you) to have to read a lot more than they really need.

##### Share on other sites

I'd rather you called this an "unexpected feature" than a bug. Homing missiles are cool.

Maybe after you fix it, you can still keep that behavior so that it activates with a powerup or some special code.

##### Share on other sites
15 hours ago, Alpha_ProgDes said:

I'd rather you called this an "unexpected feature" than a bug. Homing missiles are cool.

Maybe after you fix it, you can still keep that behavior so that it activates with a powerup or some special code.

Hey Alpha!  Thank you for writing that article about the journey for beginner programmers and the 10 "must make" games to break into the industry.  It has been a big inspiration for me in my journey to become a 2D game programmer so far and has helped me not get burnt out when learning this stuff.

It's been a great way to learn how to program 2D games incrementally and I've found it is easy to stay motivated with this model of progress.

My dream is to start my own indie company, Glydion, and make my own 2D games that remind players of the 16-bit era (Super Nintendo, Sega, Neo-Geo, etc.) which is where my passion is.

Yes lol, I'll make this "homing" feature a powerup for when I go through these 10 games again and make (version 2.0) versions of these games in the future.

22 hours ago, Lactose said:

In general, try to post only the relevant bits of code if you can, instead of all code.

The bug lies in Game::Update. When you loop over all missiles, you set all missiles to head towards clickedX and clickedY (you set the end variables for all missiles).

Instead, only set the end variable when you create a new missile. Maybe add it to the constructor and pass it in there.

Doing this will uncover a new bug. All missiles should now fly to where they are intended to, but they will still keep on facing where you clicked. This is a similar bug to the one above -- you are using clickedX and clickedY instead of endX and endY to calculate targetX and targetY.

As a last thing, I would suggest changing the way you comment stuff. Currently, you are commenting what the code does, in a lot of detail. Instead, try commenting only the cases where it isn't obvious why the code is doing what it does.

Stuff like "Ground *ground; //The ground" is unnecessary, and it forces the reader (in most cases, the reader is you) to have to read a lot more than they really need.

Hey Lactose, thank you for your feedback.  Yeah I think you're right, I do seem to be commenting on lines that are pretty self-explanatory, which must make it hard for others to sift through it.

I think I took that "comment everywhere" feedback from my early programming courses too seriously lol  .

Hmm, I've never thought of passing the clickedX and clickedY in the missile constructor.

I'll change it up with your suggestions and run some tests.  Thank you.

22 hours ago, MarcusAseth said:

Hi Glydion, one small suggestion, you should edit the post above, double click on the code and choose the C++ display option, it makes it easier to read

Hey Marcus!  Hope you've been doing well, friend.

I didn't even realize that the code I inserted was in HTML hehe.  I've changed it to C++ now for enhanced readability.  My bad.

##### Share on other sites

Hey guys, just wanted to update on this topic.

Lactose hit the nail right on the head, as soon as I passed the clickedX and clickedY into the constructor, the missiles no longer "re-aligned" themselves to the next missile but continued on their original path niiiice.

Here's an animated GIF for a visual:

Now I have a new bug hehe, all of the missiles, regardless of how many have been fired, will all be destroyed (disappear) at the exact same time lol.

Let me try to fix that and I'll post an update about it later.  Thanks again to all who responded.  Have a good one.

##### Share on other sites

Hi everyone, an update to the new bug in my program.

I can't seem to figure out why all the missiles are popping out of the vector at the exact same time.

Here's another GIF to illustrate and the pertinent code below.

https://i.imgur.com/4dZjEJv.gifv

The only case where the missiles don't all disappear concurrently is if the first missile path is very far away from the cannon, as shown in the 1st launch test in the GIF above.

In this example, I'm able to get 4 quick shots off before the original 1st missile reaches the top of the screen.

Here's what I got for code:

Game.cpp

void Game::Update(float delta)
{
// Game logic
int mX, mY;
SDL_GetMouseState(&mX,&mY);

float vectorX = mX - cannon->x;
float vectorY = mY - cannon->y;

SetCannonAngle( vectorX, vectorY );

if( isFiring == true )
{
Missile* newMissile = new Missile(renderer, cannon->x, cannon->y, clickedX, clickedY);
missileVec.push_back(newMissile);
isFiring = false;
}

// If there are missiles in the vector container, set their direction
// angle, fire them and update the missile positions
for( int i = 0; i < missileVec.size(); i++ )
{
float targetX = missileVec[i]->endX - missileVec[i]->x;
float targetY = missileVec[i]->endY - missileVec[i]->y;

missileVec[i]->angle = SetMissileAngle(targetX,targetY);

missileVec[i]->Update(delta);

if( missileVec[i]->destroyNow == true )
{
missileVec.pop_back();
}
}
}

Missile.cpp

void Missile::Update(float delta)
{
// Call move function
Move();

// If the missile is moving,
// update its position
this->x += directionX * speed * delta;
this->y += directionY * speed * delta;

// If the missile goes BEYOND the
// destination x and y (endX and endY),
// set the missile's position to endX
// and endY and stop movement
if( sqrt( pow( this->x - startX, 2) + pow( this->y - startY, 2) ) >= distance )
{
this->x = endX;
this->y = endY;
this->destroyNow = true;
}
}

As per the above code, each missile has a Boolean destroy flag (destroyNow) that signals when to pop the missile out of the vector, which I have set to when the missile reaches its destination on the screen.

So this works when the first missile has to travel a longer distance than the subsequent missiles that succeed it, but when the first missile has a short path and then you click the Left mouse button to set a longer path for the next missile, both of them are popped out of the vector and disappear at the same time, when their destruction should be independent of each other.

I've been racking my brain trying to figure it out for 2 days' straight so my brain is now mush lol.

How would you all go about removing missiles independently of each other given my code above?

I appreciate all feedback, thank you in advance.

Edited by Glydion

##### Share on other sites
if( missileVec[i]->destroyNow == true )
{
missileVec.pop_back();
}

This seems fishy to me... if your first missile is set to be destroyed you are doing pop_back() effectively destroying the last missile which has nothing to do with the first

Also if I am not mistaken, by doing pop_back() on a vector of raw pointers you now have leaked a missile because delete is never called on that missile thus its destructor is not fired and you thrown away the only pointer to it

I would try using a different data structure to store the Missiles, one that allows you to remove from the middle of it as well like a DoubyLinkedList (called list in C++ I think) or maybe a map where the key is the missile ID which you assign to every missile at spawn and have a static variable distributing IDs, or maybe I would try to keep it a vector but when you find a missile that need to be destroyed, you swap it with the last, carefully destroy the last element and decrese your counter loop by one so you process again the current ID which is the previous(before the swap) last element.

Edited by MarcusAseth

##### Share on other sites
Missile* newMissile = new Missile(renderer, cannon->x, cannon->y, clickedX, clickedY);
missileVec.push_back(newMissile);

If it isn't clear what you are doing here, you are essentially creating a pointer named newMissile, which then has its value set to point to the memory that is allocated by using new, so now only newMissile refers to that memory.

Then you use push_back on it, which copies the value of the pointer into the vector, so now we have two things pointing at that memory.

for( int i = 0; i < missileVec.size(); i++ )
{
float targetX = missileVec[i]->endX - missileVec[i]->x;
float targetY = missileVec[i]->endY - missileVec[i]->y;

missileVec[i]->angle = SetMissileAngle(targetX,targetY);

missileVec[i]->Update(delta);

if( missileVec[i]->destroyNow == true )
{
missileVec.pop_back();
}
}

Now this part is an issue, basically you're iterating over each pointer to a missile. The problem is that presumably the first missile in the container will be the earliest fired one, and likely to explode the first. What happens when it explodes? it sets destroynow to true, and then the next time the loop runs the first missile it sees is one that is destroyed.

Problem is push_back and pop_back do exactly what they say, they push or pop the element on the back. So now when your first missile explodes, whether you have 1 missile or 10, it removes the last one. Then it goes to the next ones, they may not be destroyed yet, so it exits the loop. But what happens the next update loop? It checks the first missile, sees it is destroyed, and pops the back again. Basically you're going to infinitely pop off all of your missiles.

In addition, you are popping the missile pointers off without ever calling delete on them to free the memory they point to.

There's a few methods you could use to remove elements in a vector like this. If you use an iterator(missileVec::iterator) then you can use erase when you get to a missile you want gone. Erase will remove that element and shift the other elements back to fill in the space. Another method is swap and pop, essentially if there are more than two elements in the vector you take the last element and swap it with the one you want to erase, then pop it off the back.

Just make sure to delete these missiles, or use a smart pointer like unique_ptr, which is vector safe.

EDIT: Managing memory and ownership like this is a pretty big part of any program, especially stuff like this for games. It might be worthwhile for you to research your options a bit and understand some of the various ways you can store things and pass ownership or references around.

Edited by Satharis

##### Share on other sites

I looked at your code real fast :

Why does the missile need to delete a texture ?

You should have only 1 texture loaded for all missiles.

##### Share on other sites
23 hours ago, MarcusAseth said:

if( missileVec[i]->destroyNow == true )
{
missileVec.pop_back();
}

This seems fishy to me... if your first missile is set to be destroyed you are doing pop_back() effectively destroying the last missile which has nothing to do with the first

Also if I am not mistaken, by doing pop_back() on a vector of raw pointers you now have leaked a missile because delete is never called on that missile thus its destructor is not fired and you thrown away the only pointer to it

I would try using a different data structure to store the Missiles, one that allows you to remove from the middle of it as well like a DoubyLinkedList (called list in C++ I think) or maybe a map where the key is the missile ID which you assign to every missile at spawn and have a static variable distributing IDs, or maybe I would try to keep it a vector but when you find a missile that need to be destroyed, you swap it with the last, carefully destroy the last element and decrese your counter loop by one so you process again the current ID which is the previous(before the swap) last element.

Hey Marcus,

Oh right, pop_back removes the last element, which is not the first Missile if I click the left button more than once, hmmm.

I actually tried to place a delete statement in my code after destroyNow is set to true, but then I figured out that newMissile only exists within the scope of the if( isFiring == true ) block and falls out of scope when the If block is done, so I pushed it into the Vector in the code above because I didn't want the missile to be lost (that was my thinking at the time).

And my program freezes if I try to call delete missileVec within the loop so yes, I seriously have to reconsider my Missile container.

20 hours ago, Satharis said:

Missile* newMissile = new Missile(renderer, cannon->x, cannon->y, clickedX, clickedY);
missileVec.push_back(newMissile);

If it isn't clear what you are doing here, you are essentially creating a pointer named newMissile, which then has its value set to point to the memory that is allocated by using new, so now only newMissile refers to that memory.

Then you use push_back on it, which copies the value of the pointer into the vector, so now we have two things pointing at that memory.


for( int i = 0; i < missileVec.size(); i++ )
{
float targetX = missileVec[i]->endX - missileVec[i]->x;
float targetY = missileVec[i]->endY - missileVec[i]->y;

missileVec[i]->angle = SetMissileAngle(targetX,targetY);

missileVec[i]->Update(delta);

if( missileVec[i]->destroyNow == true )
{
missileVec.pop_back();
}
}

Now this part is an issue, basically you're iterating over each pointer to a missile. The problem is that presumably the first missile in the container will be the earliest fired one, and likely to explode the first. What happens when it explodes? it sets destroynow to true, and then the next time the loop runs the first missile it sees is one that is destroyed.

Problem is push_back and pop_back do exactly what they say, they push or pop the element on the back. So now when your first missile explodes, whether you have 1 missile or 10, it removes the last one. Then it goes to the next ones, they may not be destroyed yet, so it exits the loop. But what happens the next update loop? It checks the first missile, sees it is destroyed, and pops the back again. Basically you're going to infinitely pop off all of your missiles.

In addition, you are popping the missile pointers off without ever calling delete on them to free the memory they point to.

There's a few methods you could use to remove elements in a vector like this. If you use an iterator(missileVec::iterator) then you can use erase when you get to a missile you want gone. Erase will remove that element and shift the other elements back to fill in the space. Another method is swap and pop, essentially if there are more than two elements in the vector you take the last element and swap it with the one you want to erase, then pop it off the back.

Just make sure to delete these missiles, or use a smart pointer like unique_ptr, which is vector safe.

EDIT: Managing memory and ownership like this is a pretty big part of any program, especially stuff like this for games. It might be worthwhile for you to research your options a bit and understand some of the various ways you can store things and pass ownership or references around.

Hi Satharis, I wasn't kidding when I said I was an absolute beginner, and I would have never realized what I was doing without someone pointing it out for me.

As I mentioned to Marcus, my worry with newMissile was that it would be out of scope as soon as the isFiring IF block was completed, which is why I thought that adding it to the vector was the only way to save the missile data so I can update it in a For loop further down.  I didn't even consider that I had two pointers instead of just one, so thanks for letting me know.

That also lended itself to my delete issue with a missile that was to be destroyed, I tried earlier to call destroy missileVec but it just ends up crashing the program.  So I need to figure out how to delete the memory after popping off the pointers.

The "swap and pop" method for deleting objects from a vector sounds like a good idea and I would get some good practice in to implement it in my game.  I'll give it a shot.

But first things first, I'll have to look at how to free the memory for destroyed missiles.

Yes, I definitely need to research memory management.  Can you clarify what you mean by ownership in this context?  Thank you.

12 hours ago, the incredible smoker said:

I looked at your code real fast :

Why does the missile need to delete a texture ?

You should have only 1 texture loaded for all missiles.

Hello Smoker,

I'm not 100% sure what you mean.  The missile destructor frees the memory that was used by its own missile texture upon deletion.

Don't objects have to be responsible for deleting their own resources?  Please clarify.

Yes, I do load the same texture every time a new missile is created.  Hmmm, how would I go about loading the texture only one time if a missile is created when a Left mouse click occurs?  I can't seem to visualize how to load the missile texture only once if I'm calling the constructor with each new missile.  Thanks.

##### Share on other sites
Quote

But first things first, I'll have to look at how to free the memory for destroyed missiles.

Just let a smart pointer handle it for you

#include <memory>
std::vector<std::shared_ptr<Missile>> Missiles;
std::shared_ptr<Missile> NewMissile = std::make_shared<Missile>(/*Missile Constructor arguments here */);
Missiles.push_back(NewMissile);

I am not too familiar with shared_ptr yet, but if I got it right it keeps an internal count of "how many co-owners to the object he is managing there are" (object he is managing is your Missile), and when all this co-owners (meaning multiple shared_ptr) are destroyed, only then your Missile destructor is called, so basically when you call pop_back() the missile destructor is fired immediately since you have a single shared_ptr managing your Missile, and even with many of this pointing to the same object you can't delete it twice.

As a rule of thumb, if you're a begginner like me in 2017 and you're writing new and delete, then something is wrong (I often do the same though, this old books fault  )

Edited by MarcusAseth

##### Share on other sites
1 hour ago, Glydion said:

As I mentioned to Marcus, my worry with newMissile was that it would be out of scope as soon as the isFiring IF block was completed, which is why I thought that adding it to the vector was the only way to save the missile data so I can update it in a For loop further down.  I didn't even consider that I had two pointers instead of just one, so thanks for letting me know.

That also lended itself to my delete issue with a missile that was to be destroyed, I tried earlier to call destroy missileVec but it just ends up crashing the program.  So I need to figure out how to delete the memory after popping off the pointers.

The "swap and pop" method for deleting objects from a vector sounds like a good idea and I would get some good practice in to implement it in my game.  I'll give it a shot.

But first things first, I'll have to look at how to free the memory for destroyed missiles.

Well no, you are right, it does go out of scope, then you only have the vector pointer referring to the missile object. I only really pointed that out to make it clear that they were pointers and where they were going.

Thing to remember about pointers is that they are just variables themselves so you can delete the memory they point to even while they're still in the vector, they just will be pointing to unallocated memory if you try to use them after that.

if( missileVec[i]->destroyNow)
{
auto size = missileVec.size();
auto lastIndex = size - 1;

if(size > 1 && i != lastIndex)
{
Missile* swap = missileVec[lastIndex];
missileVec[lastIndex] = missileVec[i];
missileVec[i] = swap;
--i;
}

delete missileVec[lastIndex];
missileVec.pop_back();
}

Something like this would work for deleting.. though I probably did something stupid in it, but you get the idea.

Alternative is to use a smart pointer in the container, then it just deletes it when you pop it off.

##### Share on other sites

Hi, normally i have a class for the list, wich holds the texture and first missile.

You pass the list to the missile constructor so the missile can have a pointer to the texture from the list.

You only load the texture once, at startup of your level, and delete it when you close your level.

##### Share on other sites
12 hours ago, Satharis said:

Well no, you are right, it does go out of scope, then you only have the vector pointer referring to the missile object. I only really pointed that out to make it clear that they were pointers and where they were going.

Thing to remember about pointers is that they are just variables themselves so you can delete the memory they point to even while they're still in the vector, they just will be pointing to unallocated memory if you try to use them after that.


if( missileVec[i]->destroyNow)
{
auto size = missileVec.size();
auto lastIndex = size - 1;

if(size > 1 && i != lastIndex)
{
Missile* swap = missileVec[lastIndex];
missileVec[lastIndex] = missileVec[i];
missileVec[i] = swap;
--i;
}

delete missileVec[lastIndex];
missileVec.pop_back();
}

Something like this would work for deleting.. though I probably did something stupid in it, but you get the idea.

Alternative is to use a smart pointer in the container, then it just deletes it when you pop it off.

That isn't the best way to delete an item in a vector at all IMO.  You can do something much simplier:

for (auto it = missileVec.begin(); it != missileVec.end();) {
if (it->deleteNow) {
delete *it;
it = missileVec.erase(it);
}
else {
++it;
}
}	
Basically using iterators to loop through the missile vectors and if one needs to be deleted, you can delete the pointer, then remove the pointer form the vector, else move to the next one.
Edited by BeerNutts

##### Share on other sites
6 hours ago, BeerNutts said:

Basically using iterators to loop through the missile vectors and if one needs to be deleted, you can delete the pointer, then remove the pointer form the vector, else move to the next one.

Yes, and I specified that you could use erase, or something like swap and pop if he didn't want to use an iterator. Which is why I rewrote it using the same method he was using.

The iterator method is also a waste of time if you already have the index of what you want to get rid of, in this case it is fine since he is examining the entire container, but yeah.

EDIT: More specifically in situations where performance is a concern erase is actually slower than swap and pop anyway since it shifts the elements back and ends up being O(n).

Edited by Satharis

##### Share on other sites
On 9/12/2017 at 11:24 PM, Satharis said:

Well no, you are right, it does go out of scope, then you only have the vector pointer referring to the missile object. I only really pointed that out to make it clear that they were pointers and where they were going.

Thing to remember about pointers is that they are just variables themselves so you can delete the memory they point to even while they're still in the vector, they just will be pointing to unallocated memory if you try to use them after that.


if( missileVec[i]->destroyNow)
{
auto size = missileVec.size();
auto lastIndex = size - 1;

if(size > 1 && i != lastIndex)
{
Missile* swap = missileVec[lastIndex];
missileVec[lastIndex] = missileVec[i];
missileVec[i] = swap;
--i;
}

delete missileVec[lastIndex];
missileVec.pop_back();
}

Something like this would work for deleting.. though I probably did something stupid in it, but you get the idea.

Alternative is to use a smart pointer in the container, then it just deletes it when you pop it off.

Hey Satharis,

Thanks for providing a prototype of how to implement the "swap and pop" method with respect to the code I provided.  It really helped me understand how to delete the desired missile, and now the missiles are popped out properly and they are now being deleted to free the memory, hopefully preventing any memory leaks.

Since this is just a "version 1" of this Missile Command game, I'm okay with swap and pop to remove missiles at this point as I will be returning to this game a tiny bit later and re-factoring it (as AlphaProgDes recommends in his article).

I'll practice "iterating" through the vector the second time around, as per Beer Nut's suggestion.

Yeah, so the issue was indeed the fact that "pop_back" on its own just removes the last missile that was added at the end, without realizing that the missile I wanted deleted was the "older" missile that was already in the vector container.

I'll be moving on to creating and managing falling asteroids next as part of the Missile Command game, and I'll post an update later on once I've made progress.

Thanks to everyone who posted a response to help me out, I'm going to read up on memory management until I understand it.  Have a good night.

## Create an account

Register a new account

• ### Forum Statistics

• Total Topics
628282
• Total Posts
2981805
• ### Similar Content

• Does anyone have any experience (good or bad) using Docker for developing a game with SDL and/or C++?
I can see it being useful for sharing the build environment, though games typically being compiled, GUI applications makes Docker seem like a bit more of a foreign concept.
• By CodeBro1
I can't get SDL to make any rectangles on my Mac. The program I wrote worked completely fine on a Windows computer, but whenever I call the SDL_FillRect function on my Mac, nothing appears on the screen. I suspect this may be caused by a compilation issue. I tried to install SDL Unix-style using Homebrew. Inserted below is what I write into the terminal and my code.

g++ -o main test.cpp -L/usr/local/Cellar/sdl2/2.0.6/lib -lSDL2 -lSDL2main -I/usr/local/Cellar/sdl2/2.0.6/include/SDL2
#include <SDL.h> #include <iostream> #include <stdio.h> const int SCREEN_WIDTH = 640; const int SCREEN_HEIGHT = 480; const int PLAYER_WIDTH = 20; const int PLAYER_HEIGHT = 20; SDL_Window* w = NULL; SDL_Surface* s = NULL; SDL_Renderer* r = NULL; SDL_Rect BG = {0,0,SCREEN_WIDTH,SCREEN_HEIGHT}; bool init() { if (SDL_Init(SDL_INIT_VIDEO) < 0) { printf("Couldn't initialize. error: %s\n", SDL_GetError() ); return false; } else { w = SDL_CreateWindow("test", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, SCREEN_WIDTH, SCREEN_HEIGHT, SDL_WINDOW_SHOWN); if(w == NULL) { printf("Couldn't make window. error:%s\n", SDL_GetError() ); return false; } else { s = SDL_GetWindowSurface(w); r = SDL_CreateRenderer(w,-1,SDL_RENDERER_ACCELERATED | SDL_RENDERER_PRESENTVSYNC); SDL_SetRenderDrawColor(r,0,0,0,0); return true; } } } void close() { SDL_FreeSurface(s); SDL_DestroyWindow(w); SDL_DestroyRenderer(r); SDL_Quit(); } class Player { private: int x; int y; int xvel; int yvel; int acceleration; SDL_Rect collider; public: bool landed; Player() { x = 0; y = SCREEN_HEIGHT - PLAYER_HEIGHT; xvel = 0; yvel = 0; acceleration = -1; collider.x = x; collider.y = y; collider.w = PLAYER_WIDTH; collider.h = PLAYER_HEIGHT; landed = true; } int getx() { return x; } int gety() { return y; } void setx(int num) { x = num; } void sety(int num) { y = num; } void setyvel(int num) { yvel += num; } void setxvel(int num) { xvel += num; } void move() { x += xvel; if( (y + PLAYER_HEIGHT) > SCREEN_HEIGHT) { y = SCREEN_HEIGHT - PLAYER_HEIGHT; yvel = 0; landed = true; } else { if(landed != true) { y += yvel; yvel = yvel - acceleration; } } collider.x = x; collider.y = y; } void render() { SDL_SetRenderDrawColor(r,255,255,255,255); SDL_RenderFillRect(r,&collider); SDL_SetRenderDrawColor(r,0,0,0,255); } }; int main(int argc, char *argv[]) { bool quit = false; SDL_Event e; //event for event polling Player p; int count = 0; if(init() == false) { printf("Init failed: %s\n", SDL_GetError()); } else { while(quit == false ) { while( SDL_PollEvent(&e) != 0) { if(e.type == SDL_QUIT) { quit = true; } if(e.type == SDL_KEYDOWN) { switch(e.key.keysym.sym) { case SDLK_LEFT: if(e.key.repeat == 0) { p.setxvel(-10); } break; case SDLK_RIGHT: if(e.key.repeat == 0) { p.setxvel(10); } break; case SDLK_UP: if(count < 5) { p.setyvel(-3); ++count; } p.landed = false; break; } } if(e.type == SDL_KEYUP && e.key.repeat == 0) { switch(e.key.keysym.sym) { case SDLK_LEFT: p.setxvel(10); break; case SDLK_RIGHT: p.setxvel(-10); break; case SDLK_UP: count = 0; break; } } } SDL_RenderClear(r); SDL_RenderFillRect(r,&BG); p.move(); p.render(); SDL_RenderPresent(r); } } close(); }

• https://github.com/ingframin/CPongC/tree/master
I have a bug that make my game crash outside of code::blcoks but run just fine in it.
My guess is that I have an uninitialized pointer somewhere that is initialized by codeblocks when I run the code inside it.
Can someone please try and let me know?
I use codeblocks + GCC under Windows.
• By Romee
Hello everyone,
For learning purposes, I want to try to make a small graphical snake game in assembly. I've thought about where to start and which tools to use and personally I thought about using SDL and C to handle the keyboard input and the graphical output, but I would really like to know from other if there are better/easier ways to do this.

Thanks
• By Dzuvan
Hello, I'm having a rather strange bug. I'm making puzzle game.
When the game starts(PlayState) for the first time  pieces are indeed correctly placed, but after that sometimes they appear fine and most of the time only half of each piece is present ( I'm composing them out of 2 rectangles). The other parts appear at coordinates like this : Part2: X[1] : 842150923, Y[1]: 842150691. It happens even if I don' t use rand().
I'm using a state machine:
PlayState can go to:
- pause state
PauseState can go to:
- Resume