Skip to content
This repository has been archived by the owner on Jul 30, 2020. It is now read-only.

Added InspectorCurveAttribute #191

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

SugoiDev
Copy link
Collaborator

Needed something like this to restrict curves to a certain range and just whipped this a few minutes ago.
Always impressed with how easy we can do things in FullInspector.

Placed in the Modules folder, since InspectorRange is already there.

Would appreciate a quick review before merging.

@SugoiDev
Copy link
Collaborator Author

Already stumbled on something.

How can I make this work on collections?

Currently, it will error out:

[Warning] Failed to create attribute property editor System.Collections.Generic.List`1[UnityEngine.AnimationCurve] VerticalMovementCurves for FullInspector.Modules.InspectorCurveAttributeEditor`1[TElement]
0. FullInspector.Internal.AttributePropertyEditor.TryCreate()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Core/Editor/PropertyEditors/AttributePropertyEditor.cs:197
1. FullInspector.PropertyEditor.GetCachedEditors()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Core/Editor/PropertyEditor.cs:123
2. FullInspector.PropertyEditor.Get()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Core/Editor/PropertyEditor.cs:188
3. FullInspector.Internal.fiEditorGUI.EditPropertyHeightDirect()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Core/Editor/fiEditorGUI.cs:370
4. FullInspector.Internal.fiEditorGUI.EditPropertyHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Core/Editor/fiEditorGUI.cs:381
5. FullInspector.Internal.ReflectedPropertyEditor.GetElementHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Core/Editor/PropertyEditors/ReflectedPropertyEditor.cs:396
6. FullInspector.PropertyEditorExtensions.GetElementHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Core/Editor/IPropertyEditorExtensions.cs:268
7. FullInspector.Internal.<.cctor>c__AnonStorey<>c.<>m__<.cctor>b__0_17()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Core/Editor/fiLateBindingsBinder.cs:66
8. FullInspector.Internal.PropertyEditor.GetElementHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Core/fiLateBindings.cs:319
9. FullInspector.PropertyEditor.DoGetHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Modules/tkControl/Controls/tkPropertyEditor.cs:107
10. FullInspector.tkControl`2.GetHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Modules/tkControl/tkControl.cs:153
11. FullInspector.VerticalGroup.DoGetHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Modules/tkControl/Controls/tkVerticalGroup.cs:72
12. FullInspector.tkControl`2.GetHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Modules/tkControl/tkControl.cs:153
13. FullInspector.StyleProxy.DoGetHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Modules/tkControl/Controls/tkProxy.cs:23
14. FullInspector.tkControl`2.GetHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Modules/tkControl/tkControl.cs:153
15. FullInspector.VerticalGroup.DoGetHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Modules/tkControl/Controls/tkVerticalGroup.cs:72
16. FullInspector.tkControl`2.GetHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Modules/tkControl/tkControl.cs:153
17. FullInspector.tkControl`2.GetHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Modules/tkControl/tkControl.cs:162
18. FullInspector.Internal.fiEditorGUI.tkControlHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Core/Editor/fiEditorGUI.cs:269
19. FullInspector.Internal.tkControlBehaviorEditor`1.OnGetHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Modules/tkControl/Editor/tkControlBehaviorEditor.cs:22
20. FullInspector.BehaviorEditor`1.GetHeight()    Assets/Plugins/Vendor/FullInspector/FullInspector2/Core/Editor/IBehaviorEditor.cs:101

@SugoiDev
Copy link
Collaborator Author

I read some of the code and thought I had figured a solution, but failed.

What I tried to do was adding a params to GetAttributes

public interface fiICollectionAttributeProvider {
    IEnumerable<object> GetAttributes(params object[] attributeArgs);
}

Then, in InspectorCollectionItemAttributesAttribute (and maybe in InspectorCollectionAddItemAttributesAttribute), I would use

public InspectorCollectionItemAttributesAttribute(Type attributes, params object[] attributeArgs) {
    if (typeof(fiICollectionAttributeProvider).Resolve().IsAssignableFrom(attributes.Resolve()) == false) {
        throw new ArgumentException("Must be an instance of FullInspector.fiICollectionAttributeProvider", "attributes");
    }

    var instance = (fiICollectionAttributeProvider)Activator.CreateInstance(attributes);
    AttributeProvider = fiAttributeProvider.Create(instance.GetAttributes(attributeArgs).ToArray());
}

But it didn't work.
What I couldn't understand, was that this would work normally

public class InspectorMovementCurvesCollection: fiICollectionAttributeProvider {
    public IEnumerable<object> GetAttributes(params object[] attributeArgs) {
        yield return new InspectorCurveAttribute(0, 0, 10, 10);
    }
}

Meaning, it would actually use the (0, 0, 10, 10) in the constructor.

But, if I instead used yield return new InspectorCurveAttribute(attributeArgs); (of course, with a proper constructor in the InspectorCurveAttribute), it would simply not work (i.e., it would ignore the constructor on InspectorCurveAttribute completely, not even using the default values).

Could you shed some light on this, @jacobdufault ?


What I ended up doing was to just have a default that matches my current needs

public class InspectorMovementCurvesDefault: fiICollectionAttributeProvider {
    public IEnumerable<object> GetAttributes() {
        yield return new InspectorCurveAttribute();
    }
}

I'll update the PR with this.

@jacobdufault
Copy link
Owner

Any idea why the constructor forwarding approach did not work? I would expect that Activator.CreateInstance(object[] params) would call the right constructor.

Is this the usage you were trying to go for?

[InspectorCollectionProxy(typeof(InspectorCurveAttribute), 0, 0, 1, 1)]
public LIst<AnimationCurve> entries;


namespace FullInspector.Modules {
[CustomAttributePropertyEditor(typeof(InspectorCurveAttribute), ReplaceOthers = true)]
public class InspectorCurveAttributeEditor<TElement> : AttributePropertyEditor<TElement, InspectorCurveAttribute> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since AnimationCurve is a sealed class, you don't need to make this editor generic. So

public class InspectorCurveAttributeEditor : AttributePropertyEditor<AnimationCurve, InspectorCurveAttribute> {

namespace FullInspector.Modules {
[CustomAttributePropertyEditor(typeof(InspectorCurveAttribute), ReplaceOthers = true)]
public class InspectorCurveAttributeEditor<TElement> : AttributePropertyEditor<TElement, InspectorCurveAttribute> {
private static T Cast<T>(object o) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed when the class is not generic.



/// <summary>
/// Uses the default constructor from <see cref="InspectorCurveAttribute" /><br />
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: No indentation after ///
(applies to every comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants