Luca3317 / TMPEffects

Easily animate Unity text and apply other effects with custom tags
https://luca3317.dev/tmpeffects
MIT License
236 stars 10 forks source link

Crash in inspector when selecting a database asset #20

Open maarten-abbey opened 1 month ago

maarten-abbey commented 1 month ago

I select a database asset, such as the provided TMPEffects/Resources/Databases/ShowAnimationDatabase.asset. The inspector shows nothing, and an error message is posted to the console.

TargetParameterCountException: Number of parameters specified does not match the expected number.

System.Reflection.RuntimeMethodInfo.ConvertValues (System.Reflection.Binder binder, System.Object[] args, System.Reflection.ParameterInfo[] pinfo, System.Globalization.CultureInfo culture, System.Reflection.BindingFlags invokeAttr) (at <46af11b490fa44239a17a904ee7462ef>:0)
System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at <46af11b490fa44239a17a904ee7462ef>:0)
System.Reflection.MethodBase.Invoke (System.Object obj, System.Object[] parameters) (at <46af11b490fa44239a17a904ee7462ef>:0)
TMPEffects.SerializedCollections.Editor.SCEditorUtility.HasDrawerForType (System.Type type, System.Boolean isPropertyManagedReferenceType) (at ./Library/PackageCache/com.luca3317.tmpeffects/Runtime/SerializedDictionary/Editor/Scripts/Utility/SCEditorUtility.cs:109)
TMPEffects.SerializedCollections.Editor.SerializedDictionaryInstanceDrawer.CreateDisplaySettings (UnityEditor.SerializedProperty property, System.Type type) (at ./Library/PackageCache/com.luca3317.tmpeffects/Runtime/SerializedDictionary/Editor/Scripts/SerializedDictionaryInstanceDrawer.cs:277)
TMPEffects.SerializedCollections.Editor.SerializedDictionaryInstanceDrawer.<InitializeSettingsIfNeeded>g__InitializeSettings|49_0 (System.Boolean fieldFlag) (at ./Library/PackageCache/com.luca3317.tmpeffects/Runtime/SerializedDictionary/Editor/Scripts/SerializedDictionaryInstanceDrawer.cs:170)

(etc)

The issue is in the line return getDrawerMethod.Invoke(type, new object[] { type }) != null; in SCEditorUtility.cs. For me, this is within the #if UNITY_2022_3_OR_NEWER brackets.

I've added a try around this line, returning a false in the catch, and that solves the issue for me.

I have a clean installation of TMPEffects set up from just a few days ago. I use the next upcoming version of Unity, version 2023.3.0b10. I believe this version branch will at some point be part of "Unity 6".

Luca3317 commented 1 month ago

Weird... I guess the method signature was changed again. Can you go ahead and check how many parameters it expects and which ones?

Luca3317 commented 1 month ago

Bump

maarten-abbey commented 1 month ago

I tried looking into this, but I'm not exactly sure how this method is to be used.

When I "F12" getDrawerMethod.Invoke, its header seems to be: public object Invoke(object obj, object[] parameters); In my case, the 'type' variable and the new object[] { type } variable is a System.String.

But maybe this is what's going wrong: this line is executed because getDrawerMethod.GetParameters().Length is not 2. Maybe it expects it to be 1 - but it is actually 3. See here:

image

Hope this helps!

Luca3317 commented 1 month ago

Seems like they did add another parameter then. I tried replicating this on the Unity6 preview but didnt run into this issue.

I've added a try around this line, returning a false in the catch, and that solves the issue for me.

That class is part of teh SerializedDictionary package, so I didnt really look at it besides when I ran into the issue of it expecting two instead of one parameter on 2022 and above -- does everything still look right visually in the inspector when you just return false, specifically the databases and SceneCommands / SceneAnimations fields on the TMPWriter / TMPAnimator respectively?

