Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OnChange attribute does not work for Choice options #367

Open
LeeTwentyThree opened this issue May 24, 2023 · 3 comments · May be fixed by #510
Open

OnChange attribute does not work for Choice options #367

LeeTwentyThree opened this issue May 24, 2023 · 3 comments · May be fixed by #510
Labels
Priority: High Status: 🆕 Not Started This item hasn't been started Type: Bug

Comments

@LeeTwentyThree
Copy link
Member

Describe the bug
If a choice option has the OnChange attribute, an exception is thrown upon value change (after the option's value has been saved and recognized).

[Error  : Unity Log] InvalidOperationException: Late bound operations cannot be performed on types or methods for which ContainsGenericParameters is true.
Stack trace:
System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at <eae584ce26bc40229c1b1aa476bfa589>:0)
System.Reflection.MethodBase.Invoke (System.Object obj, System.Object[] parameters) (at <eae584ce26bc40229c1b1aa476bfa589>:0)
HarmonyLib.Traverse.GetValue (System.Object[] arguments) (at <474744d65d8e460fa08cd5fd82b5d65f>:0)
Nautilus.Options.Attributes.MemberInfoMetadata`1[T].InvokeMethod (T config, System.Object[] arguments) (at <0d1aa834df874e409ee7db3e5983cf6b>:0)
Nautilus.Options.Attributes.ConfigFileMetadata`1[T].InvokeEvent[TSource] (Nautilus.Options.Attributes.MemberInfoMetadata`1[T] memberInfoMetadata, System.Object sender, TSource e) (at <0d1aa834df874e409ee7db3e5983cf6b>:0)
Nautilus.Options.Attributes.ConfigFileMetadata`1[T].InvokeOnChangeEvents[TSource] (Nautilus.Options.Attributes.ModOptionAttributeMetadata`1[T] modOptionMetadata, System.Object sender, TSource e) (at <0d1aa834df874e409ee7db3e5983cf6b>:0)
Nautilus.Options.Attributes.ConfigFileMetadata`1[T].HandleChoiceChanged[Any] (System.Object sender, Nautilus.Options.ChoiceChangedEventArgs`1[T] e) (at <0d1aa834df874e409ee7db3e5983cf6b>:0)
System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at <eae584ce26bc40229c1b1aa476bfa589>:0)
Rethrow as TargetInvocationException: Exception has been thrown by the target of an invocation.
System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at <eae584ce26bc40229c1b1aa476bfa589>:0)
System.Reflection.MethodBase.Invoke (System.Object obj, System.Object[] parameters) (at <eae584ce26bc40229c1b1aa476bfa589>:0)
Nautilus.Options.Attributes.OptionsMenuBuilder`1[T].RouteEventHandler (System.Object sender, System.EventArgs e) (at <0d1aa834df874e409ee7db3e5983cf6b>:0)
Nautilus.Options.Attributes.OptionsMenuBuilder`1[T].EventHandler (System.Object sender, System.EventArgs e) (at <0d1aa834df874e409ee7db3e5983cf6b>:0)
Nautilus.Options.ModOptions.OnChange[T,E] (System.String id, T value) (at <0d1aa834df874e409ee7db3e5983cf6b>:0)
Nautilus.Options.ModChoiceOption`1[T].<AddToPanel>b__17_0 (System.Int32 index) (at <0d1aa834df874e409ee7db3e5983cf6b>:0)
UnityEngine.Events.InvokableCall`1[T1].Invoke (T1 args0) (at <bd0d47c27bd84106afaaecc2c74cdb94>:0)
UnityEngine.Events.UnityEvent`1[T0].Invoke (T0 arg0) (at <bd0d47c27bd84106afaaecc2c74cdb94>:0)
uGUI_Choice.set_value (System.Int32 value) (at <9ec6c1b62c324758b9c2d79021505be6>:0)
uGUI_Choice.NextChoice () (at <9ec6c1b62c324758b9c2d79021505be6>:0)
UnityEngine.Events.InvokableCall.Invoke () (at <bd0d47c27bd84106afaaecc2c74cdb94>:0)
UnityEngine.Events.UnityEvent.Invoke () (at <bd0d47c27bd84106afaaecc2c74cdb94>:0)
UnityEngine.UI.Button.Press () (at <ce6ecc2ca9f24767b31663d408ff6f4f>:0)
UnityEngine.UI.Button.OnPointerClick (UnityEngine.EventSystems.PointerEventData eventData) (at <ce6ecc2ca9f24767b31663d408ff6f4f>:0)
UnityEngine.EventSystems.ExecuteEvents.Execute (UnityEngine.EventSystems.IPointerClickHandler handler, UnityEngine.EventSystems.BaseEventData eventData) (at <ce6ecc2ca9f24767b31663d408ff6f4f>:0)
UnityEngine.EventSystems.ExecuteEvents.Execute[T] (UnityEngine.GameObject target, UnityEngine.EventSystems.BaseEventData eventData, UnityEngine.EventSystems.ExecuteEvents+EventFunction`1[T1] functor) (at <ce6ecc2ca9f24767b31663d408ff6f4f>:0)
UnityEngine.EventSystems.ExecuteEvents:Execute(GameObject, BaseEventData, EventFunction`1)
FPSInputModule:ProcessMousePress(MouseButtonEventData)
FPSInputModule:ProcessMouseEvent()
FPSInputModule:OnUpdate()
ManagedUpdate:Execute(Queue)
ManagedUpdate:ExecuteRange(Queue, Queue)
ManagedUpdate:LateUpdate()

To Reproduce
Steps to reproduce the behavior:

  1. Create an attribute-based option system.
  2. Add a choice option to the config.
  3. Add an OnChange attribute to the same option.
  4. Launch the game and open the Options menu.
  5. Change the option's value. The exception should now appear in the log.

Expected behavior
No exception (callback method should be actually called properly).

Screenshots
image
@LeeTwentyThree LeeTwentyThree mentioned this issue May 24, 2023
Merged
@JKohlman JKohlman self-assigned this May 24, 2023
@MrPurple6411 MrPurple6411 added the Status: 🆕 Not Started This item hasn't been started label Sep 25, 2023
@JKohlman JKohlman removed their assignment Dec 17, 2023
@JKohlman
Copy link
Contributor

Un-assigned myself cause I just don't have the motivation outside of work to dig super deep into weird generics again, for a start at some debug logs you can check JKohlman@88e63a4. TLDR is it's generics, always has been

@LeeTwentyThree
Copy link
Member Author

Example of the issue:

Nautilus/Example mod/ConfigExamples.cs

Lines 144 to 186 in 83b6874

/// <summary>
/// <para>A <see cref="ChoiceAttribute"/> is represented by a group of options where only one can be selected at a time,
/// similar in usage to a dropdown or radial button group.</para>
///
/// <para>Here, we are defining a <see cref="ChoiceAttribute"/> with the label "My index-based choice", where the underlying
/// <see cref="int"/> field represents the index in an array of choices, where the values "One", "Two" and "Three" make
/// up that array. As we are not specifiying a default value, the index will by 0 by default.</para>
///
/// <para>The <see cref="OnChangeAttribute"/> is optional and allows us to specify the name of a method in the Config class to
/// call when the value has been changed via the options menu. Note that in many cases, you won't need to specify an OnChange
/// event, as the values are automatically saved to disk for you as specified by the <see cref="MenuAttribute"/>, and are
/// updated in the instance of <see cref="Config"/> returned when registering it to the options menu.</para>
///
/// <para>Here, we are specifying the name of a method which can handle any OnChange event, for the purposes of demonstrating
/// its usage. See <see cref="MyGenericValueChangedEvent(ModOptionEventArgs)"/> for an example usage.</para>
/// </summary>
[Choice("My index-based choice", "One", "Two", "Three"), OnChange(nameof(MyChoiceValueChangedEvent))]
public int ChoiceIndex;
/// <summary>
/// Here, we are defining a <see cref="ChoiceAttribute"/> which uses a <see cref="string"/> as its backing field,
/// where the value represents which option is currently selected, and are specifying "Foo" as the default.
/// </summary>
[Choice("My string-based choice", "Foo", "Bar"), OnChange(nameof(MyChoiceValueChangedEvent))]
public string ChoiceValue = "Foo";
/// <summary>
/// Here, we are defining a <see cref="ChoiceAttribute"/> which uses an <see langword="enum"/>-type as its backing field,
/// and the string values for each value defined in the <see langword="enum"/> will be used to represent each option,
/// so we don't need to specify them. The options will be "One", "Two" and "Three".
/// </summary>
[Choice("My enum-based choice"), OnChange(nameof(MyChoiceValueChangedEvent))]
public CustomChoice ChoiceEnum;
/// <summary>
/// <para>Here, we are again defining a <see cref="ChoiceAttribute"/> with a <see langword="enum"/>-type as its backing field,
/// however we are also specifying custom strings which will be used to represent each option in the menu.</para>
///
/// <para>An option of <see cref="CustomChoice.One"/> will be represented by the <see cref="string"/> "1",
/// <see cref="CustomChoice.Two"/> will be represented by the <see cref="string"/> "2", and so on.</para>
/// </summary>
[Choice("My customised enum-based choice", "1", "2", "3"), OnChange(nameof(MyChoiceValueChangedEvent))]
public CustomChoice ChoiceCustomEnum;

@JKohlman
Copy link
Contributor

Minimized reproduction:

[Choice("TestOption", options: new[] { "Choice One", "Choice Two" })]
[OnChange(nameof(OnTestOptionChange))]
public string choiceValue = "Choice One";

public void OnTestOptionChange<T>(object sender, ChoiceChangedEventArgs<T> e)
{
}

@MrPurple6411 MrPurple6411 linked a pull request Jan 2, 2024 that will close this issue
Screw you I WIN..... #510
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Priority: High Status: 🆕 Not Started This item hasn't been started Type: Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Screw you I WIN..... SubnauticaModding/Nautilus
3 participants
@JKohlman @MrPurple6411 @LeeTwentyThree