Is there anything with this design that could really make things break later on?

I hear so much bad about going this way, but I feel it's just perfect.

If I understand correctly, the resources derive from a base class that stores parameters as some name-value pairs.

I have also been thinking a lot how to associate parameters with resources so that they could be easily read from/written to files and perhaps used to automatically generate in-game menus for editing. However, storing the name literals is wasteful. For example, probably most "sprites" contain "frames" parameter. Each resource then stores this same literal for nothing. Also, searching parameters by name is slow, so the values should be cached in actual variables for run-time use.

I think I found a rather good way to overcome these issues. I only store parameters as proper typed variables. For bulk actions (serialization etc.), the resource creates a collection of generic variable objects, which act as mediators. For example:

class CSprite
{
private:
 int frames;
 std::string name;

public:
 std::vector<CVar> getParameters()
 {
  std::vector<CVar> p;

  p.push_back(CVar("name", &name));
  p.push_back(CVar("frames", &frames));

  return p;
 }
};

The generic variables (type CVar) store pointers to the actual data. To construct a resource from a datafile, the getParameters() is called to get a routing to the data. These parameters are only used temporarily. My actual implementation uses a base class CVarBase for CVar<T> and vectors of base class pointers are returned, so this was a simlified example.

I know this is controversial, but to be honest, that just makes it a lot more complicated than it needs to be.
It is probably because I'm still a novice, but I just feel adding tons of small classes do nothing more than make everything messy.

[...]

Sorry for being stubborn, but I need some serious convincing to see the faults of this.

The Sprite class, as an example, defines how a sprite is represented when being loaded into working memory, i.e. it manages sprite typical data for runtime purposes. So far so good. Now you add the possibility to load the data from a XML fragment, so that the Sprite class can construct its own instances. Okay, but we need to generate Sprite instances at runtime as well, so give it another factory method for this, too. If I have that, I could generate sprites by code and save them for later reload, so having a save method would be nice. Well, I want to support reading from and writing to files, but reading from and writing to memory would make networking more convenient, so methods for that are fine as well. Hmm, now that levels get bigger, I want to support binary data files as well as XML files. Oh wait, now that my Sprite class has this new fantastic feature, but existing files have not, I need to support a second generation of routines, and, since I want to support older engines, perhaps also saving routines. Now just putting in the render routines for OpenGL 3 for older machines and those for OpenGL 4 would complete it mostly. Support for mouse picking, because of the editor I'm planning, is a must of course. A bit of collision detection, and ...

Although exaggerated, the story above is what happens in reality. To defeat this from the very beginning, the single responsibility principle was defined. Sure, at the moment you say "I have only 3 responsibilities in my manager class, that is still maintainable", and you're right. However, this changes with time, and it always changes in the wrong direction if you not defeat it explicitly.

Notice that this does not mean that there is not something like a manager. However, such a manager would be a facade class, where clients find a concentrated API, but the work is done behind the facade by dedicated objects then, just controlled by the manager.

In the end, SRP simplifies the ability to exchange parts (for example the implementation of the resource cache), and to develop and test aspects separated (for example the versioning of resources on mass storage), and to still understand a piece of software after half a year or when being developed by another person, and to re-use it in another context as well. Its advantage is found in the mid to long run.

Propably i will go with Juliean way, due to the fact it's fastest way without redesigning current design.

But, Irlan idea is really cool. I know that i need to create own manager for each resources (i just call them loaders).

ResourceManager is intended to be a Facade just to don't have pass multiple managers between classes (yeah i know that the god objects are anti-pattern but in this case it's easier and clearer than having to store everywhere multiple managers).

Is there anything with this design that could really make things break later on?

I hear so much bad about going this way, but I feel it's just perfect.

If I understand correctly, the resources derive from a base class that stores parameters as some name-value pairs.

I have also been thinking a lot how to associate parameters with resources so that they could be easily read from/written to files and perhaps used to automatically generate in-game menus for editing. However, storing the name literals is wasteful. For example, probably most "sprites" contain "frames" parameter. Each resource then stores this same literal for nothing. Also, searching parameters by name is slow, so the values should be cached in actual variables for run-time use.

Nono, I don't store the frames parameter in Resource.

This map is defined in Resource:

std::map<std::string, Parameter> m_parameters;

I know this is controversial, but to be honest, that just makes it a lot more complicated than it needs to be.
It is probably because I'm still a novice, but I just feel adding tons of small classes do nothing more than make everything messy.

[...]

Sorry for being stubborn, but I need some serious convincing to see the faults of this.

The Sprite class, as an example, defines how a sprite is represented when being loaded into working memory, i.e. it manages sprite typical data for runtime purposes. So far so good. Now you add the possibility to load the data from a XML fragment, so that the Sprite class can construct its own instances. Okay, but we need to generate Sprite instances at runtime as well, so give it another factory method for this, too. If I have that, I could generate sprites by code and save them for later reload, so having a save method would be nice. Well, I want to support reading from and writing to files, but reading from and writing to memory would make networking more convenient, so methods for that are fine as well. Hmm, now that levels get bigger, I want to support binary data files as well as XML files. Oh wait, now that my Sprite class has this new fantastic feature, but existing files have not, I need to support a second generation of routines, and, since I want to support older engines, perhaps also saving routines. Now just putting in the render routines for OpenGL 3 for older machines and those for OpenGL 4 would complete it mostly. Support for mouse picking, because of the editor I'm planning, is a must of course. A bit of collision detection, and ...

