GetNiceFullName incorrectly formats array types

[Nihlus]

Describe the bug?

GetNiceFullName() produces incorrect output for typeof(DynamicBoneChain.BoneData[]) and other array types. As far as I understand it, the intended output is "FrooxEngine.DynamicBoneChain+BoneData[]"; however, the actual output is

• ".BoneData[]" on Mono
• "FrooxEngine.BoneData[]" on .NET 8

This causes FrooxEngine's compatibility hash to be different on different runtimes, resulting in sessions that should ostensibly be compatible with each other to not be.

To Reproduce

Compile and run the following code on Mono and .NET 8.

var name = typeof(DynamicBoneChain.BoneData[]).GetNiceFullName();
Console.WriteLine(name);

Observe output.

Expected behavior

GetNiceFullName should produce expected output. The output should be identical across different runtimes.

Screenshots

No response

Resonite Version Number

2024.7.8.1130

What Platforms does this occur on?

Windows, Linux

What headset if any do you use?

No response

Log Files

No log files are applicable.

Additional Context

The direct cause of the issue is most likely that DeclaringType is null on the array type. On .NET 8, DeclaringType is also null, but Namespace is correctly inherited from the outer type. GetNiceFullName or FormatType should probably check for an array type and then format the base type name based on the element type of the array type (GetElementType())

Additionally, GetNiceFullName doesn't take into account whether t.Namespace might be empty, and returns a strangely-formatted string when operating on a type declared in a global namespace (as is the case with BoneData on Mono, having no declaring type and no namespace).

Reporters

No response

[Frooxius]

I am confused by this report. Right now we do not support .NET 8?

[Nihlus]

The .NET 8 part comes by way of https://github.com/Nihlus/Crystite, where I've got a headless running on .NET 8.

The fact that it's different on different runtimes is more of a parenthesis right now, to be fair - the core thing about the report is that GetNiceTypeName doesn't correctly format array types, since it doesn't include parent types in the name.

If you have two types with the same name but nested in different types, GetNiceTypeName would produce the same result if operating on an array type of those element types.

[shiftyscales]

Can you please verify that this issue occurs on official / unmodified clients, @Nihlus?

[BlueCyro]

We currently don't support .NET 8 officially. If this is in fact due to a mod, then we can't make any guarantees about whether this behavior is right or wrong since the issue would have arisen from a usage of the program that isn't officially supported. (I.e. unofficially patching it to run on a newer runtime through a variety of methods)

The gesture to give us a heads up is very much appreciated, but unless this happens on vanilla in some capacity, we can't action this since it's not part of an official implementation and may differ from how this is handled internally. The issue tracker is meant for issues regarding the stock experience of the platform.

[Nihlus]

The issue occurs on the stock client without any mods. It's not related to any modifications - rather, it's a purely implementation-related bug in the mentioned function.

Array types are distinct types in the C# type model, and they live in the root namespace without any nesting regardless of the element type. GetNiceFullName and FormatType doesn't have any handling for array types at all, resulting in an incorrect output in any case involving such a type. Most commonly, this happens for parameter types in delegates.

[Frooxius]

I'll need to look into this more, but I've fixed the namespace being included when there's none for now in 2024.7.9.1044

[Nihlus]

Thanks for the quick turnaround! The fix does take care of the broken output (leading . which is definitely invalid as a type name), but the actual underlying problem still remains. For the type in the original example - DynamicBoneChain.BoneData - the intended output is probably FrooxEngine.DynamicBoneChain+BoneData[] instead of BoneData[] as is produced using the latest updated code.

My suggestion would be to check for HasElementType in similar fashion to how nullables are checked for and explicitly handled in FormatType, and then do a recursive call on the element type to get the full, properly-formatted name which you'd then append the encompassing type's symbol to. As C# is today, encompassing types would be arrays (followed by []), pointers (followed by *) and references (in, out, and ref, which all are followed by a & in the full typename). I couldn't find any instances of a reference producing incorrect output, but I'd imagine it's because none of them that are considered as part of the data model are also a nested type.

EDIT: I'm @ jgullberg in the Discord server if you want to chat live or get any clarifications about what I'm on about - feel free to ping me anytime :smile:

[Frooxius]

Yes, like I said I need to look more into this more and figure the best approach, but right now other issues are higher priority, so I haven't been able to yet.

[Nihlus]

Yep, I totally get that!

[Frooxius]

This has now been fixed in 2024.7.10.1077! I've implemented proper decomposition/re-composition of array types with our type formatter & parser, so any combination should work properly and be consistent.

Thanks, closing!