Thumper's resource management refactor.

[Thumperrr]

There now exists a GenericResourceManager class that can be declared with different template parameters to manage different types of resources.

Fixes #67, Fixes #69, will cause merge conflicts with #71

See also #72 which is an alternative to this PR.

Discuss.

[LB--]

You've got comments to document the code but they're the wrong style for doxygen ;)

Also, some code is not indented properly.

[LB--]

These are global variables(!), and they are also defined in the header instead of the cpp file.

[Thumperrr]

We could declare them as static if it helps.
Having a static global variable is exactly the same as a singleton.

The point is to declare them once (and only once) and allocate the resources once (and only once).
The only way I can think to do that is through a global variable/singleton.

[Thumperrr]

they're the wrong style for doxygen

I didn't comment fully because I figured it would be reviewed first.

[LB--]

This looks overly complex: we've got multiple manager classes, virtual member functions, deleters, multiple manager instances, additional classes for the loading the resources, etc.

Compare it to #72 which has one manager class & one instance, and there are just different kinds of resources that it can manage. No deleter stuff, no excess classes, no virtual member functions.

[bjumbeck]

I know I haven't been active on this project, but just wanted to voice my opinion here since we kind of discussed it over the IRC channel a few days ago.

I personally prefer this manor of resource manager where you have a single generic template which you can use for all your resources. It is quite easy then to just define a manager for each type of resource which in my opinion is a good thing as it keeps them separated and makes managing them easier. Like when you have a texture, sound buffer and shader all for a single fireball and everyone is named fireball.xxx.

Most of the time you will have multiple cases where this is true and if they all reside in a single manager it opens the door for easy bugs that could be hard to find.

So multiple instances of a manager for each resource type is a plus in my eyes instead of a con.

Though I will agree with LB that there is a few parts of the implementation where I think it is a bit overboard for what the class is made to do and could be simplified (Everything should be in a single generic class in my opinion and there should be no virtual functions since you won't be inheriting from it). But overall I like the design of it.

Though as I told LB this is probably just because I am used to doing it in a similar manner and it is what I am most used to. Either way I think both managers are good manager and each have it's pros and cons.

Though if I could give a little suggestion for both designs and this is just my opinion. I think both of the designs could be simplified a bit. Resource managers are very simple in what they do and their implementations should reflect that in my opinion.

For example a resource mangager has only a very few tasks.

1. It should be able to load a resource when requested to. This should only happen usually in stages (Like the beginning of a level, or a new game state for example) and not whenever the resource is needed. It should be up to the programmer to choose when everything is loaded and when it is released, all the manager needs to do is hold that resource until that happens. The only reason I pointed this out is because a lot of people go overboard and design managers that can load on demand, automatically free resources, ect. which is in my opinion more trouble then it is worth and causes way to many bugs.
2. It should give a resource whenever it is requested.
3. It should be able to free a resource(s) when requested to.

And that is it (With exceptions of a few helper methods). So something simple with a interface like this should be more then enough.

void loadResource(/* Whatever You use for ID's and a filepath to the resource */);
// Whatever overloads you need.

Resource& getResource(/*Whatever ID you use */);

void freeResource(/* Again ID */);

One helper function I do like is one that allows you to set the path to where your resources are stored that way you don't have to type long resource names while loading.

But anyways again this is just my opinion and since I am not working on the project it means little ;p so choose whatever works best for you guys.

[LB--]

There's no reason to have both loadResource and getResource because to load a resource you can just ignore the return value of getResource. It's also trivial to add freeResouce to either design should we need it.

I personally prefer this manor of resource manager where you have a single generic template which you can use for all your resources.

That's what mine is, except instead of templating the class I template the getter. What good is a resource manager which only manages one kind of resource? If you have dozens of types of resources, you need dozens of managers, and you have to modify code in several locations whenever you introduce a new type of resource.

[bjumbeck]

There's no reason to have both loadResource and getResource because to load a resource you can just ignore the return value of getResource. It's also trivial to add freeResouce to either design should we need it.

Sure there is it adds clarity. I would much rather have clarity in a program then to save space by not having a extra method. The compiler could care less either way. Clarity helps people with writing code and especially with reading it since code is always 10 times harder to read then write.

I tend to try to stick to the methodology of a function should do only one thing and that is it. If you have functions that try and do multiple things it just complicates the design in my opinion. Like using getResource() as a way to load resources and also retrieve them.

Most people if they saw code like

textureHolder.getResource("Fireball.png");
textureHolder.getResource("Player.png");
...

sf::Sprite player(textureHolder.getResource("Player.png"));
sf::Sprite fireball(textureHolder.getResource("Fireball.png"));

Would wonder what the heck is going on because they seem to be requesting a resource twice (Once without doing anything with it it seems). They then might assume the first calls to be a error and delete them, or they just dig into the resource manager class and try and figure out what is going on. Either way it takes time away and causes confusion in my opinion.

Whereas when creating a method for each task

// I like using enums as ID's though most people hate it ;p
textureHolder.loadResource(Textures::Fireball, "Fireball.png");
textureHolder.loadResource(Textures::Player, "Player.png");
...

sf::Sprite player(textureHolder.getResource(Textures::Player));
sf::Sprite fireball(textureHolder.getResource(Textures::Fireball));

it clearly shows what it is doing to the coder and he doesn't need to guess or dig into the code to find out.

Again this is all just my opinion and just how I generally tend to go about things and I realize everyone is different.

What good is a resource manager which only manages one kind of resource? If you have dozens of types of resources, you need dozens of managers, and you have to modify code in several locations whenever you introduce a new type of resource.

I personally prefer having multiple managers and specifically in 2D games you won't get anywhere near half a dozen types of resources that require a manager usually. The most you usually see if 3 (Texture, Font, SoundBuffer and Shader) maybe 4.

But I think I will leave you guys to deciding on this. Good luck with the project.