Odin uses the wrong property drawers for some Animancer transitions

Issue #798 resolved
Kailas Dierk created an issue
  1. I’m using Unity 2019, but I doubt this has anything to do with the Unity version.
  2. Import Odin Inspector and Serializer.
  3. Import Animancer Lite (or Pro).
  4. Right Click in the Project window and go to Create → Animancer → Mixer Transition → Linear.
  5. The Inspector looks like this:

6. The Inspector should look like this (which it does if we don’t have Odin or give it a [DrawWithUnity] attribute):

The main problem is that the wrong PropertyDrawer type is being used.

  • The Transition field is in the generic base class in Assets/Plugins/Animancer/Utilities/Transitions/AnimancerTransitionAssetT.cs.
  • The ScriptableObject class (LinearMixerTransitionAsset) and the generic field type (LinearMixerTransition) are located in Assets/Plugins/Animancer/Utilities/Transitions/LinearMixerTransitionAsset.cs
  • The Drawer class is nested inside LinearMixerTransition at the bottom of that script and it has an explicit [CustomPropertyDrawer] attribute for LinearMixerTransition so I can’t see any reason why it wouldn’t get used.
  • But the drawer that actually gets used is my base Animancer.Editor.TransitionDrawer which has a `[CustomPropertyDrawer(typeof(ITransitionDetailed), true)]`.
  • So the problem could just be that Odin is picking the base drawer class because the interface matches even though LinearMixerTransition.Drawer’s attribute matches the class (which should have priority).

The other problem is that the Odin Inspector in step 5 has indented the fields too far. Here’s what it looks like without Odin if I comment out the LinearMixerTransition.Drawer so that it picks the base TransitionDrawer:

Comments (10)

  1. Tor Esa Vestergaard
    • changed status to open

    Hmm, will have to take a look at whether that can be resolved on our end in a consistent way.

    Odin does not discrimate by the type of matching of drawers (by interface or precise class or whatever), but instead by priority, which can be declared by drawers. In the case of drawers with equal priority, Odin resolves conflicts by the alphabetic names of the drawers.

    You can always see the order of drawers on a member drawn by Odin by putting [ShowDrawerChain] on the member. It could be this is as easily resolved as adding this to the drawer you want to be preferred.

    #if ODIN_INSPECTOR
    [Sirenix.OdinInspector.Editor.DrawerPriority(0, 0, 2)]
    #endif
    

    If you want to have a more speedy back-and-forth dialogue about this, please feel free to write on our support Discord as well, as it's a bit faster to work through there usually :)

  2. Tor Esa Vestergaard

    As for the indentation, that would be because of how Odin displays polymorphism. I would guess that a quick #if’ed attribute of [HideReferenceObjectPicker] on the given member or type would resolve that.

  3. Kailas Dierk reporter

    Odin does not discrimate by the type of matching of drawers (by interface or precise class or whatever), but instead by priority

    Unity does though, so Odin should follow the same rules unless it has a good reason not to. Just sort by priority, then for anything with equal priority sort using the same rules Unity does.

    It could be this is as easily resolved as adding this to the drawer you want to be preferred.

    Animancer doesn’t require Odin and my assemblies shouldn’t have any need to reference it at all. If Odin is going to change the way other plugins are drawn, then it’s Odin’s responsibility to draw them correctly. I shouldn’t have to add extra dependencies and hacks to my code to cover up bugs in other completely unrelated systems.

  4. Tor Esa Vestergaard

    We can certainly look into changing the way priority for old-style Unity property drawers is handled amongst themselves - it should ideally match Unity’s method as closely as possible, that will go on the backlog of issues to handle.

    The polymorphism drawing is a different issue (and not a bug) where both your and the Odin polymorphism code are adding indentation, and there I’m afraid you will either have to account for the current indentation in your drawer and not add extra indentation yourself if it is already there, or add some conditional Odin attributes to modify how Odin draws this, or some other way of compensating for the difference.

  5. Kailas Dierk reporter

    Checking whether the label is empty seems to work for detecting whether or not I should add indentation. I can’t think of any other reason my drawer would be given an empty label.

  6. Tor Esa Vestergaard

    I can report that this issue has been fixed in Odin 3.0.10, which should release this week. (Tested with Animancer Lite 7.2.)

    Odin should now use a Unity PropertyDrawer ordering that far more closely matches how Unity itself does it :)

  7. Log in to comment