BepInEx / BepInEx.ConfigurationManager

Plugin configuration manager for BepInEx
https://www.patreon.com/ManlyMarco
GNU Lesser General Public License v3.0
231 stars 53 forks source link

Integer or Float values not showing up in the mod settings. #15

Closed vicky5124 closed 3 years ago

vicky5124 commented 3 years ago

Describe the bug

When binding an integer value to the configuration, it does generate configurable entries for it on the configuration file BepInEx/config/org.bepinex.plugins.Test.cfg, but the ConfigurationManager menu does not display this options. The default and configured values are accessible through code just fine, so this is an issue with this mod.

To Reproduce

namespace Test

open BepInEx
open BepInEx.Configuration
open UnityEngine;

[<BepInPlugin("org.bepinex.plugins.Test", "Test", "0.0.1")>]
type Test() =
    inherit BaseUnityPlugin()

    let KeyDef = ConfigDefinition("Confing Thing", "Keybind")
    let NumberDef = ConfigDefinition("Config Thing", "Number")

    // This shows
    member this.Keybind =
        this.Config.Bind(KeyDef, KeyboardShortcut(KeyCode.F9))

    // This doesn't show
    member this.Number =
        this.Config.Bind(NumberDef, 1)

    member this.Awake() =
        this.Logger.LogInfo("Hello, World!")
        // Proof that the value is readable
        this.Logger.LogInfo("Value -> %d", this.Number.Value)

And then in-game, open the mod settings, and see how the Number value is missing, but the Keybind value appears just fine.

Expected behavior

The Number setting appears as a configurable type-able box, as it should.

Screenshots and logs

No errors nor warnings appear.

Desktop

ghorsington commented 3 years ago

Greetings!

According to the F# spec, member defines properties which means that Config.Bind is run lazily only when config value is used.

The way ConfigFile works is if Config.Bind is not called on an entry at least once, it's considered orphaned. ConfigManager shows only used entries (which makes sense, since orphaned entries can be caused for example by plugin updates). ConfigManager should collect registered values at every time when ConfigManager option is opened and closed, but if Config.Bind is never called at least once, it will never see the entry.

A close equivalent of your code in C# is

[BepInPlugin("org.bepinex.plugins.Test", "Test", "0.0.1")]
public class Test : BaseUnityPlugin
{
    internal ConfigDefinition KeyDef = new ConfigDefinition("Confing Thing", "Keybind");
    internal ConfigDefinition NumberDef = new ConfigDefinition("Config Thing", "Number");

        // member in F# defines getters/setters when possible instead of pure variables
    public ConfigEntry<KeyboardShortcut> Keybind => Config.Bind(KeyDef, new KeyboardShortcut(KeyCode.F9));
    public ConfigEntry<int> Number => Config.Bind(NumberDef, 1);

    public void Awake()
    {
        Logger.LogInfo("Hello, World!");
                // Account for inlining that F# compiler does on Release builds
        Logger.LogInfo($"Value -> {Config.Bind(NumberDef, 1).Value}");
    }
}

So Keybind and Number might not be registered. Now, it is still interesting that Number would be visible at all since it should get registered. If anything, it's Keybind that should not be visible as your test code never uses Keybind so Config.Bind is never called for it which causes BepInEx to consider it orphaned. Are you sure it's Number that's not visible? Perhaps it's actually Keybind.

Could you do some additonal testing? That would include:

  1. Try referencing both Keybind and Number in Awake so that Config.Bind is called for both
  2. Try instead using let/val to evaluate Config.Bind eagerly once either in the constructor or Awake. I'm no F# expert (not even a proper full-time language user 😅), but on whim I thought of using self-referential types (fully acknowledging the possible negative performance effects)

    namespace Test
    
    open BepInEx
    open BepInEx.Configuration
    open UnityEngine;
    
    [<BepInPlugin("org.bepinex.plugins.Test", "Test", "0.0.1")>]
    type Test() as this =
        inherit BaseUnityPlugin()
    
        let KeyDef = ConfigDefinition("Confing Thing", "Keybind")
        let NumberDef = ConfigDefinition("Config Thing", "Number")
    
        let Keybind = this.Config.Bind(KeyDef, KeyboardShortcut(KeyCode.F9))
        let Number = this.Config.Bind(NumberDef, 1)
    
        member this.Awake() =
            this.Logger.LogInfo("Hello, World!")
            // Both values should be registered now without using them

    You can of course choose your own approach of eagerly calling Config.Bind

