Setting up keyboard bindings using JSON and reflection

My game uses configuration files in JSON format. One of them is used for setting up control bindings, and it looks like this:

{
  "DisplaySettings": {
    "IsFullScreen": false,
    "PreferredBackBufferWidth": 800,
    "PreferredBackBufferHeight": 600
  },
  "GamePadSettings": {
    "Back": "Exit",
    "DPadDown": "MoveDown",
    "DPadLeft": "MoveLeft",
    "DPadRight": "MoveRight",
    "DPadUp": "MoveUp"
  },
  "KeyboardSettings": {
    "Escape": "Exit",
    "S": "MoveDown",
    "A": "MoveLeft",
    "D": "MoveRight",
    "W": "MoveUp"
  }
}

In the Main method of the Program class (my composition root), I read the file and hand it off to the Input class:

private static class Program
{
    private static void Main()
    {
        IFileHandler fileHandler = new JsonHandler();

        var settings = fileHandler.Read( "settings.json" );
        var level = fileHandler.Read( "level.json" );
        var player = fileHandler.Read( "player.json" );

        using ( var game = new MyGame( new Graphics( settings.DisplaySettings, level, player),
                                       new Input( settings.GamePadSettings, settings.KeyboardSettings ) ) )
        {
            game.Run();
            fileHandler.Write( "player.json", player );
            fileHandler.Write( "level.json", level );
        }
    }
}

fileHandler.Read returns one of these (these are default values):

```
public class Settings
{
public DisplaySettings DisplaySettings = new DisplaySettings();
public GamePadSettings GamePadSettings = new GamePadSettings();
public KeyboardSettings KeyboardSettings = new KeyboardSettings();
}
public class DisplaySettings
{
public bool IsFullScreen = false;
public int PreferredBackBufferWidth = 800;
public int PreferredBackBufferHeight = 600;
}
public class GamePadSettings
{
public string Back = "Exit";
public string DPadDown = "MoveDown";
public string DPadLeft = "MoveLeft";
public string DPadRight = "MoveRight

I'm not sure about binding directly to methods based on a configuration file like this, because it means you have to be very careful about what methods does your Game type expose (assuming the player has access to the configuration file, which it probably does).

Consider adding a level of abstraction, either something like PlayerActions class, with methods that would just call the real methods, or maybe a Dictionary.

Also, I think player inputs should be handled by some kind of player object, not the game object directly. Otherwise, things like split-screen would be hard to implement.

Some specific notes:

-
Public fields are usually discouraged. But the syntax for properties with default values is quite verbose. And it would be nice to make the types immutable by using private setters and constructors, but that could cause issues with deserialization. So in the end, I think public fields are okay here.

-
Think about using dictionaries for the two input settings. That would allow you to avoid repetition in calling AddXxxBinding. Though it would also make setting the defaults less nice, so I'm not sure this would be actually better.

-
I think it would make sense to save Game into a field of Input. That way, you don't have to pass it as a parameter all the time.

-
There are several changes I would make to the AddButtonBinding() and AddKeyBinding() methods:

-
The methods are almost the same. I would combine them into one generic method.

-
Should bad method name really cause no action? I would at least log it somewhere, if you don't want to crash completely.

-
You can use Delegate.CreateDelegate() instead of MethodInfo.Invoke and a lambda.

With those changes, the method would look something like:

private void AddBinding(
    Game game, Dictionary bindings, TInput input, string methodName)
{
    var method = FindMethod(game, methodName);

    if (method == null)
        throw new InvalidOperationException();

    bindings.Add(input, (Action)Delegate.CreateDelegate(typeof(Action), game, method));
}

-
Your FindMethod() searches for static methods too, but you're invoking the found method as an instance method. Because of that, I would remove BindingFlags.Static.

Also, FindMethod() doesn't have much to do with input, it probably belongs to a separate class.

StackExchange Code Review Q#42232, answer score: 3