Validate key and value types

[stefanbursuc]

We want to restrict the dictionary's key and value types as follows:

Key:

• needs to be serializable by Unity.
• cannot be a type derived (directly or indirectly) from UnityEngine.Object.
  Because Unity serializes them as references to other objects in the scene or
  in the Project (prefabs, scriptable objects, etc.). This means that the object can
  be null (and so, not a valid dictionary key).
• needs to be equatable. So basically, the key can either be a primitive type,
  an enum, a struct, or a class that implements Equals and GetHashCode.
  This is needed so that the keys are distinguished from one another by value
  and not by refence.

Value:

• needs to be serializable by Unity.

[stefanbursuc]

Hi. You can also take a look at (and integrate) the branch:

• https://github.com/stefanbursuc/Bewildered-Core/tree/types-validation-examples

That contains examples of multiple types for keys and values.

[stefanbursuc]

[MechWarrior99]

Hi, first, thank you for taking the time to create this PR!
You raise some good points about keys and values, I forgot to test these ones.

needs to be serializable by Unity.

I think to be inline with how Unity handles this currently with lists, it would be better to simply not show the dictionary in the inspector at all. Also this avoids the problem of the text going over the center line.

cannot be a type derived (directly or indirectly) from UnityEngine.Object.

While I see what you mean, this would be a needless restriction. For one, dictionaries can be set through code, and in that case a UnityEngine.Object is completely valid key type. After thinking about it a bit, I think a better way to handle it would be to add a clause in the serialization to handle null UnityEngine.Objects like duplicate keys are handled.

needs to be equatable.

I'm sorry I don't follow this one. Can you try to explain the need for this to me?

If you want, I can make these changes my self and just close this PR. Or if you would like to that is great!

[stefanbursuc]

I think to be inline with how Unity handles this currently with lists, it would be better to simply not show the dictionary in the inspector at all. Also this avoids the problem of the text going over the center line.

Sure, as long as you are trying to be as similar to how Unity handles these cases with lists for example. Although these kind of messages could help the user to know why his UDictionary is not serialized (i.e.: is the key or the value unserializable).

While I see what you mean, this would be a needless restriction. For one, dictionaries can be set through code, and in that case a UnityEngine.Object is completely valid key type. After thinking about it a bit, I think a better way to handle it would be to add a clause in the serialization to handle null UnityEngine.Objects like duplicate keys are handled.

If you want to allow UnityEngine.Objects to be used as keys, then the key value needs to be checked against null in OnAfterDeserialize. Currently the call to ContainsKey throws ArgumentNullException. Or, I guess, just need to make sure that we don't serialize null keys.

I'm sorry I don't follow this one. Can you try to explain the need for this to me?

I was thinking that for an end user (i.e. a designer that populates the dictionary from the editor) it shouldn't be a difference on how the key is seen as being a duplicate or not based on if the key is a class or a struct. Without the class overriding those two functions, two keys with the same fields values will still be seen as different keys (as by default the comparison is done by comparing the refences). I attached an image to make the end user perspective clearer when using structs vs classes.

[stefanbursuc]

But I understand that the goal here is to make this dictionary implementation as close to how Unity would have implemented it. So, some of my proposals would break that :)

I don't mind if you want to make the changes you think are worth it by yourself ;)

[stefanbursuc]

I'm sorry I don't follow this one. Can you try to explain the need for this to me?

I was thinking that for an end user (i.e. a designer that populates the dictionary from the editor) it shouldn't be a difference on how the key is seen as being a duplicate or not based on if the key is a class or a struct. Without the class overriding those two functions, two keys with the same fields values will still be seen as different keys (as by default the comparison is done by comparing the refences). I attached an image to make the end user perspective clearer when using structs vs classes.

Thinking a bit more on this, I think that it doesn't make sense to serialize keys that have the same field values.

If as long as the dictionary is created and used at runtime in the same session, you can lets say have a reference to a key and can retrieve the value by using that key, the key's actual reference meaning is lost when is serialized, as when this happens, only the field's values are serialized.

In this case when you deserialize, you cannot distinguish anymore two keys with the same field values. For example, you cannot retrieve a specific value by creating a key object and populate it's fields with the desired query.

I.e. From the image from previous post, creating a key object with values:

someInt = 0  
someFloat = 0  
someString = "a"  

Will not return either of those two dictionary entries.

Or look at the native string class, for example. It would be pretty useless to be used as a key, if it didn't override the two mentioned functions.