Configuration store

[aoki-marika]

Adds CConfigurationStore, used for accessing database-backed settings.

This PR only adds the infrastructure, it does not replace CConfigIni and does not function for the end user. Doing everything in one go makes it near impossible to review, so instead this sets the foundation for future PRs to build upon where they can then migrate individual components to this new system.

For quick reference, the general workflow for using settings is as such:

• Add a new setting to CSetting:

/// <summary>
/// ...
/// </summary>
public static readonly CIntSetting MySetting = new CIntSetting(ECategory.System, @"MySetting");

• Add a default value to CConfigurationStore.tInitialiseDefaults():

tSet(CSetting.MySetting, 0, bReplaceValue: false);

• Use the setting:

using (var configuration = new CConfigurationStore())
{
    int nMySetting = configuration.tGet(CSetting.MySetting);
    configuration.tSet(CSetting.MySetting, nMySetting + 1);
}

[ericpignet]

Hi @aoki-marika ,

Thanks for the contribution,
A few remarks for me. Some are similar to the ones I made in issue #35 , I think we should clarify this topic and then progress.

• I'm not sure what type of settings we plan to store here. That would be good to clarify before we add dependencies. If the goal is to eventually migrate stuff from Config.ini, there should be a way for the end-users to see and update the settings. I'm not saying it should be all coded before we start putting the framework in place, but we should at least have some early design.
  For example, if we decide not to implement an import from ini file, we will need all settings to be configurable from the game. But this means we also need an easier way to add a config option in the game as it's a hassle today. For example we could configure when creating the setting where if it is displayed in the UI or not, where in the menu it is displayed, what is the description etc.

• I think that if we want to introduce a SQLite storage, it should be designed to be per-user, and per-profile, with possibly in the future a way to associate a profile to a box.def folder, so that some of the settings can be adjusted to the Gitadora version.
  I understand it can be added in the future, but it may be tricky to update data model after it's used by many users which use different DTXMania NX versions. So I would prefer if we discuss and choose a datamodel which will be ready for upcoming evolutions.

• If this configuration store is used as extensively as CConfigIni at the moment, it needs to be super efficient. CConfigIni settings are used in the game loop (tUpdateAndDraw) which should never take more than a few ms to not impact framerate. Even if the SQLite lib has internal caching (I don't know if it's the case), I'm concerned about doing an SQL query for each read and update. We shouldn't leave it to the consumer code to have its own caching, it should be part of the framework.
  Note that we have some caching already in DTXMania using class binary serialization/deserialization.

[aoki-marika]

I definitely agree with a lot of your points, with hindsight and more recent experience with SQLite I can see a few oversights I made here.

First point

I intended this to be used exclusively for in-game configurable settings. There shouldn't be any hidden "settings" which are only used by the game present, like CConfigIni's Version and window frame. Many of the settings available within Config.ini simply do not have a place in the interface. Personally I would like to just cull all of them, but this isn't exactly an option here, so I think it should be kept around for these old settings. DTXPath and PolyphonicSounds are good examples of such cases. With this as the basis, I was also intending it to be a sort of "fresh start" for settings. Many settings are currently fragmented and confusing, such as switching between CLASSIC and XG modes being several settings that "sort of do the same thing but not really but effectively", which could be made into a single version setting or something like that. So my plan was less of a new version of CConfigIni but more of a redesign of it.

On the topic of the interface, it wasn't really much of a thought at the time, but it definitely should've been. ISetting<T> would probably be a good place to put this as it can then also be shared between the quick settings and settings screen. Perhaps something like the below:

new CIntSetting(ECategory.System, @"MySetting")
{
    strName = "My Integer Setting",
    strDescription = "My integer setting which changes something arbitrary.",
    nMinValue = 0,
    nMaxValue = 10,
    nDefault = 0,
};

However for CEnumSetting this becomes a bit more complicated as I would like it to display a description per-case. Maybe something like this:

new CEnumSetting<EMyEnum>(ECategory.System, @"MyEnumSetting")
{
    strName = "My Enumerated Setting",
    strDescription = "My enumerated setting which changes something arbitrary.",
    strValueDescriptions = new Dictionary<EMyEnum, string>()
    {
        { EMyEnum.One: "Show 1" },
        { EMyEnum.Two: "Show 2" },
        { EMyEnum.Three: "Show 3" },
    },
    eDefault = EMyEnum.One,
};

Unsure on that one though, that's just off the top of my head. Localisation may also be something to consider, but I don't know how that is currently being handled by other parts of the codebase.

I'm unsure on what you specifically mean by "where in the menu it is displayed". If you mean by the position within the list that it is placed, then I don't think ISetting<T> is an appropriate place to define this. If you instead mean the category it is displayed in, as in the list of "System", "Drums", "Guitar", and "Bass" on the side of the current settings screen, then this is already available via ISetting<T>.eCategory. I hadn't looked at the current settings screen code, but I have no doubt it's awful to work with haha. I was definitely intending to rewrite it for this, where it will be much easier to add new settings to it.

Second point

I'm unsure what you mean by "per-user" and "per-profile"; are these not the same thing? Also "associate a profile to a box.def folder", again unsure. To clarify, I consider profiles to be a single player on the same machine, such as two friends both playing on the same setup. Are you perhaps referring to something else?

Game storage should, in my opinion, definitely not be per-profile. There should be a single database with a Profiles table which other profile-specific table's rows then associate themselves with, such as Settings gaining a ProfileID column, which was the original plan. There are many benefits to keeping everything in one database such as a song store being able to operate independently of profiles, with scores then linking to songs and profiles, without any ugly cross-database queries.

I intended to move this to a more generic storage class in the profiles PR, but there's no reason not to implement it here. My current plan is to have a CDatabaseContext class which contains a single connection to the backing SQLite database, which are created by CDatabaseContextFactory. It would go something like this:

public class CDatabaseContext : IDisposable
{
    private readonly SQLiteConnection connection;

    public CDatabaseContext(string strConnection)
    {
        connection = new SQLiteConnection(strConnection);
        connection.Open();
    }

    public void Dispose()
    {
        connection?.Close();
        connection?.Dispose();
        connection = null;
    }

    // execute, query, etc. methods...
}

public class CDatabaseContextFactory
{
    private const string str_filename = @"game.db";

    public CDatabaseContext tGetContext() => new CDatabaseContext($@"Data Source={str_filename};Version=3;");
}

public class CConfigurationStore
{
    private readonly CDatabaseContextFactory contextFactory;

    public CConfigurationStore(CDatabaseContextFactory contextFactory)
    {
        this.contextFactory = contextFactory;
    }

    public T tGet<T>(ISetting<T> setting)
    {
        using (var context = contextFactory.tGetContext())
        {
            ...
        }
    }
}

CDatabaseContext allows more elegant short-lived connections, CDatabaseContextFactory could later be extended with things such as locking a database mutex for better thread-safe writes, and stores can further themselves from relying on SQLite directly.

Third point

Very much agreed, I should have done some profiling to ensure it's up to snuff. I'll run some tests after the other points are addressed and optimise accordingly. Caching settings is a very simple thing to implement so I'd say it's fine to leave it until then, no point optimising the current code when it seems it will receive a lot of changes.

---

Sorry for the long write-up, just wanted to lay everything out so we could come to conclusions on where this should go. Also I appreciate you taking time out to review, I know this is a busy time of year for everyone.