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.

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

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

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

@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:

/// <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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants