Config menu for OWML #147

amazingalek · 2020-07-04T20:50:05Z

No description provided.

OWML.ModLoader/ModLoader.cs

OWML.ModHelper.Menus/ModsMenu.cs

OWML.ModHelper.Menus/OwmlConfigMenu.cs

* removed mod data from config menus * introduced default config for owml

OWML.Common/Menus/IModConfigMenu.cs

amazingalek · 2020-07-05T12:33:53Z

OWML.ModLoader/ModData.cs

@@ -17,15 +15,5 @@ public ModData(IModManifest manifest, IModConfig config, IModConfig defaultConfi
            DefaultConfig = defaultConfig;
        }

-        public void ResetConfig()


Removing this is a breaking change but I think no mods used it. Did any of you?

I actually don't have to remove this any more, after introducing the base class... kinda wanna do it still 🤷

I suppose resetting to defaults is handled by ModConfig now?

Yeah, which do you think should do it - ModData or ModConfigMenu?

I think it depends whether or not you want to implement "Reset all mods to defaults" feature

If you think there's value in keeping the reset method in mod data, I'll revert it. I don't have a strong opinion on this. Give the word :)

Only value I can see is making implementing "Reset All to Defaults" easier. And only reason I can see why user might want to reset all is if there are problems with outdated config and there is either multiple of them or user can't find which one is misbehaving

BTW, I suggest you to think about picking up type changes from default config (i.e. number to slider or to enum)

And warning on (seemingly) incompatible types

That sounds hard. Personally I don't think it would be worth it. But either way I'd argue it's outside the scope of this PR :)

amazingalek · 2020-07-08T19:36:05Z

#153 first