Also, can you go ahead and test what happens if you pass in the boolean outside of the array? So getDrawerMethod.Invoke(type, new object[] { type }, isPropertyManagedReferenceType )

Luca3317 commented 1 month ago

Bump

maarten-abbey commented 1 month ago

I'll try this when I get back on my project, hopefully later this week.

maarten-abbey commented 4 weeks ago

That doesn't work, the only legal headers in my version are:

public object Invoke(object obj, object[] parameters);

and

public abstract object Invoke(object obj, BindingFlags invokeAttr, Binder binder, object[] parameters, CultureInfo culture);

I've updated to 0.1.5 (I think that's right, even though the tag in git says 1.0.5), I've thrown away my /Library/ folder just in case. That didn't change anything.

This is what I'm seeing if I just return false from my try/catch:

image image

Of course, I'm not exactly sure what I'm supposed to see, but I guess that all looks okay?

Let me know if I can help further testing.

Luca3317 commented 3 weeks ago

Yeah, those all look right. I guess Ill add a return false as a fallback for when none of the signatures match the cases.

Only other idea I have without testing it myself is to do getDrawerMethod.Invoke(type, new object[] { type, new object[0], isPropertyManagedReferenceType }); If you could give this one a try. If it doesnt work I think Ill just add the false fallback to the existing cases for now.

I've updated to 0.1.5 (I think that's right, even though the tag in git says 1.0.5)

Fark, I messed up the versioning. Next version will be 1.0.6 then :D

Turkeysteaks commented 2 weeks ago

Only other idea I have without testing it myself is to do getDrawerMethod.Invoke(type, new object[] { type, new object[0], isPropertyManagedReferenceType }); If you could give this one a try. If it doesnt work I think Ill just add the false fallback to the existing cases for now.

Not using this package (but I am using your fork of SerializedDictionaries) and tested this.

I got the following ArgumentException:

ArgumentException: Object of type 'System.Object[]' cannot be converted to type 'System.Type[]'.
System.RuntimeType.CheckValue (System.Object value, System.Reflection.Binder binder, System.Globalization.CultureInfo culture, System.Reflection.BindingFlags invokeAttr) (at <f724beac1c704825ad74571377c52f70>:0)
System.Reflection.RuntimeMethodInfo.ConvertValues (System.Reflection.Binder binder, System.Object[] args, System.Reflection.ParameterInfo[] pinfo, System.Globalization.CultureInfo culture, System.Reflection.BindingFlags invokeAttr) (at <f724beac1c704825ad74571377c52f70>:0)
System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at <f724beac1c704825ad74571377c52f70>:0)
System.Reflection.MethodBase.Invoke (System.Object obj, System.Object[] parameters) (at <f724beac1c704825ad74571377c52f70>:0)
AYellowpaper.SerializedCollections.Editor.SCEditorUtility.HasDrawerForType (System.Type type, System.Boolean isPropertyManagedReferenceType) (at ./Library/PackageCache/ayellowpaper.serialized-dictionary/Editor/Scripts/Utility/SCEditorUtility.cs:124)

Changing new object[0] to new Type[0] as follows:

return getDrawerMethod.Invoke(
                            type,
                            new object[] { type, new Type[0], isPropertyManagedReferenceType }
                        ) != null;

Has prevented any further errors. I've made a quick PR https://github.com/Luca3317/SerializedDictionary/pull/1 with this change

Luca3317 commented 2 weeks ago

Interesting! What version of Unity are you on? Same as @maarten-abbey?

Turkeysteaks commented 2 weeks ago

Interesting! What version of Unity are you on? Same as @maarten-abbey?

I'm on version 6000.0.23f1 and having this problem.

Your previous fix was working fine for me on 2023.3.40f1, and I upgraded to Unity 6 the other day and the bug came back, so I assume it's from the same cause as OP

Luca3317 commented 6 days ago

Sounds good. If it happens on Unity 6 Ill go ahead and test it myself before pushing a fix for this (+ taking your PR) 👍