Add FieldLoader/Saver support for ImmutableArray, FrozenSet, FrozenDictionary.

[RoosterDragon]

As config is often meant to be loaded and then never modified, it is helpful to support collections that are read-only after creation. This allows various classes that load config from YAML and elsewhere to deserialize straight into read-only collections and enforce invariants around data mutation.

Change classes that use FieldLoader to use read-only collections.

Fixes #22033

[anvilvapre]

untested.

curious if it would be noticeable faster.

curious if/how it would cause for more efficient dictionaries.

[RoosterDragon]

There's an overhead at load time - about twice the cost to create a read-only collection over a mutable one.

The Frozen sets/dicts will improve lookup performance, since that's what those types do. That said most of our sets/dicts from config have so few elements I don't imagine it'll be noticable.

Overall, this PR is likely neutral performance wise. The main benefit of the PR is to express that these various collections cannot be modified, which will help prevent bugs.

[PunkPun]

If we added hotloading, I assume we'd do it by replacing the immutable collections? Updating the readonly variables?

[PunkPun]

I wonder if this breaks documentation

[PunkPun]

freeze?

[RoosterDragon]

This set won't live very long, so I've chosen to use a regular set to avoid the additional overhead of creating a frozen one.

But both are valid enough options.

[PunkPun]

freeze?

[RoosterDragon]

These are also short lived.

[PunkPun]

maybe to wait for #22163 before merge as that adds newer dotnet versions add a collection expression for empty frozen sets?

[RoosterDragon]

If we added hotloading, I assume we'd do it by replacing the immutable collections? Updating the readonly variables?

If depends where hotloading wants to inject the "actually we might change this at runtime" concept, since currently everything can variously assume the trait info never changes and cache things based on that.

I'd be tempted to use "TraitInfos are immutable, but we can swap in a new TraitInfo at runtime". This allows the TraitInfo itself to cache whatever things it likes for performance. Then traits can reference a traitinfo but must always perform live lookups against the info, in case a new traitinfo info gets swapped in later.

This would avoid an issue that you have even with mutable collections, where if the trait captures the collection into an enumerable or something, swapping out a new one won't help anyway.

e.g. var filtered = info.SomeCollections.Where(...); captures info.SomeCollection, so swapping in a new SomeCollection won't help, as filtered still uses the old one.

I wonder if this breaks documentation

I've made some fixes and run a diff before/after - docs are the same except for the InternalTypes (the backing C# types) changing. Friendly descriptions are available for the new types.

maybe to wait for #22163 before merge as that adds newer dotnet versions add a collection expression for empty frozen sets?

It'll be cleaner with that for sure. But the style rule can do the cleanup for us even if we merge this first.

[pchote]

My preference / suggestion would be to restrict hot-loading to the full Ruleset/World to avoid any issues with cached state.

[PunkPun]

Chat localisations have regressed

Otherwise surface level testing didn't reveal anything else

[RoosterDragon]

Chat localisations have regressed

Nice spot, fixed.

[PunkPun]

👍

[pchote]

This touches a lot of code, so lets take this now to reduce the chance of bitrot / rebase collisions.

[pchote]

https://github.com/OpenRA/OpenRA/wiki/Changelog-(bleed)/_compare/70b57b9b9edf5f42720e14cfaa29653c0c2413fc