Fix/openmw-config

[magicaldave]

Addresses issues with openmw config parsing discovered by myself and another user in the OpenMW discord that were trying to switch to tes3merge. Notes from #MWSE copied here:

QQ: I have two minor grievances with openmw implementation I would like to change, but idk if you would be okay with this:
It makes more sense to me that TES3Merge.exe would be in the same directory as openmw.cfg, not a child directory. In this manner, you keep tes3merge and its config adjacent to openmw.cfg, OR you simply change directory to one which contains openmw.cfg.
If in an openmw installation is detected, OpenMW proper has its own overwrite directory, referred to as data-local in openmw.cfg. This has default locations you can rely on just like openmw.cfg itself (but it's important to note that it's a child directory of the openmw.cfg dir on Windows and NOT unices). So, it'd be nice if perhaps we could explore using that as the install dir, since... I think that's probably what will end up happening if you run tes3merge under MO2 for a vanilla install, just by wildly different means.

Less important, don't actually care:

The class constructor for OpenMWInstallation never actually appears to use this function which returns the hardcoded system path, so.... I'm not really sure what to do about that but it seems like it's worth removing since it's not really how TES3Merge handles detection anyway.

[magicaldave]

Okay, I went ahead and just did the whole thing.
tl;dr I looked over the openmw.cfg spec at https://openmw.readthedocs.io/en/latest/reference/modding/paths.html at I think tes3merge should be fully compliant now (in all the ways it would care about, at least).

