Generic Resource_Manager WIP with lots TODO

Just posting this for a peer review. Largely incomplete..

Resource_Manager.h

Code:

#pragma warning(disable: 4786)

#include <vector>
#include "MS3D.H"
#include <map>

class Resource_Cache // The "containers" and their data structures
{
friend class Resource_Manager;
public:
	struct	MS3DModelRenderData
	{
		float Position[3]; // will be changed to vectors eventually
		float Orientation[3];
		MS3DModel *pModel;
	};

	std::vector<MS3DModelRenderData*> MS3DModels; // holds pointers to all MS3D Models
private:

	bool Add_MS3DModel_To_Cache(std::string Filename); // loads a milkshape model to the cache

};

template < typename resource_t >
class Manager					// container managers
{

    std::map< std::string , resource_t* > Resources;

public:

    resource_t & Load( const std::string & filename );


	virtual ~Manager( void )
	{
		std::map< std::string ,  resource_t * >::iterator destroyer, end;
		for ( destroyer = Resources.begin() , end = Resources.end() ; destroyer != end ; ++destroyer ) 
		{
			delete destroyer->second;
		}
	}	

};

class Resource_Manager	// Global Manager
{
	Resource_Cache GlobalResources;
    Manager<MS3DModel> Models;

	std::string get_extension(std::string Filename);

public:
	void Add_Resource(std::string Filename)
	{
		
		if (get_extension (Filename ) == "ms3d")
		{
			GlobalResources.Add_MS3DModel_To_Cache(Filename);
		}
		
	}

};

Resource_Manager.cpp

Code:

#include "Resource_Manager.h"
#include <windows.h>

////////////////////////////////////
// Resource Cache //////////////////
////////////////////////////////////
bool Resource_Cache::Add_MS3DModel_To_Cache(std::string Filename)
{
	MS3DModelRenderData *Model;
	Model = new MS3DModelRenderData;
	Model->pModel = new MS3DModel;

	if ( Model->pModel->Load_MS3D_Model( Filename ) == false )
	{
		MessageBox( NULL, "Couldn't load the model data.", "Error", MB_OK | MB_ICONERROR );
		return 0;									// If Model Didn't Load, Quit
	}

	 MS3DModels.push_back(Model); 

	return 1;
}
///////////////////////////////////////////////
///////////////////////////////////////////////
///////////////////////////////////////////////

///////////////////////////////////////////////
// Manager Template ///////////////////////////
///////////////////////////////////////////////
template < typename resource_t >
resource_t & Manager<resource_t>::Load( const std::string & filename ) 
{
	std::map< std::string , resource_t* >::iterator entry = Resources.find( filename );
    if ( entry != Resources.end() ) // if the entry is found
	{ 
		return * entry->second; // return the reference to the resource
    } 
	else
	{
		resource_t* resource( new resource_t( filename ) );
		
		// first we gotta load the resource, then we can make a pair
		// and return the reference
	
		resources.insert( std::make_pair( filename , resource ) );
		return * resource;
    }
}
///////////////////////////////////////////////
///////////////////////////////////////////////
///////////////////////////////////////////////


///////////////////////////////////////////////
// Resource Manager ///////////////////////////
///////////////////////////////////////////////
std::string Resource_Manager::get_extension(std::string Filename)
{
	std::string::size_type last_dot_index = Filename.rfind( '.' );
	std::string extension = Filename.substr( last_dot_index + 1 ); //+1 to not include the dot...
	return extension;
}
///////////////////////////////////////////////
///////////////////////////////////////////////
///////////////////////////////////////////////

Thats about it so far, pretty simple...

TODO list includes:

Make it so the Add_Resource function uses the managers to compose a list of already loaded resources and their references.

Add more functionality to the Load functions in Resource_Cache to return a reference to a resource, so that you can properly pair up a filename with its cooresponding resource


Hmm, that is all I can think of...

EXPLANATION:

Well, Resource_Cache holds containers of information, right now currently only for MS3D Models. The Manager template manages the I/O of the Resource_Cache, the Resource_Manager class decides which manager template gets utilized (one for each type of resource reference)...

a few comments

• you're passing your strings by value, as discussed in other thread
• instead of
  
  Code:

  std::vector<MS3DModelRenderData*> MS3DModels;

  consider using a boost.ptr_vector to make memory management easier
• I would typedef
  
  Code:

  std::map< std::string , resource_t* > Resources;

  to
  Code:

   typedef typename std::map< std::string , resource_t* > ResourceMap;

  this makes declarations like
  
  Code:

  std::map< std::string ,  resource_t * >::iterator

  much easier to read.
  
  Code:

  ResourceMap::iterator

that's a very brief glance

Thanks for the review..

Question, Do you think this is a logically correct way to go about doing this?

I'm having trouble figuring out the order of operations for a Global Add_to_Cache function...


Pseudocode

Code:

void Add_Resource(std::string & filename)
{
   if (get_extension( filename ) == "ms3d")
      A. use template to check for already existing file
     A1. If exists, pushback a reference to the resource into the vector // done
     A2. If doesn't exist, proceed to load file, return the reference
     B2. Use template to make the filename and the reference a pair to prevent future reloading
};

This would suggest I can't have one Load template function...

I need a check_existence function that performs a return reference function, and another function that takes a filename and a reference and makes a pair out of the two..

I can't do steps A and B2 in the same function, I don't think...

Any suggestions?

Is this going to be a Windows resource manager? If so, it sounds like a cool idea.

Well, it is ment to be an internal system for a game really...

I plan on adding support for .ms3d, .mp3's, maybe a few others, really only things pertaining to the game..

Updated code.. please review, point out any pending errors..

Resource_Manager.h

Code:

#pragma warning(disable: 4786)

#include <vector>
#include "MS3D.H"
#include <map>

class Resource_Cache
{
friend class Resource_Manager;
public:
	struct	MS3DModelRenderData
	{
		float Position[3]; // will be changed to vectors eventually
		float Orientation[3];
		MS3DModel *pModel;
	};

	std::vector<MS3DModelRenderData*> MS3DModels; // holds pointers to all MS3D Models
private:

	bool Add_MS3DModel_To_Cache(std::string Filename); // loads a milkshape model to the cache

};

template < typename resource_t >
class Manager
{

    std::map< std::string , resource_t* > Resources;

public:

	bool Check_Existance( const std::string & filename );
    resource_t * Return_Resource_Reference( const std::string & filename );
	

	virtual ~Manager( void )
	{
		std::map< std::string ,  resource_t * >::iterator destroyer, end;
		for ( destroyer = Resources.begin() , end = Resources.end() ; destroyer != end ; ++destroyer ) 
		{
			delete destroyer->second;
		}
	}	

};

class Resource_Manager
{
	Resource_Cache GlobalResources;
    Manager<MS3DModel> Models;

	std::string get_extension(std::string Filename);

public:

	void Add_Resource(std::string Filename);


};

Resource_Manager.cpp

Code:

#include "Resource_Manager.h"
#include <windows.h>

////////////////////////////////////
// Resource Cache //////////////////
////////////////////////////////////
bool Resource_Cache::Add_MS3DModel_To_Cache(std::string Filename)
{
	MS3DModelRenderData *Model;
	Model = new MS3DModelRenderData;
	Model->pModel = new MS3DModel;

	if ( Model->pModel->Load_MS3D_Model( Filename ) == false )
	{
		MessageBox( NULL, "Couldn't load the model data.", "Error", MB_OK | MB_ICONERROR );
		return 0;									// If Model Didn't Load, Quit
	}

	 MS3DModels.push_back(Model); 

	return 1;
}
///////////////////////////////////////////////
///////////////////////////////////////////////
///////////////////////////////////////////////

///////////////////////////////////////////////
// Manager Template ///////////////////////////
///////////////////////////////////////////////
template < typename resource_t >
resource_t * Manager<resource_t>::Return_Resource_Reference(const std::string & filename ) 
{
	std::map< std::string , resource_t* >::iterator entry = Resources.find( filename );
    if ( entry != Resources.end() ) // if the entry is found
	{ 
		return entry->second; // return the reference to the resource
    } 
/*
	else
	{

		resource_t* resource( new resource_t( filename ) );
		
		// first we gotta load the resource, then we can make a pair
		// and return the reference
	
		resources.insert( std::make_pair( filename , resource ) );
		return * resource;
    }
*/
	else return false;
}
template < typename resource_t >
bool Manager<resource_t>::Check_Existance( const std::string & filename )
{
	std::map< std::string , resource_t* >::iterator entry = Resources.find( filename );
    if ( entry != Resources.end() ) // if the entry is found
	{ 
		return true;
    } 
	else return false;
}
///////////////////////////////////////////////
///////////////////////////////////////////////
///////////////////////////////////////////////



///////////////////////////////////////////////
// Resource Manager ///////////////////////////
///////////////////////////////////////////////
std::string Resource_Manager::get_extension(std::string Filename)
{
	std::string::size_type last_dot_index = Filename.rfind( '.' );
	std::string extension = Filename.substr( last_dot_index + 1 ); //+1 to not include the dot...
	return extension;
}

