-
Hi... I am making this space game. Everything's going well so far. I managed to draw the player, the lasers and the enemies. I set the collisions such that when the laser hits the enemy, the laser and the enemy are both erased (lasers and enemy are both declared as sprite lists). The collision works... But only for the first 3 or 4 enemies. After that, the lasers don't affect the enemies. Is there anything wrong in the code? I failed to find any mistake. Here's my code:
if(!enemytex.loadFromFile("images/enemy.png"))
return EXIT_FAILURE;
std::list<sf::Sprite> enemy(MAX, sf::Sprite(enemytex));
for(std::list<sf::Sprite>::iterator enemyit = enemy.begin(); enemyit != enemy.end(); enemyit++)
{
enemyit->setOrigin(enemyit->getGlobalBounds().width/2, enemyit->getGlobalBounds().height/2);
}
std::list<sf::Sprite>::iterator enemyit = enemy.begin();
int dist = (int)(resolution.getDesktopMode().width - enemyit->getGlobalBounds().width);
int wastage = (int)enemyit->getGlobalBounds().width/2;
srand((unsigned int)time(NULL));
float y = -200;
for(std::list<sf::Sprite>::iterator enemyit = enemy.begin(); enemyit != enemy.end(); enemyit++)
{
int x = rand() % dist + wastage;
enemyit->setPosition((float)x, y);
y = y - enemyit->getGlobalBounds().height * 2;
}
if(!greenlasertex.loadFromFile("images/playerlaser.png"))
return EXIT_FAILURE;
std::list<sf::Sprite> greenlaser;
float enemyspeed = 100.f;
float playerspeed = 400.f;
float laserspeed = 800.f;
sf::Clock clock;
long double score = 0;
long double lvlnum = 1;
int shots = 0;
while(window.isOpen())
{
float timer = clock.restart().asSeconds();
window.clear();
for(std::list<sf::Sprite>::iterator enemyit = enemy.begin(); enemyit != enemy.end(); enemyit++)
{
enemyit->move(0.f, enemyspeed * timer);
window.draw(*enemyit);
}
if(sf::Keyboard::isKeyPressed(sf::Keyboard::Space))
{
shots++;
}
if((shots != 0) && (shots % 19 == 0))
{
sf::Sprite newgreenlaser(greenlasertex);
newgreenlaser.setOrigin(newgreenlaser.getGlobalBounds().width/2, newgreenlaser.getGlobalBounds().height/2);
newgreenlaser.setScale(0.1f, 0.1f);
newgreenlaser.setPosition(player.getPosition().x, player.getPosition().y);
greenlaser.push_back(newgreenlaser);
}
for(std::list<sf::Sprite>::iterator greenlaserit = greenlaser.begin(); greenlaserit != greenlaser.end(); greenlaserit++)
{
greenlaserit->move(0.f, -laserspeed * timer);
window.draw(*greenlaserit);
}
int erased = 0;
std::string str, line, word, level, lvl, number;
str = std::to_string(score);
word = "Score: ";
line = word + str;
ScoreText.setString(line);
number = std::to_string(lvlnum);
lvl = "Level ";
level = lvl + number;
LevelText.setString(level);
***INSIDE THIS BOX***
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
std::list<sf::Sprite>::iterator enemyit = enemy.begin(), next;
std::list<sf::Sprite>::iterator greenlaserit = greenlaser.begin(), reload;
while(enemyit != enemy.end())
{
next = enemyit;
next++;
while(greenlaserit != greenlaser.end())
{
reload = greenlaserit;
reload++;
if(enemyit->getGlobalBounds().intersects(greenlaserit->getGlobalBounds()))
{
enemy.erase(enemyit);
greenlaser.erase(greenlaserit);
++erased;
score = score + 10;
int diff = (int)score;
if(diff % 500 == 0)
{
enemyspeed = enemyspeed + 25.f;
lvlnum++;
number = std::to_string(lvlnum);
level = lvl + number;
LevelText.setString(level);
}
str = std::to_string(score);
line = word + str;
ScoreText.setString(line);
}
greenlaserit = reload;
}
enemyit = next;
}
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
***INSIDE THIS BOX***
for(int i = 0; i < erased; ++i)
{
sf::Sprite temp(enemytex);
temp.setOrigin(temp.getGlobalBounds().width/2, temp.getGlobalBounds().height/2);
float y = -200;
int x = rand() % dist + wastage;
temp.setPosition((float)x, y);
y = y - temp.getGlobalBounds().height * 2;
enemy.push_back(temp);
}
window.display();
}
I apologize if the code is too long. That is as minimal and complete as it can get. I wanted to show the whole code of the enemy and laser because I can't pinpoint the problem, and it is pretty big. Nevertheless, I highlighted the collision part inside the code for easier reference. Please help. Thank you.
-
This would probably be a good time to learn to use a debugger. In all likelihood if you set a breakpoint inside your collision code and just step through it while looking at all the relevant variables (especially those lists) you can probably figure it out yourself.
-
OK... Thanks. I'll do that. But, in the code above... Are there any anomalies in it not related to debugging? Any coding mistakes that causes the collision not to happen? I just want to make sure of that before I start debugging. Thanks.
-
Nothing jumped out at me, and this sounds like exactly the kind of problem I'd never be able to find without using a debugger myself, which is why I recommended that.
-
Alright... OK, I'll try that. Thanks. :)
-
In my opinion one of the best debugging options one has is to 'cout << ' any and all variables in a given method, just to make sure that method/function is doing what it should.
-
It's one option, and sometimes an indispensable one, but for the vast majority of problems it's a lot faster and easier to use a real debugger. There are also many problems that cout<< just cannot help with at all, which are extremely common when doing graphics or timing of any sort (ie pretty much anything you'd use SFML for). The only downside is you have to learn how to use the debugger first.
-
Indeed a good debugger can do much more than a simple 'cout <<' but it just saddens me that these kids nowadays just post online "MY CODE NO WORK WHY IT NOW WORK!?!?' without even checking to make sure the method/function even gets entered and its variables get changed to what they should be first.
One of the reasons I LOVE programming is it is problems solving half the time...but some of these kids nowadays... Hell, my first c++ book for a class was literally "How to be a good debugger". nothing on the actual language, just a 200 page book on how to debug shit yourself. It was the most valuable class I've ever taken.
-
Yeah, definitely. It's a little annoying that the vast majority of the questions asked here seem to come from people who clearly either haven't learned C++ at all yet (I still don't get why people do that), or can't be bothered to debug/read tutorials/check documentation.
Though it helps to remember that there's probably a huge silent majority doing all sorts of cool stuff with SFML without ever needing to ask silly questions here.
-
"MY CODE NO WORK WHY IT NO WORK!?!?"
OK, dude... I did not say that! ;D
I did do a bit of tweaking myself before posting the question.
Yeah, definitely. It's a little annoying that the vast majority of the questions asked here seem to come from people who clearly either haven't learned C++ at all yet (I still don't get why people do that), or can't be bothered to debug/read tutorials/check documentation.
I am actually very much interested to learn C++ from scratch. The only reason I'm skipping and doing all this is purely for the sake of marks. It's a college project and I'm already kind of overdue. I'm not saying this is anybody's fault. But time constraints are preventing me from learning anything properly. So, I'm just doing what I can and submit it. But on a personal level and for my future career, I will definitely learn all the A - Z of programming. It's my passion. But for the time being, please show some mercy or leniency or whatever the word is, and don't judge me. I'm not like all the morons out there. :)
Back on topic:-
I did some debugging. Found out that the "list iterator is not dereferencable". Did some tweaking with the code and still got the same error. I guess it may be in the collision code. Something to do with greenlaserit and enemyit. I'm unable to tell. Definitely somewhere in here:
std::list<sf::Sprite>::iterator enemyit = enemy.begin(), next;
std::list<sf::Sprite>::iterator greenlaserit = greenlaser.begin(), reload;
while(enemyit != enemy.end())
{
next = enemyit;
next++;
while(greenlaserit != greenlaser.end())
{
reload = greenlaserit;
reload++;
if(enemyit->getGlobalBounds().intersects(greenlaserit->getGlobalBounds()))
{
enemy.erase(enemyit);
greenlaser.erase(greenlaserit);
}
greenlaserit = reload;
}
enemyit = next;
}
-
When you get an error you don't understand always try googling it first. There's plenty of stackoverflow questions about that exact one. Basically, when you use erase() on an iterator, it becomes invalid (for obvious reasons). The good news is erase() itself returns a valid iterator for you.
-
I did try googling it. That's how I ended up with the while loop that is used to increment the iterator, instead of the generic for loop with its increment counter. But, thanks for the advice. I will search a bit deeper, see what I can find. Your reply actually helped narrow down the possibilities of errors. I'll see what I can do. I shall post again if I hit any dead ends or if I have any queries. Thanks. :)
-
OK... I am officially admitting defeat! >:(
I am not at all getting this! Is collision between two sprite lists even possible? Because, if it isn't, I am literally barking up the wrong tree. I have tried every solution I found on my search. So far, nothing. I even read a lot of material related to it. I am not able to apply it. Can someone explain to me, please.
-
of course it's possible. A list of sprites is the same as a sprite, just more of them, and sprite to sprite collision is certainly possible.
Quit trying to use techniques you don't fully understand, it only leads to these issues. My advice, access each element of the lists using objects and loop over the lists using for loops. Something nice and basic. Get that working and then you can move on to more complicated stuff.
-
How do I access list elements using objects? I googled it but could not find any thing relevant to it. Does it also use iterators to access the elements in the list. I don't want you to tell me any code or anything. Please just elaborate on that so I have an idea on how I can implement it. Please tell me this. Thank you. :)
-
I noticed your using lists, I'm not overly familiar with them as I always take preferences to vectors, however back when I was learning this stuff I would do:
for(int i = 0; i < enemyList.size(); i++)
{
sf::Sprite enemySpr = enemyList[i];
for(int j =0; j < otherList.size(); j++)
{
sf::Sprite otherSpr = otherList[j];
if(enemySpr.intersects(otherSpr))
{
...//whatever you want to do on a collision
}
}
}
Nice and simple and easy to understand. Not fast by any means, but it'll do the job of getting things to work
I should note if your going to be removing items from the lists, make sure iterate from back to front.
Edit: Formatting
-
OK... So, I tried out your method. It gave me the same error. Turns out the error had nothing to do with the type of container. More to do with incrementing the iterator. Here's the code again.
std::list<sf::Sprite>::iterator enemyit = enemy.begin(), next;
std::list<sf::Sprite>::iterator greenlaserit = greenlaser.begin(), reload;
while(enemyit != enemy.end())
{
next = enemyit;
next++;
while(greenlaserit != greenlaser.end())
{
reload = greenlaserit;
reload++;
if(enemyit->getGlobalBounds().intersects(greenlaserit->getGlobalBounds()))
{
enemy.erase(enemyit);
greenlaser.erase(greenlaserit);
}
greenlaserit = reload;
}
enemyit = next;
}
Here, there are two while loops and what I am doing is I am using the inner loop to check for collision and if collision occurs, I am erasing the enemy iterator. And after doing that, the inner loop will keep looping until the while condition stops being satisfied. It won't enter the outer loop until it finishes that. Problem is, the enemy iterator is being erased in the inner loop and I am not incrementing it until the program goes back to the outer loop. How do I do that so that the enemy iterator is incremented in the inner loop as soon as it is erased. I've tweaked around a lot moving code around inside and outside the loops, adding new variables, still nothing. Please help me. Thanks.
-
Having run into this problem in the past, my impression is that there's at least two decent ways of attacking it.
1) Use the return value of erase(). It's there precisely to help you avoid leaving your iterators invalidated. This is probably the "right" way of doing it, since it's built into the STL and probably results in cleaner code than #2.
2) Don't do the erase()s immediately. Instead make a vector of sprite iterators, have the collision check push the collided entities' iterators into that vector, then after all the collision checking is done call erase() on all the iterators in that vector.
-
Wow... Although the second method is pretty straightforward and what most people would opt for despite being a bit lengthy and a little messy, I am interested in experimenting with the first method. Thank you for that. You guys have been very helpful. I shall post again when I have any updates. Thanks! :) :) :)
-
It really depends on the container type. For std::deque and especially std::vector, using the return value of erase() in a loop is very inefficient. You should prefer the STL algorithms std::remove() or std::remove_if(), combined with the erase() overload for an iterator range.
WDR, keep variable declarations local and use for instead of while. Your code is very difficult to read, because your iterators are declared all over the place and have a far too wide scope. A typical iterator loop looks as follows, with the iterator declared inside the loop head:
for (auto itr = container.begin(); itr != container.end(); ++itr)
And I agree with Ixrec's second point: Your code will become much cleaner, safer and more efficient, if you simply set a flag in the objects that should be removed. If your objects have attributes like hitpoints, this can be implicitly achieved by a condition like "hitpoints <= 0". Then, all you need to do is a std::remove_if() or std::list::remove_if() at the end of each frame.
-
Even better then removing from the list and re-adding (well I think so), just use a flag like 'isAlive' to determine if the object should be updated or drawn. When you go to shoot another laser, just check if any in the list first are 'isAlive == false', if it is, reuse it, if there isn't any, add a new one.
-
When you go to shoot another laser, just check if any in the list first are 'isAlive == false', if it is, reuse it, if there isn't any, add a new one.
One should never be checked against 'true' or 'false'. Simply write 'if(isNotAlive)' or 'if(!isAlive)'. ;)
-
Guys... I tweaked around. I'm not sure, but is this a way about it?
bool alive = true;
bool isAlive() const
{
alive = false;
}
for(std::list<sf::Sprite>::iterator enemyit = enemy.begin(); enemyit != enemy.end(); enemyit++)
{
for(std::list<sf::Sprite>::iterator greenlaserit = greenlaser.begin(); greenlaserit != greenlaser.end(); greenlaserit++)
{
if(enemyit->getGlobalBounds().intersects(greenlaserit->getGlobalBounds()))
{
isAlive = false;
}
}
}
enemy.remove_if(!isAlive);
greenlaser.remove_if(!isAlive);
Please tell me... Did I put it right? ???
-
Did you try it?
-
Yes, I did! No collision is occurring. The lasers and enemies just pass through each other. Is there something wrong with that? Am I right?
-
What you're doing with alive and isAlive doesn't really make any sense. You need to step back and rethink that stuff completely.
Hint: The biggest error is that you're trying to store the results of all the collision tests in only one bool. You're supposed to give the entities a member so there's one bool for each of them.
Also, please try to start using a debugger so you can work out this simple stuff by yourself.
-
OK... Thanks! I'll do what you said. I'll post again if I have any more queries.