Although exaggerated, the story above is what happens in reality. To defeat this from the very beginning, the single responsibility principle was defined. Sure, at the moment you say "I have only 3 responsibilities in my manager class, that is still maintainable", and you're right. However, this changes with time, and it always changes in the wrong direction if you not defeat it explicitly.

Notice that this does not mean that there is not something like a manager. However, such a manager would be a facade class, where clients find a concentrated API, but the work is done behind the facade by dedicated objects then, just controlled by the manager.

In the end, SRP simplifies the ability to exchange parts (for example the implementation of the resource cache), and to develop and test aspects separated (for example the versioning of resources on mass storage), and to still understand a piece of software after half a year or when being developed by another person, and to re-use it in another context as well. Its advantage is found in the mid to long run.

This is probably going to sound stupid, but I'm going to ask specific questions so I can get some confusion cleared up:

1. Are you saying the ResourceManager should call a ResourceLoader? If so, is ResourceLoader split into "XMLResourceLoader" and "BinaryResourceLoader"?

Again; if so, why not just have a "loadXML()" and "loadBinary()" functions inside ResourceLoader? Is that what the bad thing is?

2. Should ResourceManager have all the ResourceContainers (which then contain Resource objects)?

Should ResourceManager even call ResourceLoader?

3. Do you mean that the Sprite class should not have functions to write/read buffers? If so, do you mean there should be a SpriteSerializer class?

Also, this post was a bit longer, but I realised at least one of your points while writing it. I'm still curious what your reply is though.

Nono, I don't store the frames parameter in Resource.

This map is defined in Resource:

std::map m_parameters;

Then each class that derive from Resource, needs to implement an initialise() function which has to at least do one thing to work with the ResourceManager: set the "class" to the class name. Read my other reply again.



Either way; I disagree that it's wasteful, because it's not like you'll have thousands of resources loaded at once. Even then, it's not that bad. We're talking some kilobytes in total.

I'll agree there probably is a better way to do it though.

Ok, replying to this goes somewhat off topic, but this is very timely for me.

So you do store the parameters as name-value pairs like I suggested. It's a map. I guess that the Parameter class stores some union of admissible parameter types? Anyway, if you don't cache the values to some C++ types, each time you request a parameter from a resource you will need to make O(logN) lookup of the map, performing string comparisons. This may be neglible in some simple 2D game with a few resources, but I would not use it for rendering complex 3D scenes with hundreds of resources (with multitude of parameters) accessed each frame.

Storing the names may not take more than a few kilobytes, but usually such degeneracy suggests that the design could be improved.

First of, what we discuss here is the OOP way of doing things, and that in a manner with respect to the mid to long run. It is just an approach to defeat problems that may occur in the future when software should be extended or reworked in some way. There is no constraint to follow it absolutely; software worked also before OOP was invented. In the end software development ever suffers from demand for high quality but low effort. But without being sensitive to the danger of what may happen when software grows, bad things will happen.

EDIT

I should perhaps highlight that XML and binary, as long as used by me, is meant not as generic but as specific file format, i.e. "my game's specialized XML file format" and "my game's specialized binary file format".

/EDIT

1. Are you saying the ResourceManager should call a ResourceLoader? If so, is ResourceLoader split into "XMLResourceLoader" and "BinaryResourceLoader"?
Again; if so, why not just have a "loadXML()" and "loadBinary()" functions inside ResourceLoader? Is that what the bad thing is?

Yes, I mean that if the ResourceManager object has determined that a specific resource requested by a client is not available from the cache and hence need to be loaded, it delegates the loading to a ResourceLoader object.

How the ResourceLoader class works is another question. It could be an abstract base class, and XMLResourceLoader and BinaryResourceLoader are classes derived by inheriting ResourceLoader. So ResourceLoader provides the common API used by ResourceManager, and XMLResourceLoader and BinaryResourceLoader each one implement the interface, regarding of loading from an XML or binary source, respectively.

Is the ResourceManager free to make the distinction whether to use the one or other subtype of ResourceLoader? No, because the format of the data source defines which loader type is to be used, because one cannot use e.g. BinaryResourceLoader to read from an XML source. If ResourceLoader would publicly provide loadXML() and loadBinary(), the choice of what to use would be externalized, although one and only one of them will work anyway. But if ResourceLoader (i.e. the base class) provides a single abstract load() function, the ResourceManager has no need to know whether the source is XML or binary (or whatever else); it just invokes load() on the concretized ResourceLoader that is available to load the resource.

2. Should ResourceManager have all the ResourceContainers (which then contain Resource objects)?

[...]