void Resource_Manager::Add_Resource(std::string Filename)
{
		
	if (get_extension (Filename ) == "ms3d")
	{
		if ( Models.Check_Existance (Filename) == true  )
		{
			Resource_Cache::MS3DModelRenderData * RenderObject = new Resource_Cache::MS3DModelRenderData;
			RenderObject->pModel = Models.Return_Resource_Reference ( Filename );
			GlobalResources.MS3DModels.push_back(RenderObject);
		}
		else
		{

		}
	}
}		
////////////////////////////////////////////
///////////////////////////////////////////////
///////////////////////////////////////////////

I would typedef

Code:
std::map< std::string , resource_t* > Resources;
to
Code:
typedef typename std::map< std::string , resource_t* > ResourceMap;
this makes declarations like

Code:
std::map< std::string , resource_t * >::iteratormuch easier to read.

Code:
ResourceMap::iterator

I see no reason to do this. In fact it may confuse you later. I'm not a huge fan of typedefs like that because it obfuscates the code and makes the huge game engine you are coding that much more difficult to read since you gotta flip back and forth between the header and the cpp using the typedef. Personal opinion but I stay away from a lot of that stuff as much as possible.

I like how you have wrapped the manager into a template class since most of the managers do act the same it makes sense to make a template class and then set the data type later. I may in fact use this design in my own system. It sure makes more sense to write the manager code once and use it many times, than write a manager class for every object in the game. I like it.

Thanks very much for the response Bubba..

The main thing I'm concerned about is how I'm dealing with references and pointers, I'm really worried theres holes and hanging pointers lieing around...

Can anyone spot any problems with the pointers and such?

And finally, Proof of concept, completed version... currently only works with .ms3d files..

Resource_Manager.h

Code:

#pragma warning(disable: 4786)

#include <vector>
#include "MS3D.H"
#include <map>

class Resource_Cache
{
friend class Resource_Manager;
public:
	struct	MS3DModelRenderData
	{
		float Position[3]; // will be changed to vectors eventually
		float Orientation[3];
		MS3DModel *pModel;
	};

	std::vector<MS3DModelRenderData*> MS3DModels; // holds pointers to all MS3D Models
private:

	MS3DModel * Add_MS3DModel_To_Cache(std::string Filename); // loads a milkshape model to the cache

};

template < typename resource_t >
class Manager
{

    std::map< std::string , resource_t* > Resources;

public:

	bool Check_Existance( const std::string & filename );
    resource_t * Return_Resource_Reference( const std::string & filename );
	void Add_To_List( const std::string & Filename, resource_t* );

	virtual ~Manager( void )
	{
		std::map< std::string ,  resource_t * >::iterator destroyer, end;
		for ( destroyer = Resources.begin() , end = Resources.end() ; destroyer != end ; ++destroyer ) 
		{
			delete destroyer->second;
		}
	}	

};

class Resource_Manager
{
	Resource_Cache GlobalResources;
    Manager<MS3DModel> Models;

	std::string get_extension(std::string Filename);

public:

	void Add_Resource(std::string Filename);


};

Resource_Manager.cpp

Code:

#include "Resource_Manager.h"
#include <windows.h>
#include <iostream>
#include "assert.h"
////////////////////////////////////
// Resource Cache //////////////////
////////////////////////////////////
MS3DModel * Resource_Cache::Add_MS3DModel_To_Cache(std::string Filename)
{
	MS3DModelRenderData *Model;
	Model = new MS3DModelRenderData;
	Model->pModel = new MS3DModel;

	if ( Model->pModel->Load_MS3D_Model( Filename ) == false )
	{
		MessageBox( NULL, "Couldn't load the model data.", "Error", MB_OK | MB_ICONERROR );
		assert(Model->pModel != NULL);
		return 0;
	}

	 MS3DModels.push_back(Model); 

	return Model->pModel;
}
///////////////////////////////////////////////
///////////////////////////////////////////////
///////////////////////////////////////////////

///////////////////////////////////////////////
// Manager Template ///////////////////////////
///////////////////////////////////////////////
template < typename resource_t >
resource_t * Manager<resource_t>::Return_Resource_Reference(const std::string & filename ) 
{
	std::map< std::string , resource_t* >::iterator entry = Resources.find( filename );
    if ( entry != Resources.end() ) // if the entry is found
	{ 
		return entry->second; // return the reference to the resource
    } 
/*
	else
	{

		resource_t* resource( new resource_t( filename ) );
		
		// first we gotta load the resource, then we can make a pair
		// and return the reference
	
		Resources.insert( std::make_pair( filename , resource ) );
		return * resource;
    }
*/
	else return false;
}

