Issues with Layers

[tundra010]

Hello! This is my first post on SFML forums.

Recently, I have started a project in which I need a layer class, to group many drawables into one drawable. I have searched the forums, and I found some example code, which I tried to modify for my needs. However, I get strange compiler errors. Here is the code, first, then the errors:

Layer.h:

Code: [Select]


#include <SFML/Graphics.hpp>

class Layer : public sf::Drawable
{
public:
    void AddDrawable(sf::Shape d);
protected:
    typedef std::vector<sf::Shape> DrawableList;
    virtual void Render(sf::RenderTarget &target);
private:
    DrawableList myDrawables;
};



Layer.cpp:

Code: [Select]


#include "Layer.h"

void Layer::AddDrawable(sf::Shape d)
{
    myDrawables.push_back(d);
}

void Layer::Render(sf::RenderTarget &target)
{
    for(unsigned int f = 0; f < myDrawables.size(); f++)
    {
        target.Draw(*myDrawables[f]);
    }
}



Trying to use Layer:

Code: [Select]


Layer a;
a.AddDrawable(sf::Shape::Rectangle(10, 10, 40, 40, sf::Color(255, 200, 0), 5, sf::Color(0, 255, 255)));
a.SetPosition(60, 50);
App.Draw(a);


The compiler errors I get are
-error: cannot declare variable 'a' to be of abstract type 'Layer'
-error: no matching function for call to 'Layer::AddDrawable(sf::Shape)'

Any help would be greatly appreciated!
Thank you!

[Groogy]

First error is because the virtual method haven't been defined correctly. The method has to be constant. Second error, I think it is because of the object couldn't be created. But anyway, how you have defined it right now, you are copying shapes and then adding them to the vector resulting in two copies... When I think you don't want to copy it at all.

[tundra010]

Thank you for replying!
The first error is fixed by adding "const" at the end, but how do I fix the second error? How can I not copy the object?

[Groogy]

Phuu it's a tricky one. Simplest is newing them and adding pointers to the layer instead. But in the far run that will result in problems that you can do without.

You can add a second layer of abstraction for the creation of any drawable that will be added to the layer. Heard of factories? Using factories you could solve it or a simple creation method in the layer so something like:

Code: [Select]

Shape &shape = layer.CreateShape(/* values */);
Though the problem here comes to that you are using a vector, when it needs to exand the reference to the shape object would be invalid.

Example of the problem:

Code: [Select]

Shape &shape1 = layer.CreateShape(/*values*/);
Shape &shape2 = layer.CreateShape(/*values*/);
shape1.SetPosition(10,10); // CRASH!

There are solutions to this though, but it's no fun if I give you the whole thing After you've tried yourself and still can't solve it, I'll help you out.

[tundra010]

What if I do something like this:

Code: [Select]

layer.CreateShape(/*Parameters*/);

This would create a shape inside the vector in Layer, but not return the pointer, and CreateShape() would be void. Then, I could access the shape by

Code: [Select]

layer.GetVector()[0]

Or something similar to that. Is that a good idea?

[Groogy]

Well yeah you can, much simpler way to do it.

But it breaks encapsulation of layer giving you access to internals of it which is something you want to avoid, the more and more objects depend on the internals of another object, the harder it will get to work with in the long run.

But just go with your idea for now, when in doubt apply the KISS(Keep-It-Simple-Stupid) principle

[tundra010]

Thanks for the help!
But, could you show me your solution?

[Groogy]

Code: [Select]


sf::Shape &Layer::CreateShape( /* arguments */ )
{
        sf::Shape *shape = new sf::Shape( /* arguments */ );
        myList.push_back(shape);
        return *shape;
}

Layer::~Layer()
{
        // Pseudo code
        FOR_EACH(shape in myList)
        {
                 delete shape;
        }
        myList.clear();
}


This is a well working solution for when working with std::vector and also keeps the code clean from any ugly pointers. This also solves the problem with a reference to a shape would be made invalid later.

Though you have to remember, any method returning a reference, the receiving variable has to be declared as a reference or else the variable will just be copied and not referenced.

Code: [Select]


sf::Shape copyShape = layer.CreateShape(); // Copy
sf::Shape &refShape = layer.CreateShape(); // Reference

[tundra010]

Thanks for the help!

[Contadotempo]

Sorry for the thread hijacking, but I think this is a quick question and since it's about layers I thought I could ask here.
On the layer class provided on the wiki [here], what does this constructor do?

Code: [Select]

Layer::Layer(const sf::Vector2f& Position, const sf::Vector2f& Scale, float Rotation, const sf::Color& Col) : Drawable(Position,Scale,Rotation,Col)

I can't understand the need of it since I'm able to do the same thing with the default constructor (I think). If it's not too troublesome could anyone give me hint?
Thanks in advance  

[Groogy]

It let's you set the starting position, starting scale, starting rotation and starting color by simply passing it on the the constructor in the class Drawable that it inherits from.

http://www.sfml-dev.org/documentation/1.6/classsf_1_1Drawable.htm#712de01505e04a03b93cf33ae004cdbc