refactor: Nullable and Modernization for Core projects#2643
Conversation
c023453 to
b133105
Compare
|
Note: I will do a few more projects in that PR ( |
8e01cdb to
f3a7b5a
Compare
f3a7b5a to
8c62e52
Compare
|
I finished all projects under One notable exception is Note: not all nullable warnings are resolved. Some would require more thought and possibly breaking changes so I let them. |
Especially those targeting .NET Framework
947dce8 to
9d34d93
Compare
|
Welp, this one is going to create a whole bunch of merge conflicts, are you okay with taking care of the PRs that might be conflicting with this one if I were to merge this PR now ? |
|
@Eideren I can take care of the conflicts assuming all those PRs allow maintainers to do changes. |
PR Details
Similar to #2639.
Description
As in the previous PR, enable C# built-in nullable annotations on the project and remove references to our old custom ones. In addition, modernization of the code (pattern matching, file-scoped namespaces, collection initialization, etc.
Equals
Worth of note is a fix of the
Equals(object?)implementation for some structs inCore.Mathematics. While it was unlikely to be used (as the same structs also define a type version ofEquals), the implementation was doing unnecessary checks and struct boxing (which also had Resharper detect the implementation as mutating).Consider the previous implementation for
Vector3:The corresponding IL code is:
We can clearly see the virtual calls necessary for comparing the types as well as the boxing required for such call.
The new implementation is simpler:
And the corresponding IL also:
Hashcode
Another point of improvement is the computation of hashcodes. In lots of places we had a custom implementation that wasn't necessary efficient or even correct (sometimes it was missing the
uncheckedcontext). .NET provides a battle-tested implementation with the right properties and efficiency.Our previous implementation for
Matrix:Now, using
Hashcode.Combine:Swapping
Another fun improvement is how to swap two values. The usual implementation was to use a temporary variable:
We can now use a value tuple:
In my opinion it does improve readability when applied to
Matrix.Transpose.Before:
After:
Related Issue
#2155
#2156
Types of changes
Checklist