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

Hey everyone,

I’m working on something simple to get me a better understanding of MonoGame and C# in general.

For my game, I have a Player, Projectile and Enemy class. They all have a Texture2D which I load using Game1’s ContentManager. I pass this class through each class’s constructor. This feels inefficient on the long run.

So my question is, is it truly inefficient? And if it is, what is an alternative way to, for instance, access Game1’s ContentManager from other classes?

1 Like

You can always just pass in the texture instead if you want, but no it’s not wrong it’s the whole point of encapsulation so don’t worry.

If it’s a real pain though you can always make a contentmaniger in your games and make it static. Then set it to the same content as the one in your game.

Public static ContentManiger Mycontent;

Mycontent = Content;

All you would have to do is then call

Game1.Mycontent

1 Like

Thank you for your answer. I already considered passing a Texture2D as an argument, but for large projects, this would result in a huge list of textures to be loaded and declared.

Making a static variable for the ContentManager sounds good, and I will be sure to give it a try!

In fact it is passed by reference for this type of object, and not by copy, so don’t worry :wink:
Textures, RenderTargets etc work the same:

A class instance is a reference type, not a value type. When a reference type is passed by value to a method, the method receives a copy of the reference to the class instance. That is, the method receives a copy of the address of the instance, not a copy of the instance itself.

from https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/how-to-know-the-difference-passing-a-struct-and-passing-a-class-to-a-method

I am aware of this, but it still feels not right. Even passing the ContentManager if it’s a reference. When working on larger projects with a lot of classes that require the same arguments looks messy to me

Just thought of a something that may be more elegant.

You could have a base class

    Public class base
    {
      protected static ContentManiger Content;

    Public static void SetContent(ContentManiger _content)
    {
    Content = _content;
    }
    }

Then you inherit

Public myclass: base
{}

All you would then need to do is
Base.setcontent

And then call Content.Load to load a texture from each child class

PS sorry if formatting is bad I’m replying from my phone

1 Like

That’s what you do when you create a GameComponent or DrawableGameComponent.
Finally, when you use them to draw many post process effects, you end up with 9 or 10 or even more: Depth of field, ToneMapping, LensFlare, Bloom, SSAO, etc… All need a Content, a Device etc.

Another option would be to load data in a “central” method with one Content, and pass the needed objects to each function… Which can become less elegant and can add complexity if you create structs or classes to pass them to a method, as with a DrawableGameComponent to follow my example, would need a graphics device, a spritebatch ?, say… 4 textures, and many parameters for the shader effect.

Passing a content, makes you and others, know it is required, and will be used somewhere, instead of scratching their head, wondering why their model isn’t loaded etc. because the reference is hidden in a constructor.

This is not very intuitive since the manager is static (e.g. there’s only one variable holding a reference) whereas the base-class design communicates that you can pass different references to each of them (but they just end up overwriting the single static manager reference). So it would be better not to make it static.

You’d have multiple contentManagers when you want to unload the content selectively. You can only unload a whole contentManager at once. So you’d end up with one that holds permanent textures and stuff (gets never unloaded) and one for a level, that you unload if you change level, for example (that’s the way I did it).
And I made those static (a kind of singleton) by having my own static class containing those contentmanagers, getting them passed once on game construction…
http://csharpindepth.com/Articles/General/Singleton.aspx

Ill be honest i never use more then one. I pass everything but the content manager most of the time or i keep refs to it. If i were to pass it id keep a ref to it in the class i passed it to.

This seemed to work though at least it seemed to unload only the one manager not the other when i tested it.

Though i dunno about the wisdom of calling game1’s content manager from 2.
Just to show its all passable and you can keep refs around.

This example is actually super bad.

You need to be real careful, to if you start tossing them all over, its real easy to fuxer up two refs and call to a disposed object and then kapow to desktop or worse get into a loop were you loose a reference to a content manager that cant be disposed and fill up the video memory.

output

agirl disposed ? False
aguy disposed ? False
unloading game2
agirl disposed ? False
aguy disposed ? True

this would need to be in other classes
using Microsoft.Xna.Framework.Content;

using Microsoft.Xna.Framework;
using Microsoft.Xna.Framework.Content;
using Microsoft.Xna.Framework.Graphics;
using Microsoft.Xna.Framework.Input;
using System;

namespace Game1
{


    public class Game2_minimode
    {
        private ContentManager content;
        public Texture2D agirl;
        public Texture2D aguy;
        public Game2_minimode()
        {
            // this mode now has its own content manager.
            content = Game1.GetNewContentManagerInstance();
        }
        public void load()
        {
            // we load from two different managers not really a good idea but..
            agirl = Game1.GetGame1sContentManager().Load<Texture2D>("agirl");
            aguy = content.Load<Texture2D>("aguy");
        }
        public void unload()
        {
            content.Unload();
        }
    }

    public class Game1 : Game
    {
        GraphicsDeviceManager graphics;
        SpriteBatch spriteBatch;

        Game2_minimode game2;
        public static ContentManager content;

        public Game1()
        {
            graphics = new GraphicsDeviceManager(this);
            Content.RootDirectory = "Content";
            content = Content;
            game2 = new Game2_minimode();
        }
        public static ContentManager GetGame1sContentManager()
        {
            return content;
        }
        public static ContentManager GetNewContentManagerInstance()
        {
            // create a new content manager instance
            ContentManager temp = new ContentManager(content.ServiceProvider, content.RootDirectory);
            temp.RootDirectory = "Content";
            return temp;
        }
        
        protected override void Initialize() {base.Initialize();}
        
        Texture2D girl;
        protected override void LoadContent()
        {
            spriteBatch = new SpriteBatch(GraphicsDevice);

            girl = Content.Load<Texture2D>("agirl");
            game2.load();
            Console.WriteLine(game2.agirl.Name + " disposed ? " + game2.agirl.IsDisposed);
            Console.WriteLine(game2.aguy.Name + " disposed ? " + game2.aguy.IsDisposed);
            game2.unload();
            Console.WriteLine("unloading game2");
            Console.WriteLine(game2.agirl.Name + " disposed ? " + game2.agirl.IsDisposed);
            // this shouldnt work now
            Console.WriteLine(game2.aguy.Name +" disposed ? "+ game2.aguy.IsDisposed);
        }

        protected override void UnloadContent()
        {
        }

        protected override void Update(GameTime gameTime)
        {
            if (GamePad.GetState(PlayerIndex.One).Buttons.Back == ButtonState.Pressed || Keyboard.GetState().IsKeyDown(Keys.Escape))
                Exit();
            base.Update(gameTime);
        }

        protected override void Draw(GameTime gameTime)
        {
            GraphicsDevice.Clear(Color.CornflowerBlue);
            base.Draw(gameTime);
        }
    }

}

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.