KybernetikGames / animancer

Documentation for the Animancer Unity Plugin.
63 stars 8 forks source link

Scriptable Objects as Keys for Animancer Events #264

Open KybernetikGames opened 1 year ago

KybernetikGames commented 1 year ago

Use Case

This issue is split out from https://github.com/KybernetikGames/animancer/issues/262 since it could be implemented separately from the main idea presented there.

you would now be comparing references to ScriptableObjects instead of magic strings [referring to the names of Animancer Events]

That suggestion is rather controversial because unlike https://github.com/KybernetikGames/animancer/issues/257 this is already an existing feature and I couldn't reasonably take strings away from existing users. Even if we ignore the data loss and manual rework this would cause for anyone who updates, I'm not entirely convinced that the ScriptableObject workflow would be better.

With strings it's only a 2 or 3 step process:

  1. Optionally use Event Names Attributes to avoid some of the safety issues. But not all of them, in particular, if you pick a name in the Inspector then rename it in code, the serialized data won't get updated.
  2. Pick a name when setting up your event.
  3. Specify the name when setting the callback, either as a magic string or using the same string constant you gave the [EventNames] attribute.

But with ScriptableObjects you need to:

  1. Create a Key asset. KeyAsset? KeyObject?
  2. Pick the Key when setting up your event. This will be slower than picking a name from the dropdown when using an [EventNames] attribute.
  3. Add a serialized Key field to your script. If the place you want to use it isn't serializable, you'll need to put the field somewhere serializable and mess around with passing the reference around to where you need it.
  4. Go to that field in the Inspector and assign the Key.
  5. Specify the Key when setting the callback.

So even though the end result with ScriptableObjects gives better safety and potentially better performance, all those extra steps are both time sinks and opportunities to make mistakes so such a system would be a hard sell to users who just want to get stuff done and move on.

But all hope is not lost:

Solution Options

Use Both

Supporting both string and Key alongside each other might not be too bad. The event names are already stored in a separate array from the event times so that it only needs space up to the last event with a name, meaning if you don't use any names then the array is empty. So doing the same with a ScriptableObject array would mean the overhead is only one empty array per event sequence (for serialized data size, deserialization performance, and runtime memory usage) unless you use a mix of both types (which seems unlikely).

Adding an overhead for everyone regardless of which approach they use isn't ideal and offering more different ways to do things makes it harder for new users to learn. It might still be worth it, but like I said over in the mixer issue I don't like this sort of duplication.

Data Conversion

If we don't keep both, it will need to convert any old data. I'm not usually a fan of keeping obsolete stuff in my scripts just to give users a warning instead of an error when they update, but in this case that would mean losing everyone's event names which isn't really acceptable if it can be avoided. So I'd probably need to keep the old field and include an automatic updater script which loads every prefab, scriptable object, and scene in your project to go through and replace any named events with Keys.

Create Keys in the Inspector

When setting up an event in the Inspector, its Key field could have a New button which replaces the reference field with a text field where you can type a name and press enter to create a Key with that name in a default location.

This would streamline step 1 in the steps above.

Unique Scriptable Object Names

If we require that every Key have a unique name (which seems like a reasonable restriction even if only to reduce potential confusion) then we could potentially have a system like this:

This would take a lot of effort to implement, but would make steps 3 and 4 in the above list optional, bringing the Key system down much closer to the simplicity of the pure string system.

KybernetikGames commented 6 months ago

I've now implemented this feature in the upcoming Animancer v8.0.

Unfortunately, this is a breaking change. Any event names set up in an older version of Animancer will be lost.

Here's a summary of how it works:

private static readonly InternedString HitEventName = "Hit";
KybernetikGames commented 1 week ago

Animancer v8.0 is now available for Alpha Testing..