Passing the ContentManager to every class feels wrong. Is it?

I have a singleton class called Platform that handles stuff like content loading so you only need to use a static reference to access it from anywhere.

That class implements an IPlatform interface so the Platform can (and does) have a slightly different implementation on different platforms. I also use it to manage things like saving to local storage, launching external apps (e.g. browsers) and managing certain things to do with the display. Also things like vibration which MG doesn’t directly support I believe.

What you’re doing here is following a dependency inversion model of coding, and it’s actually a good thing! It may seem strange to you, but you’re keeping your code decoupled from the rest of your system and this makes it much easier to maintain, refactor, and test if you so desire. If your class has external resources it needs to interact with, it’s far better to just give the class those resources. Then it can just focus on doing the job it is designed to do.

The singleton pattern works for stuff like this; however, I would advise against doing this only because it can make your code difficult to manage. If all of your objects expect a static ContentManager object to be available on some class somewhere, you’re now stuck to that design. This might seem ok for now, but if you have any code you want to reuse on your next project, you might find that a little bit more difficult or awkward.

Passing in a handful of dependencies in through your constructor/method parameters is pretty standard, but if you find yourself having a lot more, you might consider a dependency container. This is simply an object whose responsibility is to resolve dependencies. There are a lot of implementations of this and you can find a lot of info on the internet, but you can achieve a very simple one via a Dictionary<Type, object> structure. Then you can do something like this…

public interface IDependencyContainer
{
  void Register<T>(T dependency);
  T Get<T>();
}

public class DependencyContainerImplementation : IDependencyContainer
{
  // Implement this in whatever way you like.... :) 
}

public MyGame : Game
{
  private IDependencyContainer _container;

  public override void Initialize()
  {
    _container = new DependencyContainerImplementation();
    _container.Register<ContentManager>(this.ContentManager);

    GameObject newObject = new GameObject(_container);
  }
}

public class GameObject
{
  private Texture2D _texture;

  public GameObject(IDependencyContainer container)
  {
    _texture = container.Get<ContentManager>().Load<Texture2D>("TextureName");
  }
}

You’ll probably notice that this is actually somewhat similar to a singleton implementation but removes the static class and coupling across your entire system. It’s also just an example… I tend to not do this unless I have a lot of dependencies, and mostly because adding them to my constructor starts to make my unit tests really, really unmanageable because I have to refactor them every time I add a new one :wink:

Anyway, all this was to say you’re actually doing things correctly and are following best practices. Code you write in this fashion will actually serve you better in the long run, even though it kinda feels strange at first. I know this, it felt strange to me at first too.

3 Likes

I went to a static asset retrieval and haven’t looked back. Made everything cleaner and easier to use. I know people have a problem with this type of coupling but IMO it’s minor and here is why. I believe subscribing game managers to a central store and retrieving them later is a slower, less efficient way because it does a dictionary lookup followed by a cast usually. A static call is direct with less overhead. Essentially, IMO, a central manager store just remade what static does but added more complexity and overhead. Again, everyone’s opinions differ on this. Just do what feels right to you in your project.

Anyways, I have an Assets class. In there I have a textures Dictionary<string, Texture2D>. I load all textures there by their name from their file. I do it by reading the Content Directory and loading them in, so I don’t manually do anything, it is all automated. Then all I have to do to retrieve a texture from anywhere is go Assets.Texture(“Ship”); The ContentManager never goes beyond the Game1 and Assets class.

I also load other stuff in to the Assets class (sound, effects, textures, fonts). I removed a bunch of stuff but this is roughly what it looks like.

class Assets
{

    private static Dictionary<string, Effect> effects = new Dictionary<string, Effect>();
    private static Dictionary<string, SpriteFont> fonts = new Dictionary<string, SpriteFont>(); 
    private static Dictionary<string, Texture2D> textures = new Dictionary<string, Texture2D>();
    
    public static Texture2D Texture(string name)
    {
        return Assets.textures[name];
    }
    
    public static SpriteFont Font(string name)
    {
        return Assets.fonts[name];
    }
    public static Effect Effect(string name)
    {
        return Assets.effects[name];
    }