I am not sure if this is an issue of ConfigManager or BepInEx. In any case, Confg.Bind was thought to be more of a method to define the existance of the entry and not a method to continuously access said value. The wording of Config.Bind documentation may be a bit vague here as it just mentions that method "creates a new setting".

vicky5124 commented 3 years ago

@denikson Point 1 does fix it, simply adding printfn "%d this.Number.Value" on Awake loads it, and makes it show up in the mod settings menu. As for point 2, it is not possible to use this in a let constructor, member needs to be used to access this

Are you sure it's Number that's not visible? Perhaps it's actually Keybind.

The only item that shows up in the mod menu is Keybind, number only shows up when point 1 is done.


The documentation for Config.Bind should definitely be updated to address this. And finding out why Keybind works but not Number would make solving this issue much easier, as both are technically lazy loaded, being defined in the same way. Force loading the config entry by using it on awake is not a clean solution.

ghorsington commented 3 years ago

As for point 2, it is not possible to use this in a let constructor, member needs to be used to access this

Hmmm, I can't seem to find conclusive info on this: F# spec does indeed note that

The self identifier that is declared with the as keyword is not initialized until after the let bindings are executed. Therefore, it cannot be used in the let bindings. You can use the self identifier in the do bindings section.

However, an answer on StackOverflow implies otherwise.

In fact, compiling the example I sent yields a valid constructor once viewed via a decompiler (I'm using ilSpy but even something as simple as ildasm will do):

public Test()
{
    FSharpRef<Test> this2 = new FSharpRef<Test>(null); // Some F# helper that gets generated when using `type T() as x`
    this..ctor(); // Here BaseUnityPlugin ctor is called which initializes the config
    this2.contents = this; // Sets the instance to the ref
    ConfigDefinition KeyDef = new ConfigDefinition("Confing Thing", "Keybind");
    ConfigDefinition NumberDef = new ConfigDefinition("Config Thing", "Number");
    // LanguagePrimitives.IntrinsicFunctions.CheckThis checks that `this2.contents` is not null (which is `true` at this point) and returns itself
    ConfigEntry<KeyboardShortcut> Keybind = LanguagePrimitives.IntrinsicFunctions.CheckThis<Test>(this2.contents).Config.Bind<KeyboardShortcut>(KeyDef, new KeyboardShortcut(KeyCode.F9, new KeyCode[0]), null);
    ConfigEntry<int> Number = LanguagePrimitives.IntrinsicFunctions.CheckThis<Test>(this2.contents).Config.Bind<int>(NumberDef, 1, null);
        // Auto-generated field to mark the object initialized. This seemingly prevents members from referencing themselves, which is equivalent to C# autoproperties
    this.init@8 = 1;
}

and I reference Number or Keybind in code, they turn into proper fields, which still yields the intended constructor code. From looking at the code and internal F# core methods used in this I don't see why this would fail to run. However, I am yet to test the code in practice.

Just to be double-sure, here's compiled DLL of the code I sent in previous message. This is compiled against .NET Standard 2.0 which is usable in some Unity versions, however my point is more about giving the ability to see what I see when viewing the compiled code via ilSpy:

Test2.zip

The only item that shows up in the mod menu is Keybind, number only shows up when point 1 is done.

This still seems peculiar to me. Me or @ManlyMarco will try and reproduce this as soon as possible on our setups.

What's the Unity version that Poly Bridge 2 uses? That would help with testing the code in the correct environment.

vicky5124 commented 3 years ago

What's the Unity version that Poly Bridge 2 uses?

From the logs:

[Message:   BepInEx] BepInEx 5.4.1.0 - Poly Bridge 2
[Info   :   BepInEx] Running under Unity v2018.4.30.13015200
[Info   :   BepInEx] CLR runtime version: 4.0.30319.17020
[Info   :   BepInEx] Supports SRE: False
[Info   :   BepInEx] System platform: Bits64, Windows
ghorsington commented 3 years ago

After a little bit of testing I can report that Config.Bind does behave like described before and that Number does indeed show up when using your original test code while Keybind does not.

Here's my test setup: test_setup.zip

The package contains:

I included source of both plugins and their pre-compiled DLLs. Both were compiled with F# 5.

When opening ConfigManager, I see the expected result image

That is

As such, I don't think that this ConfigManager's issue but rather just the effect of BepInEx documentation being a bit vague on what Config.Bind means.

ManlyMarco commented 3 years ago

Closing since it's not an issue with config manager.