refactor: Nullable and Modernization for Core projects

[Kryptos-FR]

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 in Core.Mathematics. While it was unlikely to be used (as the same structs also define a type version of Equals), the implementation was doing unnecessary checks and struct boxing (which also had Resharper detect the implementation as mutating).

Consider the previous implementation for Vector3:

public override bool Equals(object value)
{
    if (value == null)
        return false;

    if (value.GetType() != GetType())
        return false;

     return Equals((Vector3)value);
}

The corresponding IL code is:

IL_0000: ldarg.1
IL_0001: brtrue.s IL_0005

IL_0003: ldc.i4.0
IL_0004: ret

IL_0005: ldarg.1
IL_0006: callvirt instance class [System.Runtime]System.Type [System.Runtime]System.Object::GetType()
IL_000b: ldarg.0
IL_000c: ldobj Vector3
IL_0011: box Vector3
IL_0016: call instance class [System.Runtime]System.Type [System.Runtime]System.Object::GetType()
IL_001b: call bool [System.Runtime]System.Type::op_Inequality(class [System.Runtime]System.Type, class [System.Runtime]System.Type)
IL_0020: brfalse.s IL_0024

IL_0022: ldc.i4.0
IL_0023: ret

IL_0024: ldarg.0
IL_0025: ldarg.1
IL_0026: unbox.any Vector3
IL_002b: call instance bool Vector3::Equals(valuetype Vector3)
IL_0030: ret

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:

public override readonly bool Equals(object? value)
{
    return value is Vector3 vector && Equals(vector);
}

And the corresponding IL also:

IL_0000: ldarg.1
IL_0001: isinst Vector3
IL_0006: brfalse.s IL_0017

IL_0008: ldarg.1
IL_0009: unbox.any Vector3
IL_000e: stloc.0
IL_000f: ldarg.0
IL_0010: ldloc.0
IL_0011: call instance bool Vector3::Equals(valuetype Vector3)
IL_0016: ret

IL_0017: ldc.i4.0
IL_0018: ret

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 unchecked context). .NET provides a battle-tested implementation with the right properties and efficiency.

Our previous implementation for Matrix:

public override int GetHashCode()
{
    return M11.GetHashCode() + M12.GetHashCode() + M13.GetHashCode() + M14.GetHashCode() +
        M21.GetHashCode() + M22.GetHashCode() + M23.GetHashCode() + M24.GetHashCode() +
        M31.GetHashCode() + M32.GetHashCode() + M33.GetHashCode() + M34.GetHashCode() +
        M41.GetHashCode() + M42.GetHashCode() + M43.GetHashCode() + M44.GetHashCode();
}

Now, using Hashcode.Combine:

public override readonly int GetHashCode()
{
    return HashCode.Combine(
        HashCode.Combine(M11, M12, M13, M14),
        HashCode.Combine(M21, M22, M23, M24),
        HashCode.Combine(M31, M32, M33, M34),
        HashCode.Combine(M41, M42, M43, M44)
    );
}

Swapping

Another fun improvement is how to swap two values. The usual implementation was to use a temporary variable:

var tmp = a;
a = b;
b = tmp;

We can now use a value tuple:

(a, b) = (b, a);

In my opinion it does improve readability when applied to Matrix.Transpose.
Before:

public void Transpose()
{
    float temp;
    temp = M21; M21 = M12; M12 = temp;
    temp = M31; M31 = M13; M13 = temp;
    temp = M41; M41 = M14; M14 = temp;
    
    temp = M32; M32 = M23; M23 = temp;
    temp = M42; M42 = M24; M24 = temp;
    
    temp = M43; M43 = M34; M34 = temp;
}

After:

public void Transpose()
{
    (M21, M12) = (M12, M21);
    (M31, M13) = (M13, M31);
    (M41, M14) = (M14, M41);
    
    (M32, M23) = (M23, M32);
    (M42, M24) = (M24, M42);
    
    (M43, M34) = (M34, M43);
}

Related Issue

#2155
#2156

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My change requires a change to the documentation.
• I have added tests to cover my changes.
• All new and existing tests passed.
• I have built and run the editor to try this change out.

[Kryptos-FR]

Note: I will do a few more projects in that PR (Core.Yaml, Core.Design, Core.Translation) as well as their corresponding test projects.

[Kryptos-FR]

I finished all projects under sources/core.

One notable exception is Stride.Core.Yaml. I think we should review replacing it with https://github.com/aaubry/YamlDotNet/ (as it was based on an old version of it), and thus I thought it was not worth the effort modernizing/annotating it.

Note: not all nullable warnings are resolved. Some would require more thought and possibly breaking changes so I let them.

[Eideren]

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 ?

[Kryptos-FR]

@Eideren I can take care of the conflicts assuming all those PRs allow maintainers to do changes.
Feel free to list them here after the merge so I can follow up (or ping me on the related PR).

[Kryptos-FR]

PR with conflicts following the merge:

• [Draft] Remove depth from Stride UI #2605 ✅
• Deprecate stride math for numerics alternatives #2579 ✅
• Reformat startup WIP #2574 ✅
• feat: Remove AssemblyProcessor unused option #2481 ✅
• Drop support for UWP #2450 ✅
• WIP: Windows on Arm support #2397 (might be closed in favor of feat: Adds native dependencies for win-arm64 #2644)
• fix : Metrics - Remove all data collection from Stride.Engine #2261 ✅
• Remove BuildEngine project #2208 ✅
• Let assembly reloading bypass undo/redo to improve editor stability #2108 ✅
• [Core] Performance fixes in GameProfilingSystem and ChromeTracingProfileWriter #2050
• [WIP] Convert from SharpDX to Silk.NET. #1715 (will be dealt with by the author)