    private static string properMonoGameAssetPath(string path, string filename, string rootDir)
    {
        return Path.Combine(path.Substring(path.IndexOf(rootDir) + rootDir.Length), filename).Replace('\\', '/').Substring(1);
    }
    
    public static void LoadAllAssets(ContentManager cm)
    {
        Assets.effects = Assets.LoadAssets<Effect>(cm, "Effects");
        Assets.fonts = Assets.LoadAssets<SpriteFont>(cm, "Fonts");
        Assets.textures = Assets.LoadAssets<Texture2D>(cm, "Textures");
    }

    public static Dictionary<string, T> LoadAssets<T>(ContentManager ContentManager, string ContentDir)
    {
        DirectoryInfo dir = new DirectoryInfo(Path.Combine(ContentManager.RootDirectory, ContentDir));
        if (!dir.Exists)
            throw new DirectoryNotFoundException();

        Dictionary<string, T> tmp = new Dictionary<string, T>();
        FileInfo[] files = dir.GetFiles("*.*", SearchOption.AllDirectories);
        foreach (FileInfo file in files)
        {
            string asset = properMonoGameAssetPath(file.DirectoryName, Path.GetFileNameWithoutExtension(file.Name), ContentManager.RootDirectory);
            tmp.Add(asset, ContentManager.Load<T>(asset));
        }
        return tmp;
    }


}

And then I just call Assets.LoadAllAssets(this.Content) from the Game1 in the LoadContent section.

Edit:
Also I should note in my ContentPipeline I have 3 folders “Effects”, “Fonts” and “Textures” so it is all sorted for this retrieval process.

All asset loaded by the content are cached, so… they are loaded ONLY one time, even if many calls to load() are made for the same asset.
You are “reinventing the wheel” here :wink:

1 Like

Good to know. Though it is nice that everything is pre-cached by calling it this way before on demand in the game. Now that I know this, I’m going to drop my dictionaries and just make a static pointer to the content loader.

Thanks Alkher, I always like finding efficiencies gains.

It’s not really a big deal… at the end of the day, it’s up to you to decide how you want to design your program. These things are called best practices for a reason, but they’re also by no means a requirement :slight_smile:

I will say though, if a dictionary retrieval and cast is your only hold up then consider that the only time this would even matter is if you are doing this retrieval during an update or draw cycle. These dependencies are unlikely to change during run-time so having an object grab a reference to a dependency at construction time and hold onto it for use during Update/Draw is reliable. A dictionary lookup and cast is pretty fast these days, but any performance hit can be fairly easily mitigated if one wanted to adhere to this pattern.

Be warned you might encounter problems if you dispose assets manually (there are a lot of subjects about this on this forum)

It always amazes me how divided the opinions are on this topic. I’m going to try an answer the OP’s questions as objectively as possible without letting my personal opinion get in the way. I’ll try, I may fail.

When it comes to software design, anything can work, until it doesn’t. There’s no perfect design, but there are designs that can get you into trouble in certain situations.

Note: each design might have more than one problem but I’m just going to use one example for the sake of argument

Design 1: Passing the ContentManager to the entity

public Player(ContentManager contentManager)
{
   _texture = contentManager.Load<Texture2D>("player");
}

The first problem I see with this design is that the data about which texture to load is coupled to the class. For example, what happens if you want to make a second player and use a different textures for each instance e.g. "player1" and "player2".

There are a couple of solutions to this problem. You could pass in the texture name as a second parameter to the constructor but at this point you might as well just pass in the Texture2D directly. An alternate solution is to create a Player1 and Player2 class but unfortunately this won’t scale very well when you’ve got 32 players in your game.

Design 2: Static ContentManager

public Player(string texture)
{
   _texture = Game.MyContent.Load<Texture2D>(texture);
}

This design looks good on the surface. It appears to make your life easier because you’re only ever going to need one ContentManager in your game, right? Until one day you find a reason that you actually want to have 2 content managers. For example, when your loading level 2 you’d like to be able to unload all the content from level 1 and load the content for the next level without having to think about it too much.

There might be a few ways around this but in my experience the singleton pattern will eventually come back to bite you. You’ll be in a world of pain the moment you realize that you actually need more than one instance of something you’ve previously assumed is a singleton.

Design 3: Pass Texture2D directly

public Player(Texture2D texture)
{
   _texture = texture;
}

This design doesn’t have any of the previous problems. Constructor injection like this forces you to only inject the things you actually need to create an instance of that class. You can decide outside the Player class if you want more than one player or use more than one content manger.

This is my preferred design out of the 3 but I’ll try to be objective about it. Constructor injection isn’t perfect. The main problem with this design is that your constructors tend to have a lot of parameters passed in as your object gets more complex. There are other designs that try to solve this problem (composition, IoC, etc) but they also come with their own set of pros and cons.

In terms of efficiency none of the above designs are going to be noticeably different. By far the slowest part of this type of code is calling Content.Load. You’re going to have do to that in all cases and passing a few pointers around (weather it be Texture2D, ContentManager or looking up something from a Dictionary) isn’t going to make a lick of a difference.

3 Likes

You can also swap the way the dependency works and your Components can have a function GetTexturesNames() that returns a List<string> so you can ask what textures each component needs and pass the textures in either during initialization or during the draw call.

So a DrawableGameComponent or similar class but not derived from this (ie Design1), is a bad pattern ?

What you might be missing is “reusability” of your classes into another game, by just dropping the class into the new game, and attaching the gamecomponent, not bothering of what must be loaded by it:
In my engine, the DirtyLensFlare component loads it own data (effects, dirt textureetc,), the DepthOfField its own data again (effect), the UI (Textures for the UI etc) etc.

And by just dropping in and calling them in the game’s constructor, I don’t have to bother about what must be loaded because I need in this game a DirtyLensFlare for example, and then call a content.load for the 4 effects needed and a Texture2d load…

I’m not saying it’s a “bad pattern”. It’s just a pattern with different pros and cons like any other. If it works for you go for it. :slight_smile:

I’m all for creating reusable components if you can figure out a nice way to do it. In my experience it’s not quite that easy. Regardless of where you load your content there’s still a dependency on the content itself. In other words, if you copy the class over, you’ll also need to copy over the relevant bits of content related to it.

It’d be nice if there was a way to package up all of the related bits of content and code and call that thing an “asset” or something. Some game engines try to do something like that. However, that’s kind of a different issue to how the code is structured.

I hope the next OP’s inquiry is not Passing my object entities and input controller to every class feels wrong : - D because sooner or later you’ll have to access your object entities, input controller, etc… across the whole application.

I’ll go with static class that handles all managers I need to access across the whole application it makes my code clean simple and shorter. static class is a life saver for every .Net developer on a certain situation like game application don’t be afraid to use it ^_^y.

