Skip to content

[GameStudio] Crash whenever the user changes a struct value in the property grid #2427

@Eideren

Description

@Eideren

Release Type: Latest built in debug

Describe the bug
Let's say you have a

public class MyComp : StartupScript
{
        public List<Test2> MyField = new List<Test2>();
        [DataContract] public struct Test2{ public bool b; }
}

Adding a new item and setting its b field to true in the property grid crashes the gamestudio.


Why ? An AssetMemberNode instance runs its Update function, note the NotifyContentChanging and NotifyContentChanged below:

private void Update(object newValue, bool sendNotification)
{
var oldValue = Retrieve();
MemberNodeChangeEventArgs args = null;
if (sendNotification)
{
args = new MemberNodeChangeEventArgs(this, oldValue, newValue);
NotifyContentChanging(args);
}
var containerValue = Parent.Retrieve();
if (containerValue == null)
throw new InvalidOperationException("Container's value is null");
MemberDescriptor.Set(containerValue, ConvertValue(newValue, MemberDescriptor.Type));
if (containerValue.GetType().GetTypeInfo().IsValueType)
((GraphNodeBase)Parent).UpdateFromMember(containerValue, NodeIndex.Empty);
UpdateReferences();
if (sendNotification)
{
NotifyContentChanged(args);
}
}

NotifyContentChanging calls OnPropertyChanged on an AssetNodeViewModel instance with name b and value false further down the stack:

protected virtual void OnPropertyChanging(params string[] propertyNames)
{
var propertyChanging = PropertyChanging;
foreach (var propertyName in propertyNames)
{
#if DEBUG
if (changingProperties.Contains(propertyName))
throw new InvalidOperationException($"OnPropertyChanging called twice for property '{propertyName}' without invoking OnPropertyChanged between calls.");
changingProperties.Add(propertyName);
#endif
propertyChanging?.Invoke(this, new PropertyChangingEventArgs(propertyName));
if (DependentProperties.TryGetValue(propertyName, out var dependentProperties))
{
OnPropertyChanging(dependentProperties);
}
}
}

Notice the hashset insertion wrapped inside a DEBUG scope.

After this, NotifyContentChanged from the first snippet runs, creating a new AssetNodeViewModel with name b and value true, then immediately calls

protected virtual void OnPropertyChanged(params string[] propertyNames)
{
var propertyChanged = PropertyChanged;
for (var i = 0; i < propertyNames.Length; ++i)
{
var propertyName = propertyNames[propertyNames.Length - 1 - i];
if (DependentProperties.TryGetValue(propertyName, out var dependentProperties))
{
var reverseList = new string[dependentProperties.Length];
for (var j = 0; j < dependentProperties.Length; ++j)
reverseList[j] = dependentProperties[dependentProperties.Length - 1 - j];
OnPropertyChanged(reverseList);
}
propertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
#if DEBUG
if (!changingProperties.Contains(propertyName))
throw new InvalidOperationException($"OnPropertyChanged called for property '{propertyName}' but OnPropertyChanging was not invoked before.");
changingProperties.Remove(propertyName);
#endif
}
}
on that new instance failing the hashset test since it's an all new instance.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions