Higher Performance Sprite Container

[masskiller]

I've just finished testing and debugging my container and I thought it nice to share the code in case anyone finds a use for it.

It's a container of vertices with their respective transformations, it's use is set to work with a sole texture and maximize performance in draw calls, cutting in half the time taken. It also has a good amount of shortcut functions that could be used at will and need. The link is here:

https://github.com/SFML/SFML/wiki/Source:-High-Performance-Sprite-Container

[FRex]

AltSpriteHolder::~AltSpriteHolder()
{
    VertexHolder.clear();
    PositionHolder.clear();
    ScaleHolder.clear();
    AngleHolder.clear();
    TexRectHolder.clear();
}

Why?
Also, #define in c++? Really?

[eXpl0it3r]

As FRex pointed out, manually clearing containers is quite useless, since the STL containers are RAII objects and release allocated memory automagically when running out of scope, thus you could completely discard the destructor.

Although you might have intended to put the code under public domain and thus didn't specify any license, it might still be nicer to mention what license you put your code under.

You should additionally use a consistent style, either use #pragma once, #ifndef or both at the same time, but don't use once the one method and for something else the other method.
As for the first lines, including headers before the #pragma check defeats the whole purpose.
I would be probably nice to split the different header & source files into separated code boxes.
To better manage the code you could additionally create a GitHub repository.

[masskiller]

The Polar Vector class was programmed when I was just learning, and I never modified it so there's a clear difference between styles. I just copied the code as it was and it turns out some ghosts of my bad coding past got out.

Respecting the destructor I do it as a habit. I know I don't have to clear them but I still do just because I've always done it. I probably have to change it since it's not that necessary, but it's not bad either.

Although you might have intended to put the code under public domain and thus didn't specify any license, it might still be nicer to mention what license you put your code under.

This I forgot, thanks for pointing it out.

Edit: Just fixed everything, I hadn't checked that code in a long time, so that's to be expected I guess.

[Nexus]

Good that you didn't use new and delete! Some other points:

• The == and != operators needn't be friends.
• Implement operator!= as !(left == right) instead of duplicating code.
  
• Top-level const qualifiers at parameters like const unsigned amount are useless and misleading (the caller's argument is copied anyway).
  
• The 2 constructors of AltSpriteHolder have a very similar body, you could outsource that in a function.
  
• Copy constructor and assignment operator are flawed. Make the class non-copyable if you don't want copies. But providing copy ctor and op= without implementing them is a bad idea.
• You don't need to declare a destructor for AltSpriteHolder if it is empty.
  
• You could directly use VertexHolder[I++].texCoords = ...
  
• The variables are named inconsistently.

They are not all very important, but I thought I'd mention them anyway. If you don't adapt the Wiki, you know for the future

[masskiller]

Copy constructor and assignment operator are flawed. Make the class non-copyable if you don't want copies. But providing copy ctor and op= without implementing them is a bad idea.

Thanks I forgot that. They were added by the IDE, but at some point I just forgot they were there and didn't even bother to program them when I do need them to have a copy constructor and assignment operator.

You could directly use VertexHolder[I++].texCoords = ...

I removed this while debugging in order to be able to make console printing of the data to make sure they were being copied or assigned correctly (oddly enough, most of what went wrong was there). When I was using pos-increment I always had the problem of not being able to check the first vertex of the quad (the first time the function was being run) was correctly assigned or not, so I switched to pre-increment and left it that way, but there should be no reason for it now.

The == and != operators needn't be friends.

Why not? The static value they are using for float comparison is private, so to my understanding they need to be friends of the class to use it. I could make it public of course, but that would only mean that it'd be like a needless global variable.

This specific EPSILON could be used for other classes, but it would leave a naming inconsistency (which is already enough as it is with my bad namings). And there's also the benefit of being able to change how much precision you want to use for the class's (and only that class) comparison by changing the value without affecting other possible classes that use this EPSILON in case it where either a public static or a purely global variable.

Not to mention that the only globals I have are there out of necessity for some calculations so that the code hasn't any other dependencies, adding another one would be dirty in my opinion since that part should be changed in anything serious by either using thor's math module (not used to lessen the amount of dependencies) or a self made file used specifically to store all necessary calculation globals such as Pi and any other mathematic constant needed. I just thought it unnecessary to add such a file when the programmer may already have his own.

The variables are named inconsistently.

I guess this will be my forever flaw, I've always sucked at thinking in good and expressive names. 

They're all overall good suggestions. I'll take them into account and assimilate them into my coding. There's always something to improve. 

[Nexus]

Why not? The static value they are using for float comparison is private, so to my understanding they need to be friends of the class to use it.

Ah right, I overlooked that. Let's correct my statement: operator!= needn't be friend.

You could declare EPSILON only globally only in the .cpp file, this way you also remove implementation details from the header, and the operators don't need friend access. But if you want this value to be specific to that class, you can also leave it static.