template < typename resource_t >
bool Manager<resource_t>::Check_Existance( const std::string & filename )
{
	std::map< std::string , resource_t* >::iterator entry = Resources.find( filename );
    if ( entry != Resources.end() ) // if the entry is found
	{ 
		return true;
    } 
	else return false;
}

template < typename resource_t >
void Manager<resource_t>::Add_To_List(const std::string & Filename, resource_t * Resource)
{
	Resources.insert(std::make_pair( Filename , Resource) );
}
///////////////////////////////////////////////
///////////////////////////////////////////////
///////////////////////////////////////////////



///////////////////////////////////////////////
// Resource Manager ///////////////////////////
///////////////////////////////////////////////
std::string Resource_Manager::get_extension(std::string Filename)
{
	std::string::size_type last_dot_index = Filename.rfind( '.' );
	std::string extension = Filename.substr( last_dot_index + 1 ); //+1 to not include the dot...
	return extension;
}

void Resource_Manager::Add_Resource(std::string Filename)
{
		
	if (get_extension (Filename ) == "ms3d")
	{
		if ( Models.Check_Existance (Filename) == true  )
		{
			std::cout << "Resource is on the list!" << std::endl;
			Resource_Cache::MS3DModelRenderData * RenderObject = new Resource_Cache::MS3DModelRenderData;
			RenderObject->pModel = Models.Return_Resource_Reference ( Filename );
			GlobalResources.MS3DModels.push_back(RenderObject);
		}
		else
		{
			std::cout << "Resource not on list!" << std::endl;
			
			Models.Add_To_List(Filename, GlobalResources.Add_MS3DModel_To_Cache(Filename));
		}
	}
}		
///////////////////////////////////////////////
///////////////////////////////////////////////
///////////////////////////////////////////////

And the test.cpp

Code:

#include "Resource_Manager.h"

Resource_Manager Global_Manager;
int main()
{
	Global_Manager.Add_Resource("AttackShip.ms3d");
	Global_Manager.Add_Resource("AttackShip.ms3d");
	return 0;
}

And finally, the output.. Very simple, telling us it wasn't in the list before, therefore it loads the file, then the second line of output is telling us the file is now on the list with a proper resource, and then just returns the cooresponding resource pointer...

Code:

Resource not on list!
Resource is on the list!
Press any key to continue

Hmmm, I'm pretty sure I'm destroying all encapsulation and modularity by closely tieing together the renderlist and the resource manager this way..

Right now I'm creating a renderlist directly through the resource manager.. To keep in the spirit of a generic resource manager, this simply won't do. But my question is, what system would create a resource list, if not the resource manager?

Right now, on other forums, this system is being challenged as unmodular and not encapsulated enough for its own good. Maybe I need to re-evaluate what exactly a resource manager should do...

A. Look up resources by string-based key and protect against multiple copies of resources with the same key.

B. Manage and protect the lifetime of those resources (creation and destruction; prevent users deleting a resource via a raw pointer)

A suggests that I should add the functionality to keep a list of loaded resources, as well as a reference to the resource accompanying the string. I should be able to search for a string, check its existance, and not load the resource again.

B suggests that the resource manager itself should handle the loading of a resource, the destruction of a resource, and prevent clients from deleting the resource. I.E the Rendering engine should not be able to remove a resource from the system.


1. The worst thing you could do it provide your client raw pointers. But the client needs to have some form of the actual resource or it won't be able to call methods on that resource.

This means, when a client, such as a rendering engine, requests a resource, we shouldn't give it a raw pointer. That could lead easily to dangling pointers and memory leaks. But the engine needs something, so we can give it an object that holds the pointer.. a proxy object, or, I.E -> a handle.

2. The resource manager itself simply keeps a list of resources and their names. It is responsible for allocating these resources (it does so in LoadResource(), which I haven't written here) and freeing them as needed (either when the manger itself is destroyed, or when the manager decides the resource hasn't been accessed recently enough, or the client calls a method on the manager to forcibly release a resource).

