HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

GameState Management efficiency

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
managementefficiencygamestate

Problem

So I've read numerous tutorials on handling game states in XNA, and to each tutorial I've been read, it was different from the others. Now I want to ask which way is more efficient? I've created my own ScreenManager and what not, but it wasn't satisfying to me because it was inefficient since I didn't unload any unused content.

My question is: What is an efficient way to manage your screens and its content? Is using an enum fine? Or...?

Here's my current code:

```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using Microsoft.Xna.Framework;
using Microsoft.Xna.Framework.Content;
using Microsoft.Xna.Framework.Graphics;

namespace Menu
{
public class ScreenManager
{
// Variables
List gameScreens = new List();

// Constructors
public ScreenManager()
{
gameScreens.Add(new Splashscreen());
gameScreens.Add(new Mainscreen());
gameScreens.Add(new Playscreen());
gameScreens.Add(new Pausescreen());
}

// Public Properties
public List Screens
{
get { return gameScreens; }
set { gameScreens = value; }
}

// Public Methods
public void Add(GameScreen screen)
{
gameScreens.Add(screen);
}

public void LoadContent(ContentManager Content)
{
foreach (GameScreen screen in gameScreens)
{
screen.LoadContent(Content);
}
}

public void Update(GameTime gameTime)
{
foreach (GameScreen screen in gameScreens)
{
screen.Update(gameTime);
}
}

public void Draw(SpriteBatch spriteBatch)
{
switch (ScreenState.Instance.CurrentState)
{
case State.Splashscreen:
gameScreens[0].Draw(spriteBatch);
break;

Solution

I can't be sure of this (I need to foster my XNA knowledge), but I would probably derive that class from DrawableGameComponent so that the Update and Draw methods would be overrides (not just methods that happen to be named "Update" and "Draw"), but deriving from that class might have other consequences that I'm not aware of; again, my XNA is fairly basic - it just seems natural to me for a class that has Update and a Draw methods, to derive from DrawableGameComponent: these method names are not an accident.
Update

Your Update() method is looping through each GameScreen and calling Update() on them all:

foreach (GameScreen screen in gameScreens)
{
    screen.Update(gameTime);
}


I think doing this incurs non-necessary processing: why would you bother updating the splash screen and pause screen at every single iteration? You're wasting cycles here.
Draw

Your Draw() method is more selective:

public void Draw(SpriteBatch spriteBatch)
{
    switch (ScreenState.Instance.CurrentState)
    {
        case State.Splashscreen:
            gameScreens[0].Draw(spriteBatch);
            break;
        case State.Menu:
            gameScreens[1].Draw(spriteBatch);
            break;
        case State.Play:
            gameScreens[2].Draw(spriteBatch);
            break;
        case State.Pause:
            gameScreens[3].Draw(spriteBatch);
            break;
    }
}


I like that you're using an enum here, but I don't like that you've hard-coded the gameScreens indices. Let's see.. you're storing the screens in a List, here:

List gameScreens = new List();

public ScreenManager()
{
    gameScreens.Add(new Splashscreen());
    gameScreens.Add(new Mainscreen());
    gameScreens.Add(new Playscreen());
    gameScreens.Add(new Pausescreen());
}


Allow me to open a nitpicking parenthesis here:

Nitpick

Instead of List gameScreens = new List(); and then adding the screens in the list at construction, I would have used an interface here, and initialized everything in a single statement:

private readonly IList _gameScreens;

public ScreenManager()
{
    _gameScreens = new List 
                           {
                               new SplashScreen(),
                               new MainScreen(),
                               new PlayScreen(),
                               new PauseScreen()
                           };
}


This allows the _gameScreens private field to be readonly, which prevents the list's reference to be tampered with.

The mechanics work, but could work better if you leveraged IDictionary and turned List gameScreens into a IDictionary>:

private readonly IDictionary> _gameScreenRenderer;
private readonly GameScreen _splashScreen = new SplashScreen();
private readonly GameScreen _mainScreen = new MainScreen();
private readonly GameScreen _playScreen = new PlayScreen();
private readonly GameScreen _pauseScreen = new PauseScreen();

public ScreenManager()
{
    _gameScreenRenderer = new Dictionary>
                            {
                                { State.SplashScreen, sb => _splashScreen.Draw(sb) },
                                { State.MainScreen, sb => _mainScreen.Draw(sb) },
                                { State.PlayScreen, sb => _playScreen.Draw(sb) },
                                { State.PauseScreen, sb => _pauseScreen.Draw(sb) }
                            };
}


An Action is a delegate that points to any method that returns void and takes a SpriteBatch parameter; here the method is anonymous.

Each GameScreen becomes a field of the ScreenManager class, there's no need to have them in a list of any kind. Doing this turns your Draw implementation into something like this:

private void Draw(SpriteBatch spriteBatch)
{
    _gameScreenRenderer[ScreenState.Instance.CurrentState](spriteBatch);
}


Implement a similar mechanism for your Update method and you'll only be updating and rendering the screen for your CurrentState.

Code Snippets

foreach (GameScreen screen in gameScreens)
{
    screen.Update(gameTime);
}
public void Draw(SpriteBatch spriteBatch)
{
    switch (ScreenState.Instance.CurrentState)
    {
        case State.Splashscreen:
            gameScreens[0].Draw(spriteBatch);
            break;
        case State.Menu:
            gameScreens[1].Draw(spriteBatch);
            break;
        case State.Play:
            gameScreens[2].Draw(spriteBatch);
            break;
        case State.Pause:
            gameScreens[3].Draw(spriteBatch);
            break;
    }
}
List<GameScreen> gameScreens = new List<GameScreen>();

public ScreenManager()
{
    gameScreens.Add(new Splashscreen());
    gameScreens.Add(new Mainscreen());
    gameScreens.Add(new Playscreen());
    gameScreens.Add(new Pausescreen());
}
private readonly IList<GameScreen> _gameScreens;

public ScreenManager()
{
    _gameScreens = new List<GameScreen> 
                           {
                               new SplashScreen(),
                               new MainScreen(),
                               new PlayScreen(),
                               new PauseScreen()
                           };
}
private readonly IDictionary<State, Action<SpriteBatch>> _gameScreenRenderer;
private readonly GameScreen _splashScreen = new SplashScreen();
private readonly GameScreen _mainScreen = new MainScreen();
private readonly GameScreen _playScreen = new PlayScreen();
private readonly GameScreen _pauseScreen = new PauseScreen();

public ScreenManager()
{
    _gameScreenRenderer = new Dictionary<State, Action<SpriteBatch>>
                            {
                                { State.SplashScreen, sb => _splashScreen.Draw(sb) },
                                { State.MainScreen, sb => _mainScreen.Draw(sb) },
                                { State.PlayScreen, sb => _playScreen.Draw(sb) },
                                { State.PauseScreen, sb => _pauseScreen.Draw(sb) }
                            };
}

Context

StackExchange Code Review Q#38232, answer score: 2

Revisions (0)

No revisions yet.