I can access all my managers on a single static class like shown below :

    /// <summary>
    /// Global static game managers handles
    /// </summary>
    public static class GameMngrs
    {

        //*>> Content managers
        static public FontContentMngr           _FontMngr;
        static public TextureContentMngr        _TextureMngr;
        static public CustomEffectContentMngr   _EffectMngr;
        static public MeshContentMngr           _MeshMngr;
        static public SoundContentMngr          _SoundMngr;

        //*>> Default Font;
        static public FontContent               _ArialFont;

        //*>> 2D managers
        static public LayerManager              _LayerMngr;
        static public ScreenManager             _ScreenMngr;
        static public SSpriteEntityManager      _ScreenSpriteMngr;
        static public TileEntityManager         _TileMngr;
        static public TileSpriteEntityManager   _TileSpriteMngr;
        static public GUIEntityManager          _GUIMngr;

        //*>> 3D Environment managers       
        static public EnvMngr                   _EnvMngr;
        static public LightsMngr                _LightsMngr;
        static public SkyBoxMngr                _SkyBoxMngr;
        static public CameraMngr                _CameraMngr;

        //*>> 3D managers       
        static public WorldMngr                 _WorldMngr;
        static public ModelEntityMngr           _ModelMngr;
        static public BillboardEntityMngr       _BillboardMngr;
        static public ParticleEntityMngr        _ParticleMngr;
        static public QuadEntityMngr            _QuadMngr;
        static public TrailEntityMngr           _TrailMngr;
        static public LineEntityMngr            _LineMngr;

        //*>> Input
        static public Z.IO.Keyboard             _KEYB;
        static public Z.IO.Mouse                _MOUSE;
        static public Z.IO.Touch                _TOUCH;

        
        /// <summary>
        /// Initialize my game global managers from game device
        /// </summary>
        static public void Initialize( ZGDK.GameDevice pDevice )
        {

            //*>> Handles content managers from game device.            
            _FontMngr           = pDevice.Content.FontContentMngr;
            _TextureMngr        = pDevice.Content.TextureContentMngr;
            _EffectMngr         = pDevice.Content.CustomEffectContentMngr;
            _MeshMngr           = pDevice.Content.MeshContentMngr;
            _SoundMngr          = pDevice.Content.SoundContentMngr;

            //*>> Handles 2D entity managers from game device.
            _LayerMngr          = pDevice.D2DMngr.ScreenMngr.LayerMngr;
            _ScreenMngr         = pDevice.D2DMngr.ScreenEntityMngr;
            _ScreenSpriteMngr   = pDevice.D2DMngr.ScreenMngr.SpriteEntityMngr;
            _TileMngr           = pDevice.D2DMngr.TileMngr;
            _TileSpriteMngr     = pDevice.D2DMngr.TileSpriteMngr;
            _GUIMngr            = pDevice.D2DMngr.GUIEntityMngr;

            //*>> 3D Entity managers handles from game device.            
            _WorldMngr          = pDevice.D3DMngr.WorldMngr;
            _ModelMngr          = pDevice.D3DMngr.ModelEntityMngr;
            _BillboardMngr      = pDevice.D3DMngr.BillboardEntityMngr;
            _ParticleMngr       = pDevice.D3DMngr.ParticleEntityMngr;
            _QuadMngr           = pDevice.D3DMngr.QuadEntityMngr;            
            _TrailMngr          = pDevice.D3DMngr.TrialEntityMngr;
            _LineMngr           = pDevice.D3DMngr.LineEntityMngr;

            //*>> Handle input controller from game device.
            _KEYB               = pDevice.InputMngr.Keyboard;
            _MOUSE              = pDevice.InputMngr.Mouse;
            _TOUCH              = pDevice.InputMngr.Touch;

        }//EndMethod

      
}//EndNameSpace
1 Like

This is actually how i kinda do it most of the time as well.
I keep a static ref as dexter shows above, to the Window, Game, GraphicsDeviceManager, GraphicsDevice, SpriteBatch, the current Font, I keep the other stuff separate with overloads to initialize it or not.

This is really because im lazy (100%).
It makes setting up for testing things much faster, but i doubt its actually better then any of the other choices.
In fact it is probably a trade off of losing just a little speed if the static class gets to big.
I actually use this as a sort of intermediate way to get references all from the same spot.

There are a couple of dangers to doing the above that should be mentioned.

Any were in game1 when a device reset occurs or something you hold there gets new-ed in some strange way. It can under certain cases cause these static refs to become, what one might think of as, dangling references (as far as your game is concerned) or out of date refs (suffice it to leave it at that)
.
You should at the least, assign / re-assign the reference to this static class in the applications main load method, such as game1 load(). This is so when a reset occurs a re- load occurs and it will reassign all these references to be current. Otherwise you can end up with a really hard to track down un-obvious bug or even a null reference exception at what may appear seeming obscure random times.

1 Like

I pass mine around to each class, but I agree. If you’re going the static route, at least use a static singleton instance rather than a static class in case you need to reset, etc.

Guys, using the singleton is bad, as the Game class exposes something known as Services. This stack overflow question explains it all. As you may see, The ContentManager is already exposed as a service in the game. Personnally, I expose the Content as a shortcut property of the Game class. Problem solved.

I was just speaking in the case of the MonoGame framework that already provides a pattern for accessing services.

I also use the singleton pattern in some other projects (non MonoGame related, mainly java) and I’m fine with it.

