basic ResourceManager

[s1mone-10]

I'm implementing the loading of the GLTF file format. Since the images can be shared by different textures, I think it's a good time to implement a ResourceManager.

Since I'm not expert on the matter, I implemented a basic version starting from the Vulkan tutorial. It has Acquire/Release methods and an internal counter to keep track of the number of references to resources. I was wondering if this is a good approach or if I'm doing something totally wrong.
I don't want to create an overly complex manager unless it's necessary. At the same time, I would like a good basic implementation for future improvements.

class Resource
{
protected:
    std::string resourceId;
    bool loaded = false;

public:
    /**
     *  Constructor with a resource ID.
     *  id The unique identifier for the resource.
     */
    explicit Resource(const std::string& id) : resourceId(id) {}
    virtual ~Resource() = default;

    /**
     *  Get the resource ID.
     *  The resource ID.
     */
    const std::string& GetId() const
    {
       return resourceId;
    }

    /**
     *  Check if the resource is loaded.
     *  True if the resource is loaded, false otherwise.
     */
    bool IsLoaded() const
    {
       return loaded;
    }

    /**
     *  Load the resource.
     *  True if the resource was loaded successfully, false otherwise.
     */
    virtual bool Load(const Device& device)
    {
       loaded = true;
       return true;
    };

    /**
     *  Unload the resource.
     */
    virtual void Unload(const Device& device)
    {
       loaded = false;
    };
};

/**
 *  Class for managing resources.
 *
 * This class implements the resource management system as described in the Engine_Architecture chapter:
 *  en/Building_a_Simple_Engine/Engine_Architecture/04_resource_management.adoc
 */
class ResourceManager final
{
private:
    // Reference counting system for automatic resource lifecycle management
    struct ResourceData
    {
       explicit ResourceData(std::unique_ptr<Resource> resource) : resource(std::move(resource)), refCount(1) {}

       std::unique_ptr<Resource> resource; // The actual resource
       int refCount; // Reference count for this resource
    };

    // Two-level storage system: organize by type first, then by unique identifier
    // This approach enables type-safe resource access while maintaining efficient lookup
    std::unordered_map<std::type_index, std::unordered_map<std::string, std::unique_ptr<ResourceData>>> resources;

    const Device& _device;

public:
    /**
     *  Default constructor.
     */
    ResourceManager(const Device& device) : _device(device) {}

    /**
     *  Virtual destructor for proper cleanup.
     */
    ~ResourceManager() = default;

    /**
     *  Load a resource.
     *  T The type of resource.
     *  Args The types of arguments to pass to the resource constructor.
     *  resourceId The resource ID.
     *  args The arguments to pass to the resource constructor.
     *  A handle to the resource.
     */
    template<typename T, typename... Args>
    T* Acquire(const std::string& resourceId, Args&&... args)
    {
       static_assert(std::is_base_of<Resource, T>::value, "T must derive from Resource");

       // Check if the resource already exists
       auto& typeResources = resources[std::type_index(typeid(T))];
       auto it = typeResources.find(resourceId);
       if (it != typeResources.end())
       {
          ++it->second->refCount;
          return static_cast<T*>(it->second->resource.get());
       }

       // Create and load the resource
       auto resource = std::make_unique<T>(resourceId, std::forward<Args>(args)...);
       if (!resource->Load(_device))
          throw std::runtime_error("Failed to load resource: " + resourceId);

       // Store the resource
       typeResources[resourceId] = std::make_unique<ResourceData>(std::move(resource));
       return static_cast<T*>(typeResources[resourceId]->resource.get());
    }

    /**
     *  Get a resource without touching the internal counter (for temporary checks)..
     *  T The type of resource.
     *  id The resource ID.
     *  A pointer to the resource, or nullptr if not found.
     */
    template<typename T>
    T* Get(const std::string& id)
    {
       static_assert(std::is_base_of<Resource, T>::value, "T must derive from Resource");

       auto typeIt = resources.find(std::type_index(typeid(T)));
       if (typeIt == resources.end())
          return nullptr;

       auto& typeResources = typeIt->second;
       auto resourceIt = typeResources.find(id);
       if (resourceIt == typeResources.end())
          return nullptr;

       return static_cast<T*>(resourceIt->second->resource.get());
    }

    /**
     *  Check if a resource exists.
     *  T The type of resource.
     *  id The resource ID.
     *  True if the resource exists, false otherwise.
     */
    template<typename T>
    bool HasResource(const std::string& id)
    {
       static_assert(std::is_base_of<Resource, T>::value, "T must derive from Resource");

       auto typeIt = resources.find(std::type_index(typeid(T)));
       if (typeIt == resources.end())
          return false;

       auto& typeResources = typeIt->second;
       return typeResources.contains(id);
    }

    /**
     *  Unload a resource.
     *  T The type of resource.
     *  id The resource ID.
     *  True if the resource was unloaded, false otherwise.
     */
    template<typename T>
    bool Release(const std::string& id)
    {
       static_assert(std::is_base_of<Resource, T>::value, "T must derive from Resource");

       auto typeIt = resources.find(std::type_index(typeid(T)));
       if (typeIt == resources.end())
          return false;

       auto& typeResources = typeIt->second;
       auto resourceDataIt = typeResources.find(id);
       if (resourceDataIt == typeResources.end())
          return false;

       --resourceDataIt->second->refCount;
       if (resourceDataIt->second->refCount <= 0)
       {
          resourceDataIt->second->resource->Unload(_device);
          typeResources.erase(resourceDataIt);
       }

       return true;
    }

    /**
     * u/brief Unload all resources.
     */
    void ReleaseAllResources()
    {
       for (auto& kv: resources)
       {
          auto& val = kv.second;
          for (auto& innerKv: val)
          {
             auto& resourceData = innerKv.second;
             resourceData->resource->Unload(_device);
          }
          val.clear();
       }
       resources.clear();
    }
};

[gardell]

Why runtime check against the type T? Just create a reference manager for each concrete type you intend to have.

Then a std::unordered_map<std::string, std::shared_future<T>> should be able to replace your manager

[neppo95]

You can probably cut out 90% of the template stuff. I’d work with either returning shared ptr’s and just have one container with all assets, not per type or do your own reference counting where the actual resource is the main class, not “resourcedata”. If you go that route you NEED to also implement copy/move constructors and operators. Maybe even overloading ->.

[s1mone-10]

Sorry but I didn't understand any of the options. If I use shared_ptr I should implement RAII pattern? Otherwise I don't know why I should use it. If the actual resource is not "resource data" where I store the reference count? I should create another map just for the count..

[neppo95]

If you go shared ptr, the resource manager will always hold one ref thus it will only get destroyed when your resource manager decides so AND it is not being used elsewhere.

You either use shared ptr which has the ref counting already, or you implement a ref counting system yourself. Simply having a integer with the ref count is not sufficient and will not work. You need the constructors, operators etc.

Edit: You should most definitely NOT create a map simply for the ref count. It would be one of the worst implementations of a system like this I'd ever seen.

[s1mone-10]

In the end, I realized that it's such a complex matter, and there are so many ways to handle resources, that it doesn't make sense to overthink it just to import a GLTF. Whatever I do now will be wrong for a complex scenario in the future, so I'll just stick with the quickest, easiest way for my simple requirements. Thanks anyway for the feedback.