Manage dynamic sprites on screen

[TheEnigmist]

I'm enjoying with SFML and i was thinking about FPS and bullets. So i thought:
-player fires unknown number of bullets;
-all bullets must be destroyed or game will go in memory leak;
So i was learning about dynamic sprite on screen and deleting them after X coords reach App.Width/2.
My code works fine (i spent 2h to make it work), but my question is, there is better way to manage this problem or my code is already optimized?

Code: [Select]


#include <SFML/Graphics.hpp>
#include <iostream>
#include <list>

int main()
{
sf::RenderWindow App(sf::VideoMode(800,600,32),"SFML AnimTest",sf::Style::Titlebar | sf::Style::Close);
App.SetFramerateLimit(300);
sf::Event event;

sf::Texture bulletTexture;
bulletTexture.LoadFromFile("data/img/fire.png");

sf::Sprite *bulletSpritePtr=NULL;

std::list<sf::Sprite *> bulletList;
std::list<sf::Sprite *>::const_iterator bulletIterator;

float speed = 400.0f;

while(App.IsOpen()){

float FPS = App.GetFrameTime() / 1000.f;

if (App.PollEvent(event))
{
if (event.Type == sf::Event::Closed || (event.Type == sf::Event::KeyPressed && event.Key.Code == sf::Keyboard::Escape))
App.Close();
if(event.Type == sf::Event::MouseButtonPressed && event.MouseButton.Button == sf::Mouse::Left){
bulletSpritePtr= new sf::Sprite(bulletTexture);
bulletSpritePtr->SetPosition(sf::Mouse::GetPosition(App).x,sf::Mouse::GetPosition(App).y);
bulletList.push_back(bulletSpritePtr);
bulletSpritePtr=NULL;
}
}

//MOVE BULLETS
if(!bulletList.empty()){
for (bulletIterator = bulletList.begin(); bulletIterator != bulletList.end(); bulletIterator++){
(**bulletIterator).Move(speed * FPS, 0);
if((**bulletIterator).GetPosition().x > App.GetWidth()/2)
bulletSpritePtr=*bulletIterator;
}
}

//DELETE ALL BULLETS > APP.WITH / 2
if(bulletSpritePtr!=NULL){
bulletList.remove(bulletSpritePtr);
bulletSpritePtr=NULL;
}

App.Clear(sf::Color::Black);

if (!bulletList.empty())
for(bulletIterator = bulletList.begin(); bulletIterator != bulletList.end(); bulletIterator++)
App.Draw(**bulletIterator);

App.Display();
}
return EXIT_SUCCESS;
}

[Nexus]

I will never understand why so many people consider it a good idea to keep owning pointers in STL containers. More...

Concrete tips:

• Just use std::list<sf::Sprite> or another container. No new and delete.
• Don't declare variables before you need them, there's no reason for  event and bulletIterator to be outside the loop. Declare iterators directly in the for loop they are used.
• As far as I see, bulletSpritePtr is completely useless.
• The header <iostream> is unnecessary, too.
• Prefer pre-increment (++bulletIterator) over post-increment (bulletIterator++)
• You needn't check for empty(), since empty containers always have begin() == end().

[TheEnigmist]

I will never understand why so many people consider it a good idea to keep owning pointers in STL containers. More...

Concrete tips:

• Just use std::list<sf::Sprite> or another container. No new and delete.
• Don't declare variables before you need them, there's no reason for  event and bulletIterator to be outside the loop. Declare iterators directly in the for loop they are used.
• As far as I see, bulletSpritePtr is completely useless.
• The header <iostream> is unnecessary, too.
• Prefer pre-increment (++bulletIterator) over post-increment (bulletIterator++)
• You needn't check for empty(), since empty containers always have begin() == end().

Uhm i will try to change all code following your tips :)
PS: i've just compiled new SFML 2.0 and now i don't know how can i manage FPS, i see that GetFrameTime is no more a Window member function.

[tobybear]

PS: i've just compiled new SFML 2.0 and now i don't know how can i manage FPS, i see that GetFrameTime is no more a Window member function.

Check here:
http://sfml-dev.org/forum/viewtopic.php?t=6831

[Nexus]