The manager should have all containers that are needed to manage the resource types the manager is responsible for. I used this "weak" sentence because you may find it useful to manage resources for each game level separately, or (which means an inclusive or) you may find it useful to manage resources separated by type.

[...]
Should ResourceManager even call ResourceLoader?

In this discussion we use ResourceManager like a director. The manager knows of the loader, cache, ... and mediates between them; that is its job. When going this route, then there is no need to couple e.g. the ResourceLoader to the ResourceContainer just for the purpose to enable the loader to store the resource directly. The ResourceManager, on the other hand, is coupled to the ResourceContainer anyway, because it needs to check whether a resource is already loaded before wasting time by calling a loader.

Notice that the responsibility of ResourceLoader is to load a requested resource. Whether it is meaningful to load it and what happens to the resource after loading is out of the scope of the loader. Restricting the responsibility like so makes it easier to re-use the classes in other contexts. That is one of the points of maintainability.

3. Do you mean that the Sprite class should not have functions to write/read buffers? If so, do you mean there should be a SpriteSerializer class?

Serializing an object means to transform its data into an external representation. In other words, a specific format is used. But the format can change (e.g. XML vs. binary, or version evolution). To abstract this, we have ResourceLoader (and ResourceWriter if saving is supported). So SpriteSerializer would just be another concretization of ResourceWriter.

Notice please that a sprite (as example for any resource) is just a typed data container including some metadata. The animation sub-system alters its world transform, the graphic renderer reads it for the purpose of rendering, the resource loader is able to write data to it, and so on. The sprite itself, as a resource, does almost nothing by itself. If you want to support ResourceLoader / ResourceWriter directly from the resource, then at most something like the Memento pattern should be used.

Some more things to think about:

You have probably noticed that the loader classes so far are designed to abstract the format of the data source, ignoring the type of resource. (BTW: Here the XMLResourceLoader is not meant to understand XML files in general but XML files formatted to represent your game resources, of course.) Real world file formats often represent a single type of resource (see PNG, JPG, WAV, OBJ, ...). In such cases the concrete ResourceLoader (e.g. PngResourceLoader) is a-priorily defined to result in a specific resource type. But in cases of games we often deal with file formats that provide collections of resources (a.k.a. package files or archive files). So, if you want to support single resource loading, you need to support the internal directory that is inherent to such collections. This can be solved by implementing the concrete ResourceLoader to handle the collection stuff and to revert to other concrete ResourceLoader instances as soon as the type of the resource is determined.

Further, we have not examined the dimension of data sources. So far we load resources from files. That is fine in general because files are already abstractions, since they may be mass storage based or socket based. If ResourceLoader should be agnostic of what kind of file it is dealing with, the file has to be opened external to the ResourceLoader class. IMHO even better, any ResourceLoader should deal with an abstraction DataSource anyway.

Nono, I don't store the frames parameter in Resource.

This map is defined in Resource:

std::map m_parameters;

Then each class that derive from Resource, needs to implement an initialise() function which has to at least do one thing to work with the ResourceManager: set the "class" to the class name. Read my other reply again.



Either way; I disagree that it's wasteful, because it's not like you'll have thousands of resources loaded at once. Even then, it's not that bad. We're talking some kilobytes in total.

I'll agree there probably is a better way to do it though.

Ok, replying to this goes somewhat off topic, but this is very timely for me.

So you do store the parameters as name-value pairs like I suggested. It's a map. I guess that the Parameter class stores some union of admissible parameter types? Anyway, if you don't cache the values to some C++ types, each time you request a parameter from a resource you will need to make O(logN) lookup of the map, performing string comparisons. This may be neglible in some simple 2D game with a few resources, but I would not use it for rendering complex 3D scenes with hundreds of resources (with multitude of parameters) accessed each frame.

Storing the names may not take more than a few kilobytes, but usually such degeneracy suggests that the design could be improved.

These parameters aren't really meant to be accessed every frame though. It's mostly just to help build up the object.

I agree there are better ways though.

-snip-

Thanks a lot for your explanation! It really cleared up a lot of confusion.

I'll definitely start applying this in my library. Seems like it'll clean up quite a bit.

Thanks for actually writing such a long post to explain it to me. :)

I'll definitely start applying this in my library. Seems like it'll clean up quite a bit.

Fine :) But remember that it is only one way of all the possibilities. Use whatever is meaningful for you.

For example, in my own resource management, the ResourceLoader is not the part that deals with the file data. Also due to other reasons of de-coupling, I have ResourceType and IOFormat, both being base classes. IOFormat represents the format itself, e.g. it has stuff like "how competent are you to interpret this given block of data?". More important to this thread, however, is that it provides 2 abstract inner classes IOFormat::Importer and IOFormat::Exporter as well as 2 abstract factory methods one for the importer and one for the exporter. These both classes are which actually deal with file formatted data. The ResourceLoader itself is more or less just another thing in-between, since it uses a concrete ResourceType as well as a concrete IOFormat and IOFormat::Importer to fulfill its task.

To come to the point, somebody once said "there is no software problem that cannot be solved by another level of indirection, with the exception of having too many indirections". And so you have to decide time and again when your "too many levels" has been reached ...