… code (#153)

OWML.ModHelper.Menus/BaseConfigMenu.cs

OWML.ModHelper.Menus/ModComboInput.cs

* menu title constants

TAImatem · 2020-07-09T08:28:50Z

Other than what I mentioned things look ok to me

Raicuparta · 2020-07-09T16:44:14Z

createrelease.bat

@@ -7,6 +7,7 @@ copy "OWML.Patcher\bin\Debug\OWML.Patcher.dll" "Release\OWML.Patcher.dll"
 copy "OWML.ModLoader\bin\Debug\OWML.ModLoader.dll" "Release\OWML.ModLoader.dll"
 copy "OWML.Launcher\bin\Debug\OWML.Launcher.exe" "Release\OWML.Launcher.exe"
 copy "OWML.Launcher\bin\Debug\OWML.Config.json" "Release\OWML.Config.json"
+copy "OWML.Launcher\bin\Debug\OWML.DefaultConfig.json" "Release\OWML.DefaultConfig.json"


Now that we have a default OWML config, we shouldn't include OWML.Config.json in the releases, right? One of the main advantages of having a separate default config file is that new releases don't replace configs of previous releases (which can be very annoying if you need a custom game path). OMLW.Config.json should only be generated from the default, after OWML starts. So only OWML.DefaultConfig.json should be here.

Yeah, totally 👍
If we do this: before starting OWML the first time, how will you show/edit the config in MM?

Same as for mods. If mod manager sees only default, copy default to user config. Otherwise edit user config.

Need to make sure we copy each property and not only the whole file (in case we add new properties to config). Don't remember if mods already do this.

@Raicuparta removed config file, and creating it from default if it's missing.

Just copying the file, as there's no (reason to have a) list of generic settings.

Raicuparta · 2020-07-09T18:49:23Z

OWML.Launcher/Program.cs

@@ -13,32 +13,44 @@ public class Program
    {
        static void Main(string[] args)
        {
-            var owmlConfig = GetOwmlConfig();
+            var owmlConfig = GetOwmlConfig() ?? CreateOwmlConfig();


Won't this cause problems if you add new entries to the config? As in, if a user has a OWML.Config.json file, and then in a new update OWML adds a new configurable setting. The config exists, but is missing a property, so you're not getting that property from the default config.

I tried deleting a line from the config, to simulate your situation. It worked fine :-)
The config defaulted to the default of the data type (false for bool in this case) - it showed fine in the menu and saved properly after changing it.

Hm but what if default was supposed to be true? 😅

Hmmm... 🤔
I don't know how to code what you're suggesting without making a manually maintained mess. Any ideas?

I guess iterating over properties using reflection. Sounds hard :P

I guess too many things would have to change. What makes most sense for me is not to copy properties selectively, but to do in the moment of getting the property. So when you're trying to get OwmlConfig.GamePath, the getter checks if the property exists in OWML.Config.json, and if not, gets it from OWML.DefaultConfig.json.

Ideally, you wouldn't even need to create the OWML.Config.json file until the user changes a setting. And when they do, the user config would only needs to include the settings that differ from the default.

But I understand that this can be to big of a change, and might not be easy to do with the way the JSON serialization is done (I think it always writes the whole file from the properties).

In any case, it's at the very least something to keep in mind if we add new settings ;)

TAImatem

eh

somehow it seems to work :O

7f73095

amazingalek linked an issue Jul 4, 2020 that may be closed by this pull request

OWML config menu #139

Closed

amazingalek added 3 commits July 4, 2020 23:00

button title constants

7b22da2

constants for owml config and manifest filename

f76ece6

cleanup up base config menu

0da422d

amazingalek commented Jul 4, 2020

View reviewed changes

OWML.ModLoader/ModLoader.cs Outdated Show resolved Hide resolved

amazingalek marked this pull request as ready for review July 4, 2020 21:20

amazingalek requested review from TAImatem, Raicuparta and misternebula July 4, 2020 21:20

amazingalek commented Jul 4, 2020

View reviewed changes

OWML.ModHelper.Menus/ModsMenu.cs Outdated Show resolved Hide resolved

amazingalek commented Jul 4, 2020

View reviewed changes

OWML.ModHelper.Menus/OwmlConfigMenu.cs Outdated Show resolved Hide resolved

foreach child of transform

8cbfc03

amazingalek mentioned this pull request Jul 5, 2020

removed mod data from config menus #148

Merged

removed mod data from config menus (#148)

71c6998

* removed mod data from config menus * introduced default config for owml

amazingalek commented Jul 5, 2020

View reviewed changes

OWML.Common/Menus/IModConfigMenu.cs Show resolved Hide resolved

amazingalek added 2 commits July 5, 2020 14:24

made some properties private

5e4eb52

formatting

c9f64c0

amazingalek commented Jul 5, 2020

View reviewed changes

amazingalek added 2 commits July 5, 2020 14:34

formatting

c143ee1

Merge branch 'dev' into owml-menu

7cd3105

amazingalek added 3 commits July 8, 2020 21:56

common base class for ModConfigMenu and OwmlConfigMenu. removes hacky…

b705839

… code (#153)

re-added mod data to mod config menu

f3bcc1a

props order and input blocking title

daa1872

TAImatem reviewed Jul 9, 2020

View reviewed changes

OWML.ModHelper.Menus/BaseConfigMenu.cs Show resolved Hide resolved

OWML.ModHelper.Menus/ModComboInput.cs Outdated Show resolved Hide resolved

* reverted ModComboInput change

acebd2a

* menu title constants

Raicuparta reviewed Jul 9, 2020

View reviewed changes

creating config fike from default if missing

4a73f2f

amazingalek added 2 commits July 9, 2020 20:19

forgot a thing

cf4d187

fixed a mistake

b5e11aa

Raicuparta reviewed Jul 9, 2020

View reviewed changes

merged from dev

4fc3e14

amazingalek mentioned this pull request Jul 9, 2020

gathered all file reading/writing to one place #157

Merged

TAImatem approved these changes Jul 10, 2020

View reviewed changes

amazingalek merged commit 172f4b6 into dev Jul 10, 2020

amazingalek deleted the owml-menu branch July 10, 2020 12:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Config menu for OWML #147

Config menu for OWML #147

amazingalek commented Jul 4, 2020

amazingalek Jul 5, 2020

amazingalek Jul 8, 2020

TAImatem Jul 9, 2020

amazingalek Jul 9, 2020

TAImatem Jul 9, 2020

amazingalek Jul 9, 2020

TAImatem Jul 9, 2020

TAImatem Jul 9, 2020

TAImatem Jul 9, 2020

amazingalek Jul 9, 2020

amazingalek commented Jul 8, 2020

TAImatem commented Jul 9, 2020

Raicuparta Jul 9, 2020 •

edited

Loading

amazingalek Jul 9, 2020

Raicuparta Jul 9, 2020

amazingalek Jul 9, 2020

amazingalek Jul 9, 2020

Raicuparta Jul 9, 2020

amazingalek Jul 9, 2020

Raicuparta Jul 9, 2020

amazingalek Jul 9, 2020

amazingalek Jul 9, 2020

Raicuparta Jul 9, 2020 •

edited

Loading

TAImatem left a comment

Config menu for OWML #147

Config menu for OWML #147

Conversation

amazingalek commented Jul 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amazingalek commented Jul 8, 2020

TAImatem commented Jul 9, 2020

Raicuparta Jul 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Raicuparta Jul 9, 2020 • edited Loading

Choose a reason for hiding this comment

TAImatem left a comment

Choose a reason for hiding this comment

Raicuparta Jul 9, 2020 •

edited

Loading

Raicuparta Jul 9, 2020 •

edited

Loading