-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate key and value types #3
base: main
Are you sure you want to change the base?
Validate key and value types #3
Conversation
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.
Hi. You can also take a look at (and integrate) the branch: That contains examples of multiple types for keys and values. |
Hi, first, thank you for taking the time to create this PR!
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.
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.
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! |
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).
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 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. |
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 ;) |
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:
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. |
We want to restrict the dictionary's key and value types as follows:
Key:
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).
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: