Removing units individually by clicking on them.

[Bogdan]

What I would like to do, is to remove units individually by right-clicking on the units concerned.(SFML 2.1+Visual Studio C++ 2010 on Win 7)
I can't get it working (creating units is no problem), because of those two if lines (marked with a comment), placed near the bottom of the code.
If I go over it, it says "Error, Expression has to be of class type". Taking away the round braces in the line, produces another type of error....Any hints on how to do it right? Many thanks!

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


int main()
{
        sf::RenderWindow mMainWindow(sf::VideoMode(1000, 600), "Map");
        mMainWindow.setFramerateLimit(30);
       
        sf::RectangleShape rectangle;
        rectangle.setSize(sf::Vector2f(20, 20));
        rectangle.setFillColor(sf::Color(255,255,255,255));
        rectangle.setPosition(50, 50);


        sf::Image mapimage;
        mapimage.loadFromFile("map.png");
        sf::Texture maptexture;
        maptexture.loadFromImage(mapimage);
        sf::Sprite mapsprite(maptexture);
        mapsprite.setPosition(0, 0);

        sf::Image unitimage;
        unitimage.loadFromFile("unit.png");
        sf::Texture unittexture;
        unittexture.loadFromImage(unitimage);
        sf::Sprite unitsprite(unittexture);

        sf::RectangleShape gamelayer;
        gamelayer.setSize(sf::Vector2f(1000, 600));
        gamelayer.setFillColor(sf::Color(255,255,0,100));
        gamelayer.setPosition(0, 0);

        std::list<sf::Sprite>EnemyList;
        std::list<sf::Sprite>::iterator EnemyIt;


while (mMainWindow.isOpen())
        {
                sf::Event event;
                while (mMainWindow.pollEvent(event))
                        {
                        if (event.type == sf::Event::Closed)
                                {
                                mMainWindow.close();
                                }
                        if(event.type == sf::Event::MouseButtonPressed)
                                        if(event.mouseButton.button == sf::Mouse::Left)
                                                {
                                                sf::Vector2f mousecoords(mMainWindow.mapPixelToCoords(sf::Vector2i(event.mouseButton.x, event.mouseButton.y)));
                                                        if(gamelayer.getGlobalBounds().contains(mousecoords))
                                                        {
                                                        sf::Vector2i pixelPos = sf::Mouse::getPosition(mMainWindow);
                                                        std::cout << "x" << sf::Mouse::getPosition(mMainWindow).x << std::endl;
                                                        std::cout << "y" << sf::Mouse::getPosition(mMainWindow).y << std::endl;
                                                        sf::Sprite *Sprite;
                                                        Sprite = new sf::Sprite;
                                                        Sprite->setTexture(unittexture);
                                                        Sprite->setPosition(sf::Mouse::getPosition(mMainWindow).x,sf::Mouse::getPosition(mMainWindow).y);

                                                        EnemyList.push_back(*Sprite);
                                                        std::cout << "Objects in the list: " << EnemyList.size() << std::endl;
                                                        }
                                                }
                                        if(event.mouseButton.button == sf::Mouse::Right)
                                                {
                                                sf::Vector2f mousecoords(mMainWindow.mapPixelToCoords(sf::Vector2i(event.mouseButton.x, event.mouseButton.y)));
                                                        if(EnemyIt->getPosition().x.contains(mousecoords))           // these both lines don't work
                                                                if(EnemyIt->getPosition().y.conatins(mousecoords))
                                                                        {
                                                                        EnemyList.erase(EnemyIt);
                                                                        EnemyIt = EnemyList.begin();
                                                                        }
                                                }
                                               
                        }



        mMainWindow.draw(gamelayer);
        mMainWindow.draw(mapsprite);

        for(EnemyIt = EnemyList.begin();EnemyIt != EnemyList.end();EnemyIt++)
        {
                mMainWindow.draw(*EnemyIt);

        }


        mMainWindow.draw(unitsprite);
        mMainWindow.draw(rectangle);
        mMainWindow.display();
        mMainWindow.clear();

        }

        return 0;
}

[Ixrec]

"Error, Expression has to be of class type" means you're trying to do something on a non-class that only classes can do.  Did you try googling this error before posting here?

Specifically, look at this portion of the expression:

EnemyIt->getPosition().x

What do you think this returns?  ie, what do you think x is?  Is it a class?  Is it a number?  Does it have a contains() method?



Incidentally, EnemyIt->getPosition() is a point (well, a Vector2f).  mousecoords is also a point.  It doesn't make any sense to ask if a point contains another point.  I'm guessing what you really want is to check if mousecoords is within the bounding box of the sprite, in which case getLocalBounds() or getGlobalBounds() might help.

[Bogdan]

Ok, I've removed those two if-lines and put the following line instead of them (this now gets compiled without errors and "game" starts without difficulties):

if(EnemyIt->getGlobalBounds().contains(mousecoords))

And now comes another problem, I couldn't find any adequate solution for.
Even after commenting out these statements
                           EnemyList.erase(EnemyIt);
                           EnemyIt = EnemyList.begin();

there is still this following and new error, while running the program and right clicking:
"Expression: list iterator not derefencable"

Maybe it's something wrong with the for loop? But what?

[Strelok]

If you want to erase any sprite that contains your cursor while right clicking you must cycle through the enemies, possibly using http://en.cppreference.com/w/cpp/language/range-for . You're testing a list iterator when what you probably want to test is a reference to a sprite.
You're asking for the cursor's position many times for no reason, are you sure mousecoords.x and mousecoords.y are not what you need?

[Bogdan]

As for these three lines:

                                                        sf::Vector2i pixelPos = sf::Mouse::getPosition(mMainWindow);
                                                        std::cout << "x" << sf::Mouse::getPosition(mMainWindow).x << std::endl;
                                                        std::cout << "y" << sf::Mouse::getPosition(mMainWindow).y << std::endl;

You're right, that they're not necessary. As for the "range for" I'm still trying to figure out, what to write in it. I know about such loops from theory/books, but can't fill them with real content in this special case. However I know, how to use them with arrays.

for(base_type&  variable : container)   //Reference  (& ampersand symbol)
     statement

 int my_array[10];
 for(int& i : my_array)
     i = 0

Should it look like this?:

      

for (int &unitsprite: EnemyList)       

   -->still errors: unitsprite requires an Initaliser   (int EnemyIt; doesnt help here)   

[eXpl0it3r]

there is still this following and new error, while running the program and right clicking:
"Expression: list iterator not derefencable"

You never set EnemyIt to anything, how do you expect it to point to a valid object?

[Bogdan]

Corrected it to point to unitsprite

[eXpl0it3r]

And why would you do that? I mean why do you make it so an iterator points to a single object, while instead you could just point to that object to begin with?
Do you even understand what iterators are made for?

[Bogdan]

I've got it to work correctly, but how to resolve the error, that occurs, whenever I try to remove the last unit on the map. ---> (Object list becomes empty; error: list iterator not incrementable)

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

int main()
{
        sf::RenderWindow mMainWindow(sf::VideoMode(1000, 600), "Map");
        mMainWindow.setFramerateLimit(30);
       
        sf::Image mapimage;
        mapimage.loadFromFile("map.png");
        sf::Texture maptexture;
        maptexture.loadFromImage(mapimage);
        sf::Sprite mapsprite(maptexture);
        mapsprite.setPosition(0, 0);

        sf::Image unitimage;
        unitimage.loadFromFile("unit.png");
        sf::Texture unittexture;
        unittexture.loadFromImage(unitimage);
        sf::Sprite unitsprite(unittexture);

        sf::RectangleShape gamelayer;
        gamelayer.setSize(sf::Vector2f(1000, 600));
        gamelayer.setFillColor(sf::Color(255,255,0,100));
        gamelayer.setPosition(0, 0);

        std::list<sf::Sprite>EnemyList;
        std::list<sf::Sprite>::iterator EnemyIt;

while (mMainWindow.isOpen())
        {
                sf::Event event;
                while (mMainWindow.pollEvent(event))
                        {
                        if (event.type == sf::Event::Closed)
                                {
                                mMainWindow.close();
                                }
                        if(event.type == sf::Event::MouseButtonPressed)
                                if(event.mouseButton.button == sf::Mouse::Left)
                                        {
                                        sf::Sprite *Sprite;
                                        Sprite = new sf::Sprite;
                                        Sprite->setTexture(unittexture);
                                        Sprite->setPosition(sf::Mouse::getPosition(mMainWindow).x,sf::Mouse::getPosition(mMainWindow).y);

                                        EnemyList.push_back(*Sprite);
                                        std::cout << "Objects in the list: " << EnemyList.size() << std::endl;
                                        }                      
                        }



        mMainWindow.draw(gamelayer);
        mMainWindow.draw(mapsprite);

        for(EnemyIt = EnemyList.begin();EnemyIt != EnemyList.end();EnemyIt++)
                {
                mMainWindow.draw(*EnemyIt);
                        if(event.mouseButton.button == sf::Mouse::Right)
                                {
                                sf::Vector2f mousecoords(mMainWindow.mapPixelToCoords(sf::Vector2i(event.mouseButton.x, event.mouseButton.y)));
                                if(EnemyIt->getGlobalBounds().contains(mousecoords))
                                        {
                                        EnemyList.erase(EnemyIt);
                                        EnemyIt = EnemyList.begin();
                                        }
                                }
                }
                mMainWindow.display();
                mMainWindow.clear();

        }
        return 0;
}

[Jesper Juhl]

Instead of doing

                    EnemyList.erase(EnemyIt);
                    EnemyIt = EnemyList.begin();

Use the return value from erase.

[Bogdan]

Sorry, I don't understand what you mean.
Return value = Iterator following the last removed element?
Doesn't it follow the last removed element automatically already? Do I have to create another two iterators to set the "limits"?

[Nexus]

Use list::remove_if(). Or for other containers, use the erase-remove idiom.

Why do you even use std::list? Unless you have specific reasons, std::vector should be the default STL container.

And don't mix drawing, input (mouse click) and logic (element removal). The corresponding functionality should be in three separate places... At the very least, graphics should be kept separate.

[Jesper Juhl]

Sorry, I don't understand what you mean.

I meant this:

Code: [Select]

EnemyIt = EnemyList.erase(EnemyIt);

[Bogdan]

Replacing those two lines mentioned earlier with this, still produces the same error.
I know that I'm getting on your nervers (sorry for that), but I need  it to get finished successfully (this time only) for keeping my motivation high :-), before going further to better concepts and styles.

EnemyIt = EnemyList.erase(EnemyIt);

As for the remove_if, at the moment its very difficult for me to understand this concept.
Whatever I try, it throws me some errors, because I don't know how to "combine" it with sfml.

[Strelok]

This

                if(EnemyIt->getGlobalBounds().contains(mousecoords))
                    {
                    EnemyList.erase(EnemyIt);
                    EnemyIt = EnemyList.begin();
                    }
                }

becomes

EnemyList.remove_if([](sf::Sprite sprite){return sprite.getGlobalBounds().contains(mousecoords); });
 

If it's difficult to understand this, you should stop coding and read a book. It will save you a lot of trial and error