fix: yaml, rollback trimming of the override tag when parsing collection index to ensure inputs can be re-used

[Eideren]

PR Details

The parsing events, which are effectively tokens that the deserializer reads to create the final object, are mutated when deserializing. If you were to re-use those parsing events a second time, it would generate the same object, but the metadata would be partially lost.

A reader-like object should not be writing to the stream it is reading from.

The spot where it is called twice with the same parsing events is located here:

stride/sources/editor/Stride.Assets.Presentation/AssemblyReloading/GameStudioAssemblyReloader.cs

Lines 423 to 466 in 8d3547a

	protected override bool ProcessObject(object obj, Type expectedType)
	{
	foreach (var unloadedObject in ItemsToReload)
	{
	// If a collection, stop at parent path level (since index will be already removed, we will never visit the target slot)
	// TODO: Check if the fact we didn't enter in an item with index affect visitor states
	// Other case, stop on the actual member (since we'll just visit null)
	var expectedPath = unloadedObject.Path.Decompose().LastOrDefault()?.GetIndex() != null ? unloadedObject.ParentPath : unloadedObject.Path;
	if (CurrentPath.Match(expectedPath))
	{
	var eventReader = new EventReader(new MemoryParser(unloadedObject.ParsingEvents));
	var settings = Log != null ? new SerializerContextSettings { Logger = Log } : null;
	PropertyContainer properties;
	unloadedObject.UpdatedObject = AssetYamlSerializer.Default.Deserialize(eventReader, null, unloadedObject.ExpectedType, out properties, settings);
	// We will have broken references here because we are deserializing objects individually, so we don't pass any logger to discard warnings
	var metadata = YamlAssetSerializer.CreateAndProcessMetadata(properties, unloadedObject.UpdatedObject, false);
	var overrides = metadata.RetrieveMetadata(AssetObjectSerializerBackend.OverrideDictionaryKey);
	unloadedObject.Overrides = overrides;
	var references = metadata.RetrieveMetadata(AssetObjectSerializerBackend.ObjectReferencesKey);
	if (references != null)
	{
	var basePath = YamlAssetPath.FromMemberPath(CurrentPath, root);
	foreach (var reference in references)
	{
	var basePathWithIndex = basePath.Clone();
	if (unloadedObject.GraphPathIndex != NodeIndex.Empty)
	{
	if (unloadedObject.ItemId == ItemId.Empty)
	basePathWithIndex.PushIndex(unloadedObject.GraphPathIndex.Value);
	else
	basePathWithIndex.PushItemId(unloadedObject.ItemId);
	}
	var actualPath = basePathWithIndex.Append(reference.Key);
	ObjectReferences.Set(actualPath, reference.Value);
	}
	}
	}
	}
	return false;
	}
	}

And it is because

stride/sources/editor/Stride.Assets.Presentation/AssemblyReloading/GameStudioAssemblyReloader.cs

Lines 300 to 305 in 8d3547a

	public override void VisitObjectMember(object container, ObjectDescriptor containerDescriptor, IMemberDescriptor member, object value)
	{
	if (ProcessObject(value, member.TypeDescriptor.Type)) return;
	base.VisitObjectMember(container, containerDescriptor, member, value);
	}

Calls ProcessObject twice for the same object/path combination, once right in this scope, another one through VisitObject down the base call.
I have another PR covering that scope.

Related Issue

Fix: #2716
The previous logic processed the objects twice, see the blurb about ProcessObject above. The first pass consumes the override tag when reading the yaml events because of the key.Value = keyName; there

stride/sources/assets/Stride.Core.Assets/Yaml/AssetObjectSerializerBackend.cs

Lines 153 to 159 in 8d3547a

	public override object ReadDictionaryKey(ref ObjectContext objectContext, Type keyType)
	{
	var key = objectContext.Reader.Peek<Scalar>();
	var keyName = TrimAndParseOverride(key.Value, out var overrideTypes);
	key.Value = keyName;
	var keyValue = base.ReadDictionaryKey(ref objectContext, keyType);

It would remove the override tag, so the next read would be without the override metadata, which in term gets the collection item removed as it is not part of its base asset.

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]

@Eideren Can you provide one or two YAML samples (e.g. a few stride assets) with the issue highlighted, and/or a screenshot if the issue is only "visible" in the editor?

Ideally, we should also have a test covering that case.

[Eideren]

@Kryptos-FR sure, added in a test and appended a repro and yaml snippets to #2716 instead of here, it seemed more appropriate.

[Kryptos-FR]

LGTM. Thanks for the test case.