A couple of things, sort of related to stuff I have been doing (completely different, actually, but as you'll see very much the same).

Firstly you'll probably only have one resource manager for the app, correct? So that qualifies as a singleton. To implement this pattern, basically you make the constructor private, and make a public static class method called Instance() that returns a pointer to a resource manager. The point of the pattern is to guarantee that only one copy of the Resource manager exists. Here is a basic sample:

Code:

// resourcemanager.h
class ResourceManager {
    private:
    // Prevent explicit construction
    ResourceManager();
    ResourceManager(const ResourceManager&);
    static ResourceManager* pInstance;
    public:
    // This is the only way a caller can construct this class
    static ResourceManager* Instance();
};

// resourcemanager.cpp

// The pointer to the resourceManger:
ResourceManager* ResourceManager::pInstance = 0;

ResourceManager::ResourceManager()
{
    // any init here
}

ResourceManager* ResourceManager::Instance()
{
    // only one instance will ever exist
    if (!pInstance) pInstance = new ResourceManager();

    return pInstance;
}

Second thing is, it sounds like you're constructing objects from strings, which means you might be better off with an object factory. Basically what you'll gain here is that you can construct any object with exactly the same code, and without coupling your resource manager to any of the actual resources. Also, as you add new resources, you won't need to modify the factory, and if you come up with a suitably general Resource base class, you won't have to modify the Manager code either.

I'm just throwing this code together so you can get the idea (it's loosely based on the code provided in "Modern C++ Design"). If you have any questions, let me know.

Code:

// First you have a common base class for all resources:
class Resource {
    // some kind of common interface code goes here
};

// Here's the ResourceFactory (it's a singleton, but I'm getting 
// lazy about coding it, so I just put an Instance() function. But 
// you code it just like the Singleton above.

class ResourceFactory {
public:
    ResourceFactory* Instance();
    typedef Resource* (*CreateResourceCallback)();

private:
    typedef std::map<std::string, CreateResourceCallback> CallbackMap;

public:
    bool RegisterResource(const std::string& name, CreateResourceCallback cb) {
        return callbacks_.insert(CallbackMap::value_type(name,cb)).second;
    }

    bool UnregisterResource(const std::string& name) {
        return callbacks_.erase(name) == 1;
    }

    Resource* Create(const std::string& name) {
        CallbackMap::const_iterator i = callbacks_.find(name);
        if (i == callbacks_.end()) {
            throw std::runtime_error("Unknown resource type");
        }
        return (i->second)();
     }
private:
    CallbackMap callbacks_;
};

// Now, each new resource registers itself with the Factory:

class MS3DResource : public Resource {
  // whatever code goes here for your resource
};

// This is the callback for the Factory:
MyResource* MS3DResourceCreator()
{
    return new MS3DResource();
}

// This is a way to guarantee that your resource registers itself when the app is loading:
class FactoryRegistrar {
    FactoryRegistrar() {
        ResourceFactory::Instance()->Register("MS3D",MS3DResourceCreator);
    }
} MS3DRegistrar;

// Then at the end of all this, you can construct the resource
// from only a string:

ResourceManager::Instance()->Create("MS3D");

Well...that's it. I know I screwed something up here somewhere. Hopefully nobody complains because I didn't break things up into files as needed, didn't include <map>, <string>, etc. where appropriate.

Also, I think you can find a generic implementation of Singleton and ObjectFactory if you google for Loki. That's the name of the library that "Modern C++ Design" is based on.

Code:

typedef std::map<std::string, CreateResourceCallback> CallbackMap;

Is that the container we callback for ALL resources?

Wouldn't this be inefficient? If we're looking for a sound resource, do we really want to search through a container filled with textures, sounds, bitmaps, and models?

that's a container of the types of resource that you have, and the value stored is a callback function that will create that resource.

For instance, when you call:

ResourceFactory::Instance()->Create("MyResource");

it's doing a lookup into the map for "MyResource" and then calling the creation function that was registered for it. So the Factory isn't really storing pointers to resources at all (presumably that's the job of your manager), it's just a design pattern for creating objects from strings.

Anyway, the std::map is really a red-black tree, so it's going to be pretty efficient no matter how much stuff you throw into it.

Originally Posted by Bubba

I see no reason to do this. In fact it may confuse you later. I'm not a huge fan of typedefs like that because it obfuscates the code and makes the huge game engine you are coding that much more difficult to read since you gotta flip back and forth between the header and the cpp using the typedef. Personal opinion but I stay away from a lot of that stuff as much as possible.

what happens when you want to change from a map to a multimap (or some other associative container)? In my version you change the typedef and everything else works. In yours, you have to change every instance of the map, not to mention every iterator, const_iterator, value_type, etc.

Code in the future tense.

and if you don't like typedefs I presume you always use

Code:

std::basic_string<char, char_traits<char>, allocator<char> >

instead of std::string?

you need to know the API of any class you use anyway, so "flipping back and forth between the header and the cpp" isn't really anymore of an issue with a typedef then not.