Start adding [more responsibilities], and it starts to become more of a manager.
This (edited) quote is the core of the "manager issue" -- BlahManager is a naming convention used for classes that violate the
SRP.
If you've got something that is a cache, and a priority queue, and a decompressor, then it's responsible for too many things, so it gets named a "manager"...
You can have
* an AssetLoader which simply accepts requests to load data from disk into RAM and fires off notifications when loads are complete.
* a Decompressor which owns a shared decompression buffer and can transformed compressed streams of bytes into uncompressed streams of bytes.
* an AssetCache, for determining whether a load request needs to be generated or if an existing asset can be returned.
* a TextureFactory, which can take loaded/decompressed blobs of bytes and return a constructed texture.
Each of the above can be written in a completely decoupled manner,
so that none of them are aware of the others. The user of these classes can construct some delegates/callbacks that pipe them all together into a sensible sequence.
I think this is rarely going to be sufficient on non trivial projects. For the same reason as when a level editor loads it's '.scene' file, you wouldn't pass in a std::map of all files that might be loaded by that scene.. Additionally it just strikes me as needlessly exposing implementation details. Sure you're using a map, but what if you want to change to a hashtable later. If it actually saved a nontrivial amount of time to pass around a map directly I could understand the motivation, but it could be argued the opposite is true (as in it's easier to work with a customised wrapper than a generic container).
In my engine, the cache/map is a custom class called an AssetScope. The lifetime of assets are bound to the lifetime of an AssetScope (
i.e. when an asset scope is destructed, all the assets that were loaded using it are also destructed), so in order to load a scene, you would
have to make an AssetScope for that scene. The implementation of the AssetScope is hidden (it's a hash table), but the reason for using a custom container here is simply because my asset loading occurs across many threads concurrently, so
std::map/etc aren't valid.
My engine isn't as decoupled as the above example because the AssetScope explicitly knows what a BlobLoader is (
and vice versa, which isn't great), but it's close -- Assets are just handles that don't know the type of their data, and (
besides the above flaw) AssetScopes/Factories/Loaders/Decompressors are decoupled.
e.g.[source lang=cpp]struct Asset : NonCopyable{public: bool Loaded() const { return !!m_handle; } Handle GetHandle() const { return m_handle; }protected: Handle m_handle;};class AssetScope{public: AssetScope( ScopeAlloc& a, uint maxAssets, AssetScope* parent=0 ); template<class F> Asset* Load( AssetName n, BlobLoader& blobs, F& factory );};struct BlobLoadContext{ AssetScope* scope; AssetStorage* asset; void* factory;};class BlobLoader{public: struct Request { typedef void* (*PfnAllocate)(uint numBlobs, u32 blobSize[], u8 blobHint[], BlobLoadContext*); typedef void (*PfnComplete)(uint numBlobs, u8* blobData[], u32 blobSize[], BlobLoadContext*); BlobLoadContext userData; PfnAllocate pfnAllocate; PfnComplete pfnComplete; }; void Load(const AssetName&, const Request&); void Update();//polls requests to fire callbacks};class ShaderPackFactory : public BlobFactory<ShaderPackFactory>{public: ShaderPackFactory( GpuDevice& dev );private: void Acquire( uint numBlobs, u8* blobData[], u32 blobSize[] ); void Release( u8* blobData );};...m_blobLoader = eiNew(a, BlobLoader)( a );m_assetScope = eiNew(a, AssetScope)( a, scene.maxAssets );m_effectFactory = eiNew(a, ShaderPackFactory)( *m_gpuDevice );m_effectPackAsset = m_assetScope->Load( myAssetName, m_blobLoader, *m_effectFactory );...if( m_effectPackAsset.Loaded() ) m_effectPack = *m_effectPackAsset.GetHandle();[/source]
I have enjoyed reading this thread, but it seems that it is more and more heading in the direction like most singleton threads: everybody is throwing in their reasons for why singletons are bad. That was never really an issue to begin with. The thread starter asked for an alternative.
Either pass parameters to functions that would make use of Singletons, or pass parameters to those object's constructors (
or similarly store them in other context/state objects) and cache the pointer for use later in said functions. It really is that simple -- the rest is now a problem of refining your software engineering craft to reduce the amount of dependencies in your code-base. When using Singletons, your dependencies are invisible, so you don't realise how bad your code is, and when you switch to parameter passing without a redesign, an excess of parameter passing is simply showing you the amount of dependencies that you've been creating all along.
I kind of liked the example of the Entity class, which needs to load various types of Resources. Do you pass every ResourceManager to the Entity constructor? Do you group several ResourceManagers together into a LoadingContext and pass that? Is that really an improvement?
That's a bad example because you're trying to rescue an already bad design. What is this
Entity class that has to know about every resource type anyway? That's the reason it's hard to fix... What's the responsibility of an entity? Is it to manage the lifetime of it's member resources?
I digress... If the entity has to be responsible for resource creation, then you can pass in a directory of factories into the entity.
struct Entity { Entity( const map<ResType, Factory*>& ); }Another solution is to move resource creation out of the Entity and have other modules put resources
into an entity.
struct Entity { void AddMember( void* widget, void (*onDelete)(void*) ); };...but again, the best solution depends on exactly what the responsibility of an "entity" is, so you'd have to define that first.
4
This topic is locked