1. Quoted paths are handled according to how the engine does it
2. Nested configuration directories specified in the openmw.cfg are now loaded via recursion once those keys are encountered. Technically this is probably wrong since it would mean fallback-entries can be parsed in an incorrect order but tes3merge will never care about that.
3. Relative paths for data directories are now supported too.
4. The data-local directory is now supported too (this is basically the built-in version of mo2's overwrite folder), if the data-local key is specified by any of the openmw.cfg files loaded by TES3Merge. So it's not added implicitly, but it could be if the user went out of their way to point TES3Merge at the OpenMW install directory instead of whatever user openmw.cfg directory.

I tested this against my system openmw.cfg and a portable install of 0.48 I have set up.

Since @AnyOldName3 already chimed in here, he might as well again. Thanks boss(es?) <3

[AnyOldName3]

This isn't quite right. It stops at the first " without an & before it. Quite a few people have lines that look like data="path/to/mod A" # this is the path to mod A because they thought that they could use comments like that, but it's really just the bit of the path outside the quotes being ignored.

[magicaldave]

The docs said:

Everything after the closing quote mark will be ignored. 

Which I guess left a little more room for interpretation than I thought the first time I read it. What is the appropriate behavior if a path starts with a quote, but is not deemed to be quoted appropriately, does it just interpret, say, data="./some_dir as "./some_dir ?

[magicaldave]

71b1763 should address this

[AnyOldName3]

If it peeks a ", it does istream >> std::quoted(intermediate, '"', '&'). I just checked and that would interpret "path goes here as path goes here.

[magicaldave]

Hm. Fair. The original would have broken due to this but I think the current impl shouldn't have any problems in this respect.

[AnyOldName3]

This isn't a token. The default data-local path is ?userdata?/data.

[AnyOldName3]

There's some muddling with tokens. They can be used at the start of any path, so in principle, someone could have data=?userconfig?/mods/mod or something like that.

[magicaldave]

There's some muddling with tokens. They can be used at the start of any path, so in principle, someone could have data=?userconfig?/mods/mod or something like that.

I always keep forgetting that tokens can be generically used in this way. This maybe is worth documenting (especially that the token must be at the start, I was halfway through committing an incorrect version before re-reading that). However, your comment also leaves me guessing something that I think should be documented:

In the builtin configs, the token is used as ?userdata?data. But you said that someone could do ?userconfig?/mods

So:

1. Is there special handling for instances where a path separator is duplicated?
2. Are tokens actually assumed to include directory separators and this was perhaps a minor misremembering I'm overanalyzing? If ?userdata?data doesn't make it blow up, the token itself must include a path separator at the end, so I guess I've sort of maybe-answered this question in the course of writing it and am still unsure about duplicate separators.

EDIT: I decided it didn't really matter all that much what exactly OpenMW does here as long as the paths resolve properly so duplicate separators are always trimmed and data directories never have trailing separators.

[magicaldave]

I also keep worrying about this issue of recursion. As I understand it, if you were to do something like:

content=Morrowind.esm
content=Tribunal.esm
content=Bloodmoon.esm
config=?userconfig?

In an openmw.cfg, that would mean that the three content files defined here will be defined before any in the following config.

But, say the user does this:

content=Morrowind.esm
content=Tribunal.esm
config=?userconfig?
content=Bloodmoon.esm

Where the userconfig contains:

content=Whatever the UMOPP Merged Plugin is called.esp

Would the resulting list of content files be:

content=Morrowind.esm
content=Tribunal.esm
content=Whatever the UMOPP Merged Plugin is called.esp
content=Bloodmoon.esm

OR

content=Morrowind.esm
content=Tribunal.esm
content=Bloodmoon.esm
content=Whatever the UMOPP Merged Plugin is called.esp

?

EDIT:

Did some more testing and as of latest commit, it does seem to match up OpenMW's loading:

This was the result of maybe ten-plus configuration files running simultaneously:
configs.zip

So this should be fine, I think.

[AnyOldName3]

There's some muddling with tokens. They can be used at the start of any path, so in principle, someone could have data=?userconfig?/mods/mod or something like that.

I always keep forgetting that tokens can be generically used in this way. This maybe is worth documenting (especially that the token must be at the start, I was halfway through committing an incorrect version before re-reading that). However, your comment also leaves me guessing something that I think should be documented:

In the builtin configs, the token is used as ?userdata?data. But you said that someone could do ?userconfig?/mods

So:

1. Is there special handling for instances where a path separator is duplicated?

2. Are tokens actually assumed to include directory separators and this was perhaps a minor misremembering I'm overanalyzing? If `?userdata?data` doesn't make it blow up, the token itself **must** include a path separator at the end, so I guess I've sort of maybe-answered this question in the course of writing it and am still unsure about duplicate separators.

EDIT: I decided it didn't really matter all that much what exactly OpenMW does here as long as the paths resolve properly so duplicate separators are always trimmed and data directories never have trailing separators.

The tokens all have a trailing slash. OpenMW only runs on platforms that permit consecutive redundant path separators, so we don't need to think about this at all unless we're lexically comparing paths, in which case we'd need to normalise them anyway to get rid of things like path/./file. The documentation says Paths can be absolute, relative, or start with a token.

[AnyOldName3]

I also keep worrying about this issue of recursion. As I understand it, if you were to do something like:

content=Morrowind.esm
content=Tribunal.esm
content=Bloodmoon.esm
config=?userconfig?

In an openmw.cfg, that would mean that the three content files defined here will be defined before any in the following config.

But, say the user does this:

content=Morrowind.esm
content=Tribunal.esm
config=?userconfig?
content=Bloodmoon.esm

Where the userconfig contains:

content=Whatever the UMOPP Merged Plugin is called.esp

Would the resulting list of content files be:

content=Morrowind.esm
content=Tribunal.esm
content=Whatever the UMOPP Merged Plugin is called.esp
content=Bloodmoon.esm

OR

content=Morrowind.esm
content=Tribunal.esm
content=Bloodmoon.esm
content=Whatever the UMOPP Merged Plugin is called.esp

?

EDIT:

Did some more testing and as of latest commit, it does seem to match up OpenMW's loading:

This was the result of maybe ten-plus configuration files running simultaneously: configs.zip

So this should be fine, I think.

I thought this was fairly clear from the paragraph that says

If one openmw.cfg specifies multiple configuration directories, they’re all higher-priority than the one that specified them, and lower priority than any they go on to specify themselves. I.e. if dir1/openmw.cfg contains config=dir2 then config=dir3, and dir2/openmw.cfg contains config=dir4 the priority order will be dir1, dir2, dir3, dir4. This might be a surprise if you expect it to work like C’s #include directive.

It can't be higher priority if it ends up half way through the current file.

[AnyOldName3]

This looks way more complicated than it needs to be. It could be as simple as

if (dataDir.StartsWith('"'))
{
    auto original = dataDir;
    dataDir = "";
    for (int i = 1; i < original.Length; i++)
    {
        if (original[i] == '&')
            i++;
        else if (original[i] == '"')
            break;
        dataDir += original[i];
    }
}
}

[AnyOldName3]

This doesn't look right - you need to finish processing the rest of the current file before moving onto the next.

[magicaldave]

Okay, this is what that really long-winded comment about sequencing was in reference to. Latest commit should address this new set of comments.

[AnyOldName3]

This probably wants ParseDataDirectory, too.

[magicaldave]

Heh, yeah - I just noticed the override config I'm using wasn't writing to the data-local dir assigned for this exact reason. Current commit handles this