Please use the forum instead of private messages, so that other people are also able to help or benefit from solutions.

I'm changing that example code following your tips, but i'm getting mad in my for cicle for move sprites.

i tried these: [...]

Code: [Select]

for (std::list<sf::Sprite>::const_iterator bulletIterator = bulletList.begin(); bulletIterator != bulletList.end(); ++bulletIterator){
bulletIterator->Move(speed * FPS.GetElapsedTime() /1000.f, 0.f);
}

All doesn't work Why?

And describe the problem better, "doesn't work" is close to useless. The shown approach is good, except that you can't modify the object via const_iterator.

But it looks like you're lacking some basic knowledge about the STL, I recommend to read the corresponding chapters in a good C++ book like the C++ primer. These are very important topics, you are going to work with the standard library again and again.

[TheEnigmist]

Please use the forum instead of private messages, so that other people are also able to help or benefit from solutions.

I'm changing that example code following your tips, but i'm getting mad in my for cicle for move sprites.

i tried these: [...]

Code: [Select]

for (std::list<sf::Sprite>::const_iterator bulletIterator = bulletList.begin(); bulletIterator != bulletList.end(); ++bulletIterator){
bulletIterator->Move(speed * FPS.GetElapsedTime() /1000.f, 0.f);
}

All doesn't work Why?

And describe the problem better, "doesn't work" is close to useless. The shown approach is good, except that you can't modify the object via const_iterator.

But it looks like you're lacking some basic knowledge about the STL, I recommend to read the corresponding chapters in a good C++ book like the C++ primer. These are very important topics, you are going to work with the standard library again and again.

Sorry
Yeah, i need to read more about STL, the problem is that my teacher didn't spend much time on STL
I've solved the problem. It was the const_iterator. I'm so stupid :lol:
Right now i've Deitel&Deitel C++ book that i've used, is it good or i can burn it? :lol:

Working code if can help:

Code: [Select]

for (std::list<sf::Sprite>::iterator bulletIterator = bulletList.begin(); bulletIterator != bulletList.end(); ++bulletIterator){
bulletIterator->Move(speed * movement, 0.f);
}
movement is

Code: [Select]

float movement = clock.Restart().AsSeconds();


Thx for all your tips, hope can help to make me a better programmer :lol:


EDIT: nexus i changed the code following your tips, now it works but i want to know if you could give me more tips or this little code is "well-written"

Code: [Select]


#include <SFML/Graphics.hpp>
#include <list>

int main()
{
sf::RenderWindow App(sf::VideoMode(800,600,32),"SFML AnimTest",sf::Style::Titlebar | sf::Style::Close);
App.SetFramerateLimit(300);


sf::Texture bulletTexture;
bulletTexture.LoadFromFile("data/img/fire.png");

sf::Sprite bulletSprite(bulletTexture);

std::list<sf::Sprite> bulletList;

float speed = 400.0f;

sf::Clock clock;
while(App.IsOpen()){

float deltaMove = clock.Restart().AsSeconds();
sf::Event event;
if (App.PollEvent(event))
{
if (event.Type == sf::Event::Closed || (event.Type == sf::Event::KeyPressed && event.Key.Code == sf::Keyboard::Escape)){
App.Close();
break;
}
if(event.Type == sf::Event::MouseButtonPressed && event.MouseButton.Button == sf::Mouse::Left){
bulletSprite.SetPosition(sf::Mouse::GetPosition(App).x,sf::Mouse::GetPosition(App).y);
bulletList.push_back(bulletSprite);
}
}


//MOVE BULLETS AND DELETE
for (std::list<sf::Sprite>::iterator bulletIterator = bulletList.begin(); bulletIterator != bulletList.end();){
if(bulletIterator->GetPosition().x>App.GetWidth()/2)
bulletIterator=bulletList.erase(bulletIterator);
else{
bulletIterator->Move(speed * deltaMove, 0.f);
++bulletIterator;
}

}

App.Clear(sf::Color::Black);

for(std::list<sf::Sprite>::const_iterator bulletIterator = bulletList.begin(); bulletIterator != bulletList.end(); ++bulletIterator)
App.Draw(*bulletIterator);
App.Display();
}
return EXIT_SUCCESS;
}