I used to make the resource module load the objects, but then read about this single responsibility principle, and changed it so objects load themselves. But I still have to make them go through the module to manage and avoid duplicates.
Hah, it appear you haven't fully understood the principle about the single responsibility principle ( haha ) yet. SRP in one wording means that a class should have only one well defined task. So what does your image class now does? It at least acts as a image-data container and knows how to load an image... oups. See that bold word? Thats a very bad sign to begin with, when talking about one responsiblity.
Seeing how you tried to avoid the responsiblity from the resourceModule is a good thing, but you shouldn't have moved it to the image class. I'd personally made a seperate ImageLoader-class. This class knows how to load and create an i mage, and store it in the resourceModule.
And with this design, to get rid of the static modules, I'd need to send a bunch of pointers a few layers down just so a new object can be created.
Aye, and let met tell you, thats a very good thing! This will most importantly keep you from creating objects all over the place, and force you to seperate the graphics resource creation into their own classes, instead of messing with them wherever you want. What you show, is what you want to be able to do in a script language, and not in low/mid-level game code. In my own engine, I mostly have to pass in my different resource loaders 3 layers: It starts in the game state, there it goes into the game system, and there I'll have some loader class (terrain loader, particle loader), that takes it. Let me show you a perfect way of "hiding" those kinds of dependencies away, but in a good way:
class MaterialLoader final :
MaterialLoader(const gfx::Effects& effects, const gfx::Textures& textures, gfx::Materials& materials);
gfx::IMaterial* Load(const std::wstring& stName, const std::wstring& stEffectName, unsigned int effectPermutation, const std::wstring* pTextures, unsigned int numTextures, const SubsetVector* pSubsets = nullptr) const override;
const gfx::Effects* m_pEffects;
const gfx::Textures* m_pTextures;
Thats my material-loader. A material needs both an effect (shader) and textures, as well as some other variables. Granted this would be already a good example for wrapping parameters in a structure, but look at the constructor. I pass in the effect and texture cache (const), as well as the material cache. Thats a perfect visible dependency: Whoever looks at this class sees it needs the effect and texture cache for read, and possibly writes to the material cache. Also, this dependency is only there in the constructor. The load-method is completly independant of this. Now I construct my material loader, and after that, there is no difference in handling it that where if the material had its own loading-method.
I implemented your suggestion, it was getting pretty messy after all, so now it's:
Thats already a leap ahead IMHO, one thing you'll want to improve in the future simply to save you time and doublicated code is maybe make a generalized templated ResourceCache-class, and derive your Textures/Meshes from that. Other than that, I can just tell you from my own experience, the more sophisticated your "high" level structures get, the better those resource handling etc. will become. For example, if you were to implement a clever gamestate-system, maybe with some entity/component-system, you wouldn't need to store the resourceModule in the GameEngine-class as static anymore. You would construct it in the method that creates the very first game state, and pass the resourceModule in to that. From there on, you'd forward it to your systems if needed. Oh, I think thats a perfect example of why globals are totally unnecessary really. Since you mentioned sounds/audio sources, I'll show you how my ECS handles it. First of all, there is the audio-source component.
struct AudioSource :
AudioSource(const std::wstring& stFile, bool bLoop);
See how its constructed? From a string and a boolean. I can construct this whever I want, without having to pass in anything at all. If a boss event down the complex instance handler -> ai handler -> event dispatcher -> and so on wants to emit a sound, this is what it creates. Now my audio-system on the other hand does this:
void AudioSystem::Update(double dt)
auto vEntities = m_pEntities->EntitiesWithComponents<AudioSource>();
for(auto pEntity: vEntities)
AudioSource* pSource = pEntity->GetComponent<AudioSource>();
pSource->pEmitter = new audio::Emitter(*m_pDevice, *m_files[pSource->stFile], pSource->bLoop);
It creates the actual sound emitter, using a file that has been loaded to the audio file cache (m_files). The audio-system only gets updated once per frame in the game state (one layer down). Its not so much about that design, but about how you can decouple things like loading, and even initialization altoghether. I hope you get the idea and can take something from this, I unfortunately lack an example that is directly tailored to your problem here...
Changing it to pointers and sending to the constructor seems it would just move the dependency from Load() to Constructor().
Thats not exactly true. You've now removed the loading code from the image-class itself. Now any external structure can construct an image the way it wants, and not depend on the image class itself. If we are talking about the same thing, namely:
Object(Texture2D* texture): _texture(texture)
I'm not talking about simply passing in the resourceModule to the constructor, that would be bogus. You should let the Object (shouldn't it be image) be initialized from an external source, via the constructor. Then you have an ObjectLoader-class, that loads the fitting texture from the cache, and assigns it. See? Though I can't make a puff of smoke and a "Puff!" sound effect, the depencency has magically disappeared ;)
I never do that. I am hardly pressed trying to identify an object that is "useful" in its default state.
What's the point of having a vertex buffer that points to nothing? You'll have to handle that useless situation everywhere both inside and outside the class. Just pass in the stuff needed to create a "useful" VB in the constructor and you're done in one line. In my experience this kind of code:
Seconded. A constructor (not default) is there for a reason, and using it in a sane way is perfect for not hiding, but easing the issues of dependencies.
foo->init(); // THIS ONE, I HATE IT WITH A PASSION.. JUST INIT INIT IN THE CONSTRUCTOR!
Ah, I used to do this all the time in my past projects. Its so tedious. Half of the time, you'd forgot to init and deinit() afterwards. And whats the point? Pass in the constructor. Its a little more complication for no real gain.
I got no trouble with the global static arrays - they probably are most performant as cache friendly (maybe it is also depending on the layout of data inside) - but I am writing in c
Let me lean myself out of the window by saying that if anything, statics are anything else than cache friendly. For statics, we have no control over how and where they are created. I know little about memory optimization, but that sound wrong to me. Linear memory used concurrently, thats cache-friendly from what I know. Statics? Not so much...
Edited by Juliean, 04 September 2013 - 04:40 AM.