Esoteric Software

[bklooster]

I really do like the custom attributes that you have for the property drawers you have for accessing spine properties like bones, skins, etc. I'd love it if it wasn't string-based and instead imported and reused scriptableobjects so that it survives renames.

However, why I'm posting is I'm seeing some of these attributes not respecting all of the attribute properties for example. The SpineSkinDrawer doesn't seem to populate the NONE label properly even when the default is to have it there.

protected override void PopulateMenu (GenericMenu menu, SerializedProperty property, SpineSkin targetAttribute, SkeletonData data) {
menu.AddDisabledItem(new GUIContent(skeletonDataAsset.name));
menu.AddSeparator("");

// add NONE option if enabled on the attribute
if (targetAttribute.includeNone) {
menu.AddItem(new GUIContent(NoneString), !property.hasMultipleDifferentValues && string.IsNullOrEmpty(property.stringValue), HandleSelect, new SpineDrawerValuePair(string.Empty, property));
}

for (int i = 0; i < data.Skins.Count; i++) {
string name = data.Skins.Items[i].Name;
if (name.StartsWith(targetAttribute.startsWith, StringComparison.Ordinal)) {
bool isDefault = string.Equals(name, DefaultSkinName, StringComparison.Ordinal);
string choiceValue = TargetAttribute.defaultAsEmptyString && isDefault ? string.Empty : name;
menu.AddItem(new GUIContent(name), !property.hasMultipleDifferentValues && choiceValue == property.stringValue, HandleSelect, new SpineDrawerValuePair(choiceValue, property));
}

}
}

[Harald]

I'd love it if it wasn't string-based and instead imported and reused scriptableobjects so that it survives renames.

Unfortunately using ScriptableObject asset references would just move the problem one step further without solving it, since animation names, skin names, and the like are all identified via name strings in Spine. So renaming a skin or animation in Spine and re-exporting the skeleton would unfortunately still result in a broken reference.

However, why I'm posting is I'm seeing some of these attributes not respecting all of the attribute properties for example. The SpineSkinDrawer doesn't seem to populate the NONE label properly even when the default is to have it there.

The default is to use default as NoneString instead of None, so you see default being displayed here. Or did you mean something else?

[bklooster]

Thanks for the response. DefaultSkin isn't really what I wanted as the choice. I wanted to setup a component that would only work if the spine skeleton had a specific skin enabled so I wanted to expose a "none" option for designers so that the component would make a bit more sense. I might have a specialized use case though which is fine, good to know that default on the skin property drawer. I ended up adding a none option for the skin property drawer.

As for the scriptable object, yeah I understand. I do wish we had better routes to take with importing this data... maybe adding a GUID or something to the data export that we can use on the Unity side to handle renames so that we're not so string dependent. It makes organization and refactoring more difficult than it should be.

Thanks!

[Harald]

Thanks for the response. DefaultSkin isn't really what I wanted as the choice. I wanted to setup a component that would only work if the spine skeleton had a specific skin enabled so I wanted to expose a "none" option for designers so that the component would make a bit more sense. I might have a specialized use case though which is fine, good to know that default on the skin property drawer. I ended up adding a none option for the skin property drawer.

Your use case is indeed a special one. If I understood you correctly, then you introduced an additional invalid state value in addition to the no skin value. While this works, it should perhaps rather not be added to an existing value range but instead be stored separately, or via a derived type. For example if you would like to add an unset state to a bool type (which makes it a tri-state variable) you would not be changing the bool type but instead create a new Tristate class. Nevertheless, whatever fits your needs and makes the life of your artists easier should be fine.

As for the scriptable object, yeah I understand. I do wish we had better routes to take with importing this data... maybe adding a GUID or something to the data export that we can use on the Unity side to handle renames so that we're not so string dependent. It makes organization and refactoring more difficult than it should be.

Thanks for the input, I see your point.