Im just kinda wondering why no one commented on my example at the top. That is runnable. In case no one got the point.The output is all done from the Game2 class but that could be any class.

Two content managers are used to load 2 textures in the Game2 class
One Is game1 original content manager which is called to From game2.
Not passed to game2.

The other is local to the class instance and could be used to load or unload levels while game1’s content manager persists all game.

agirl = Game1.GetGame1sContentManager().Load<Texture2D>("agirl");

The other is created in the class and can be unloaded and reloaded.

aguy = content.Load<Texture2D>("aguy");

game1s content manager isn’t passed into game2 it is returned from game1 as a call to game1 from game2. Because it is returned by value reference the texture2d will point back to game1’s contentmanager you use it then it pops off the call stack.

= Game1.GetGame1sContentManager().Load<Texture2D>("agirl");

public static ContentManager GetGame1sContentManager()
        {
            return content;
        }

We passout thru a pair of static game1 methods but they aren’t the same.

Again the original game1 content manager getter (It’s that simple).

public static ContentManager GetGame1sContentManager()
{
    return content;
}

Or a new content manager that is a copy of the original in local scope which gets assigned to game2’s own personal content manager when you call it.

Callable from game2 this is a game1 method also,

public static ContentManager GetNewContentManagerInstance()
{
      // create a new content manager instance
      ContentManager temp = new ContentManager(content.ServiceProvider, content.RootDirectory);
      temp.RootDirectory = "Content";
      return temp;
}

in game2

