Take the 2-minute tour ×
Game Development Stack Exchange is a question and answer site for professional and independent game developers. It's 100% free, no registration required.

What are the pitfalls associated with loading and storing all references to all of your content/assets within a single static class?

I've been trying to ask myself why I would want to instantiate an assets bank and can not think of a good reason to do so.

I have made a static class which enumerates through my game assets and adds each one to a dictionary which I can access (safely exposed, of course) anywhere like so:

var tex = Assets.Image("folder\\easyaspie");
spriteBatch.DrawString(Assets.Font("font2"), new Vector2(100, 100), Color.White);

... you get the picture.

I appreciate that this gets much more complicated when you have so many assets that having them all loaded at once is not a good idea. However, there is nothing to stop you from including selective loading of assets in your static class, as well as having static methods to change the contents of the 'reference banks'.

What do you think?

Edit:

My examples are pretty shallow, I recognise it's not a good idea to attempt to grab the reference of an asset mid-draw and is much better stored locally.

share|improve this question
    
Dictionary is good, thats what resource managers do, however being able to access this from anywhere isnt going to be a great idea as it will introduce dependencies throughout your codebase, particularly when the code assumes something is loaded yet it is either not, or is still being read from disk. Its usually a better idea to use some kind of asynchronous messaging to ask for resources and be handed them when they are ready. –  Matt D Jan 2 at 4:55
    
Can this functionality be adequately provided by a static class though? –  Joe Jan 2 at 5:00
    
You know, ContentManager is already a dictionary of loaded content (until you Unload it). So any implementation of this can simply use it directly, rather than creating and filling a separate dictionary. –  Andrew Russell Jan 2 at 6:46
    
One way to do it is store a static reference to the AssetManager inside the AssetManager class, have a static getInstance() method that returns that instance. That way you have an API such hat you can access the AssetManager anywhere, but you can still control the lifespan and properly clean up when the application closes. –  rcapote Jan 3 at 17:23
add comment

2 Answers

up vote 8 down vote accepted

I would strongly recommend you do not do this.

Static classes are fine for things that lack state or data, especially in languages like C# where you cannot otherwise have free-standing (non-member) functions. Once a class has any kind of state or data, however, you should seriously re-evaluate the decision to make it static. A good rule of thumb is to prefer instance to static; it greatly improves the long-term quality and maintainability of your code if you do.

For a case like this, an asset bank, I don't think a static class is a good idea:

  • There's only one asset bank. That means you forgo the possibility of multiple banks of assets (for example, allowing you to load an unload assets as they are needed to conserve memory, or for unit testing). Once you start building this "selective loading" into a single static bank as a way to bypass this problem, you're just engineering a shoddy mockery of what you could have done more simply as an instance class.

  • Because there's only one, you have to either engineer to the lowest-common-denominator in terms of access control and safety (think cross-thread access issues), or throw such issues to the wind and demand that client code write and thread-safety wrappers needed. This results in poorly-performing code in many cases for the former option, and the latter option often leads to code duplication and the resulting bugs.

  • Being able to easily access the resources anywhere is usually a disadvantage, not an advantage. It might seem the opposite at first, but unconstrained ubiquitous access means you either already have or will be easily able to create a mess of poorly-scoped dependencies on the assets. It's the same general principle behind why globals are generally undesirable constructs.

Instead, build your asset bank as a regular class, and create a single instance of it when the game loads. Pass references to that interface to methods or other interfaces that have a need to load or otherwise access assets.

This gives you the flexibility easily support multiple banks in the future, without constraining you a design that can be very hard to recover from. Making the class static is simple enough, but if you ever need to change that you may find yourself having to clean up a mess of dependencies in client code that assumed there would only ever been a single bank, rather than just working with the bank provided.

Further, this method makes it obvious when you start to introduce dependency creep: when you find that you need to pass the asset bank reference to everything in your game, that is what tells you that your game's subsystems are not well separable and you should consider redesign their API boundaries.

I don't really see any concrete benefits to using a static class for this sort of thing except, perhaps, in an extremely simple bit of tech demo code. Most of the things that appear to be advantages to the static class decision relate to "simplicity" but usually are anything but, in the long run (and simplicity at the expense of quality is not an advantage).

share|improve this answer
1  
Additional note, debugging statics and startup sequences can be very annoying and especially when one static class depends on another for services. You'll find at some point soon that you end up having to micromanage all the statics' lifespans anyways, thus reducing the quick and easy part of the whole idea. –  Patrick Hughes Jan 2 at 5:14
1  
"this method makes it obvious when you start to introduce dependency creep: when you find that you need to pass the asset bank reference to everything in your game" This is exactly what I wanted to combat, what are the non-static ways of tackling this? –  Joe Jan 2 at 5:16
    
You create your resource manager, and pass it as a parameter to only where it is needed. or, you use asynchronous messaging to communicate with it without needing to have direct access to it. –  Matt D Jan 2 at 5:19
    
@Joe "Don't do it" is the best way to combat it. It's unfortunately a really broad topic and can't reasonably be addressed here. Fixing the problem in existing code is usually very specific to the needs of the existing code, but preventing the problem is easier: you have to tightly control what has access to the resource in question, in this case the asset bank. For this it's pretty easy, it should be easy to draw the line between what actually needs the raw assets and what can make do with the things produced from the raw assets. –  Josh Petrie Jan 2 at 5:21
    
I don't want to turn this into an extended comment discussion, but you can get in touch with me in the Game Development Chat at some point and we could discuss the topic further. –  Josh Petrie Jan 2 at 5:22
show 2 more comments

I actually don't have a problem with something like this:

spriteBatch.DrawString(StaticAssets.DefaultFont, new Vector2(100, 100), Color.White);

It's a fairly reasonable simplifying assumption to say that there is only one instance of the default font per program. The kind of scenarios where this breaks down are uncommon, complicated, and difficult to plan for in advance. YAGNI.

You wouldn't want to do this in a library for 3rd-party consumption. But for a small-to-medium-sized game, where you can easily modify your own code, it's not a problem. Make your code flexible. Do things that save development time.

When you break the rules like this, it's important to do it deliberately, and to have an easy "out". In the above case, you can right click DefaultFont, do a "Find All References" and change how it works in a matter of minutes.

The problem with the original code, doing a dictionary lookup by string like this:

spriteBatch.DrawString(Assets.Font("font2"), new Vector2(100, 100), Color.White);

Is that you can't as easily discover all the places where the rules are being broken to reliably fix them when and if the need arises.


To put it another way - you really don't want a static asset manager (and I think Josh Petrie did a good job of explaining why). But many times you do want static assets!

Usually such assets will be static to a particular class, not global. So instead of having a Draw method like this:

spriteBatch.Draw(Assets.Image("playerSprite"), position, Color.White);

Or this:

spriteBatch.Draw(StaticAssets.PlayerSprite, position, Color.White);

You should probably do something like this:

class Player
{
    private static Texture2d playerSprite;

    public static void LoadContent(ContentManager content)
    {
        playerSprite = content.Load<Texture2d>("playerSprite");
    }

    public void Draw(SpriteBatch spriteBatch)
    {
        spriteBatch.Draw(playerSprite, position, Color.White);
    }
}

And then call each class's static LoadContent method in your game's LoadContent method. It's perfectly reasonable to assume that there will only ever be one instance of your Game class to be calling LoadContent.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.