Resource Manager in C++ for Games

Question

I'm currently writing a small game, needing some advice for things I could do better on this resource management system. Speed is not crucial, and I have no memory budget.

The goal is to load and store resources so they can be reused and accessed with a string. That way, all resources are loaded at once, and can be reused by this singleton from anywhere else in the code. This system currently works, I'm just looking for ways to improve it or better leverage STL's features.

The Resource class

class Resource
{
public:
    Resource();
    virtual ~Resource();

    virtual void loadResource() = 0;
    virtual void unloadResource() = 0;

    void setResourceId(unsigned id);
    unsigned getResourceId() const;

    void setResourcePath(const std::string& path);
    std::string getResourcePath() const;

protected:
    unsigned mResourceId;
    std::string mResourcePath;

};

It's a simple base class with getters and setters, along with pure virtual loadResource() and unloadResource() functions that each derivative will override.

The ResourceManager class

class ResourceManager : public Singleton<ResourceManager>
{
public:
    ResourceManager();
    virtual ~ResourceManager();

    void addResource(Resource* resource, const std::string& name, const std::string& path);

    template <typename T>
    T* getResource(const std::string& name) {
        return dynamic_cast<T*>(mResources.find(name)->second);
    }

    static ResourceManager& getInstance();
    static ResourceManager* getInstancePtr();

private:
    std::map<std::string, Resource*> mResources;

};

The addResource implementation

void ResourceManager::addResource(Resource* resource, const std::string& name, const std::string& path)
{
    resource->setResourcePath(path);
    resource->setResourceId(mResources.size());
    resource->loadResource();

    mResources.insert(std::pair<std::string, Resource*>(name, resource));
}

The unloadResource function is called for every Resource in ResourceManager's destructor.

Usage example: loading the resource (with a hypothetical Texture class)

mResourceManager->addResource(new Texture, "test", "Resources/test.png");

Usage example: accessing the resource

mResourceManager->getResource<Texture>("test");

Like I said earlier, speed/memory isn't crucial, so all solutions are welcome.

Answer 1

The ResourceManager class

You should probably use a std::unordered_map<> (from the <unordered_map> header) instead of a std::map, since resources don't really need to be ordered as far as I can see. This will give you some speed, even if you're not concerned by that.

Consider throwing an exception from the getResource() function if the dynamic_cast<> fails. Right now, it is up the caller to check that the returned pointer is not null, it might be better if the function simply throws an exception so that client code does not have to wrap every single call with a check.

This is up to you, since now the client might have to wrap calls in try-catches. However, it certainly makes errors pop up a lot more visibly and at the call location, instead of possibly later if the client forgets to check for null.

The addResource implementation

You never check that resource is not null, which a pointer can be. Consider checking or taking a Resource& instead to ensure that you are operating on an existing value.

Your insertion into the map looks like this:

mResources.insert(std::pair<std::string, Resource*>(name, resource));

std::map<> and std::unordered_map<> both have the emplace() member function. The function takes the arguments to construct the key-value-pair internally (thus avoiding copy/move operations). You can instead write your return statement like so:

mResources.emplace(name, resource);

This looks cleaner and is more efficient.

---

Be careful with your map, I don't know what unload resource does, but it better call delete if that's (dynamically) how you're allocating your resources. However, a better idea might instead to use std::unique_ptr<> with a custom deleter. Here's a mock implementation:

#include <iostream>
#include <memory>

struct resource
{
    void unload_resource() { std::cout << "unloaded\n"; }
};

struct resource_free
{
    void operator()(resource* r)
    {
        r->unload_resource();
        delete r;
    }
};

int main()
{
    std::unique_ptr<resource, resource_free> pres{ new resource };
}

This will simplify the implementation of the destructor as std::unique_ptr<> takes care of properly releasing your resource. You can make an alias for ease of use: using resource_ptr = std::unique_ptr<...>. This allows you to write your map variable declaration in an easier to read way:

std::unordered_map<std::string, resource_ptr> mResources;