public class Game2_minimode
{
    private ContentManager content;
    public Texture2D agirl;
    public Texture2D aguy;
    public Game2_minimode()
    {
        // this mode now has its own New content manager.
        content = Game1.GetNewContentManagerInstance();

Which allows the below to actually happen in game2’s load method…

agirl = Game1.GetGame1sContentManager().Load<Texture2D>("agirl");
aguy = content.Load<Texture2D>("aguy"); // use my new local manager

as shown below in output they are loaded from the above lines.
output…

agirl disposed ? False
aguy disposed ? False

We can unload the local manager just to prove we wont unload game1’s content loaded textures that we called to load the girl texture.

unloading game2

// game1’s content still alive.

agirl disposed ? False

// game2’s content managers texture got unloaded with the local manager

aguy disposed ? True

Calling content unload only unloads this classes content not what was loaded in game 1
You could do that when you exit the class as a mode ect and load it when you come back.
In which case only one texture would actually be loaded the other would just be re reference copied in place,

So basically you can just call to game1 get a permanent app long singular content manager. Or get a temporary one from game1 and assign it or one for each mode or both. You could even put a bunch of them in a static list of managers.

Its basically all possible its just what you can handle keeping track of and how much you need.

Passing the ContentManager to every class feels wrong but this is the only way to do it… Game1.cs gets the Content var from Game.cs where Content is a non-static ContentManager , which means… It can not be used anywhere(except the classes with Game as their parent class,eg. Game1.cs) in a static manner… which means you will have to specify a variable with type of Game1 and put it as the active Game1 class,which will go even more complex and I also don’t know how to do it :sweat_smile: … If you don’t want to have a variable with type of Game1 and use its variables,then those variables should be static… A non static var can’t be passed in this manner… But some vars can be static and used anywhere like you are saying. Eg. Using it with SpriteBatch->
in Game1.cs
public static SpriteBatch spriteBatch // Changed from SpriteBatch spriteBatch
in SecondClass.cs
Game1.spriteBatch.Draw(..............)
Here you need not pass SpriteBatch as an argument.

I have just tested this code and it seems to be working… If I am wrong,please correct me :slight_smile:

Hope it helps you!

and sorry to wake this thread up after 300 days!!

Edit:
I have found a way to overcome this problem!
In Game1.cs
using Microsoft.Xna.Framework.Content; ...........//Other things public static ContentManager content; ........//Other Things void Initialize() { .....//Other Things content = Content;

What does it do?
It will make a static variable ‘content’ which stores the value of Content…
Content is non static and can’t be passed without object ref.
content is static and can be passed without object ref.

Which means, when you have to load your textures or something else…
Use
Game1.content.Load<Type>("Path");
That is it.
Code Tested and working!!!

Just thought I might toss another alternative solution in here. Cus obviously this very active topic needs even more of that, right?

So, defining some limitations… If you’re concerned about constructors growing too large as dependencies inflate on your objects, and you don’t like using a static content manager, and you don’t want to pass in a Texture2D or handle the loading outside of the class in question…

Consider using IServiceProvider. Make a new interface for the content loader, wrap the content loader, implement the interface, use it in the IServiceProvider class provided by Monogame.

In Game1, probably in initialization:

Services.AddService(typeof(IContentLoader), Content);

Then you pass ‘Services’ around to whatever class needs the services. In their constructor:

this.Content = (IContentLoader)services.GetService(typeof(IContentLoader));

True, you now are passing around something in every class constructor, same as before, but with the added advantage that you can also put other common services in there which you might have used static classes for, like input handling, graphics manager, or other things. It’s convenient, and it maintains abstraction since you’re using interfaces, so you can still do testing and mocking and all that kind of stuff.

Another alternative…

A combination of 2 other methods mentioned above. In your Initialization() or LoadContent() method on each class, include a ContentManager parameter. Pass in the manager via method injection this way. The benefit of this, over using the constructor, is that you can delay dealing with the ContentManager until you want to later. You can also implement an interface which exposes a ‘LoadContent(ContentManager)’ on every object which needs to do content loading. This would allow you to put all such objects in a large list together, and you could then iterate over the entire list at one time and run LoadContent() on them all at one time. This would be beneficial for locality of reference, and it might be a little easier to manage depending on how and when you load your entities in your game world.

Alternative.B

Also mentioned above is to include a string to the path of the texture in the constructor. You could expose that string as a public property on the object, call it TextureSource, whatever. Do the same as above with LoadContent method, but rather then passing in the ContentManager, you simply grab the TextureSource string and grab the texture outside the object (probably also in the middle of a iterator), then you can pass in the texture to the object in either a public method or property. This is a less ‘safe’ option, in the sense that you’re making the texture mutable after initialization. But that could also be a strength, possibly, if you want to play with texture swapping at runtime as a tool for doing some cool behavior.

Alternative.C

Instead of a string with the source of the texture xnb file, ensure every object gets an ID of some kind. The ID could be a simple string, a custom data class, a GUID, or just an int. Somewhere in the bowels of your game, have a repository which contains a lookup table for entities and their textures. So for example, ID “ent_lv3_critter” refers to a very specific creature you’ve defined already, possibly in a text file somewhere. In your lookup table(a simple dictionary is fine), you’d have a key value pair of <string, string>, where the key is the ID of the entity, and the value is the filepath to its texture file.

Now when you’re loading content for all your entities, you just peek at the creatures set ID, glance at the lookup table, grab the file source, load it into the ContentManager, pass the entity its new texture file.

You could also use serialization in conjunction with this. Deserialize your critter object from file, and have the file source included in the file definitions. Then when the object is initialized in your game it already has the source file ready to be read.

This method also gives you the ability to use a default in case of an oopsie(well, any of these would allow that actually). Put the Content.Load<Texture2D> in a try/catch statement and catch any loading errors(such as a bad file source). If this happens, you can now load a default texture for that entity. Like a big, pink and black checkerbox. Something obvious so when you get in-game and see bouncing pink boxes everywhere you know something went wrong with the texture loading.

Insane Variant:

Or if you REALLY hate passing anything into the class at all, even its own texture file, do all the rendering outside the class. Make a lookup table for Texture2D(as value) and tie it to a key(the entities ID). Grab all your entities and iterate through them, and for each you grab ‘DestinationRectangle’ from the entity, or maybe a single data struct like ‘RenderingContext’ which contains all the rendering information for that entity, and then draw the entity from outside the class entirely.

I’ve been awake for a really long time. I should stop typing and go to bed.

I have a AssetManager class that I set up as a service, It has multiple ContentManagers. I can request an asset from it, and it will go through each content manager it knows and return me an asset if it can, if not, it can return an alternate, or just null.

This is handy when I have a game that is separate from my my engine and/or editor.