From 33e1a6b2dda2660ed38c6062674544c73dff62b4 Mon Sep 17 00:00:00 2001 From: Curtis Shmyr Date: Sun, 13 Nov 2016 07:09:47 +0000 Subject: [PATCH] Fix frame end task race condition in ScrollPanelWidget --- .../Primitives/IObservableCollection.cs | 10 +++--- .../Primitives/ObservableCollection.cs | 18 +++++------ .../Primitives/ObservableDictionary.cs | 18 +++++------ OpenRA.Game/Primitives/ObservableList.cs | 24 +++++++------- .../Widgets/ScrollPanelWidget.cs | 31 +++++++++++++++---- 5 files changed, 60 insertions(+), 41 deletions(-) diff --git a/OpenRA.Game/Primitives/IObservableCollection.cs b/OpenRA.Game/Primitives/IObservableCollection.cs index 21ff968f90..94927f9803 100644 --- a/OpenRA.Game/Primitives/IObservableCollection.cs +++ b/OpenRA.Game/Primitives/IObservableCollection.cs @@ -16,11 +16,11 @@ namespace OpenRA.Primitives { public interface IObservableCollection { - event Action OnAdd; - event Action OnRemove; - event Action OnRemoveAt; - event Action OnSet; - event Action OnRefresh; + event Action OnAdd; + event Action OnRemove; + event Action OnRemoveAt; + event Action OnSet; + event Action OnRefresh; IEnumerable ObservedItems { get; } } } diff --git a/OpenRA.Game/Primitives/ObservableCollection.cs b/OpenRA.Game/Primitives/ObservableCollection.cs index 08e4ed48fc..92781bf75d 100644 --- a/OpenRA.Game/Primitives/ObservableCollection.cs +++ b/OpenRA.Game/Primitives/ObservableCollection.cs @@ -18,15 +18,15 @@ namespace OpenRA.Primitives { public class ObservableCollection : Collection, IObservableCollection { - public event Action OnAdd = k => { }; + public event Action OnAdd = (x, k) => { }; // TODO Workaround for https://github.com/OpenRA/OpenRA/issues/6101 #pragma warning disable 67 - public event Action OnRemove = k => { }; + public event Action OnRemove = (x, k) => { }; #pragma warning restore - public event Action OnRemoveAt = i => { }; - public event Action OnSet = (o, n) => { }; - public event Action OnRefresh = () => { }; + public event Action OnRemoveAt = (x, i) => { }; + public event Action OnSet = (x, o, n) => { }; + public event Action OnRefresh = x => { }; public ObservableCollection() { } public ObservableCollection(IList list) : base(list) { } @@ -35,25 +35,25 @@ namespace OpenRA.Primitives { var old = this[index]; base.SetItem(index, item); - OnSet(old, item); + OnSet(this, old, item); } protected override void InsertItem(int index, T item) { base.InsertItem(index, item); - OnAdd(item); + OnAdd(this, item); } protected override void ClearItems() { base.ClearItems(); - OnRefresh(); + OnRefresh(this); } protected override void RemoveItem(int index) { base.RemoveItem(index); - OnRemoveAt(index); + OnRemoveAt(this, index); } public IEnumerable ObservedItems diff --git a/OpenRA.Game/Primitives/ObservableDictionary.cs b/OpenRA.Game/Primitives/ObservableDictionary.cs index 8d823efa35..81f68c3402 100644 --- a/OpenRA.Game/Primitives/ObservableDictionary.cs +++ b/OpenRA.Game/Primitives/ObservableDictionary.cs @@ -33,19 +33,19 @@ namespace OpenRA.Primitives { protected IDictionary innerDict; - public event Action OnAdd = k => { }; - public event Action OnRemove = k => { }; + public event Action OnAdd = (x, k) => { }; + public event Action OnRemove = (x, k) => { }; // TODO Workaround for https://github.com/OpenRA/OpenRA/issues/6101 #pragma warning disable 67 - public event Action OnRemoveAt = i => { }; - public event Action OnSet = (o, n) => { }; + public event Action OnRemoveAt = (x, i) => { }; + public event Action OnSet = (x, o, n) => { }; #pragma warning restore - public event Action OnRefresh = () => { }; + public event Action OnRefresh = x => { }; protected void FireOnRefresh() { - OnRefresh(); + OnRefresh(this); } protected ObservableDictionary() { } @@ -58,14 +58,14 @@ namespace OpenRA.Primitives public virtual void Add(TKey key, TValue value) { innerDict.Add(key, value); - OnAdd(key); + OnAdd(this, key); } public bool Remove(TKey key) { var found = innerDict.Remove(key); if (found) - OnRemove(key); + OnRemove(this, key); return found; } @@ -91,7 +91,7 @@ namespace OpenRA.Primitives public void Clear() { innerDict.Clear(); - OnRefresh(); + OnRefresh(this); } public int Count diff --git a/OpenRA.Game/Primitives/ObservableList.cs b/OpenRA.Game/Primitives/ObservableList.cs index 61a9d05c30..64e810cc25 100644 --- a/OpenRA.Game/Primitives/ObservableList.cs +++ b/OpenRA.Game/Primitives/ObservableList.cs @@ -19,19 +19,19 @@ namespace OpenRA.Primitives { protected IList innerList; - public event Action OnAdd = k => { }; - public event Action OnRemove = k => { }; + public event Action OnAdd = (x, k) => { }; + public event Action OnRemove = (x, k) => { }; // TODO Workaround for https://github.com/OpenRA/OpenRA/issues/6101 #pragma warning disable 67 - public event Action OnRemoveAt = i => { }; - public event Action OnSet = (o, n) => { }; + public event Action OnRemoveAt = (x, i) => { }; + public event Action OnSet = (x, o, n) => { }; #pragma warning restore - public event Action OnRefresh = () => { }; + public event Action OnRefresh = x => { }; protected void FireOnRefresh() { - OnRefresh(); + OnRefresh(this); } public ObservableList() @@ -42,14 +42,14 @@ namespace OpenRA.Primitives public virtual void Add(T item) { innerList.Add(item); - OnAdd(item); + OnAdd(this, item); } public bool Remove(T item) { var found = innerList.Remove(item); if (found) - OnRemove(item); + OnRemove(this, item); return found; } @@ -57,13 +57,13 @@ namespace OpenRA.Primitives public void Clear() { innerList.Clear(); - OnRefresh(); + OnRefresh(this); } public void Insert(int index, T item) { innerList.Insert(index, item); - OnRefresh(); + OnRefresh(this); } public int Count { get { return innerList.Count; } } @@ -73,7 +73,7 @@ namespace OpenRA.Primitives public void RemoveAt(int index) { innerList.RemoveAt(index); - OnRemoveAt(index); + OnRemoveAt(this, index); } public T this[int index] @@ -87,7 +87,7 @@ namespace OpenRA.Primitives { var oldValue = innerList[index]; innerList[index] = value; - OnSet(oldValue, value); + OnSet(this, oldValue, value); } } diff --git a/OpenRA.Mods.Common/Widgets/ScrollPanelWidget.cs b/OpenRA.Mods.Common/Widgets/ScrollPanelWidget.cs index 599ad173d3..517105b716 100644 --- a/OpenRA.Mods.Common/Widgets/ScrollPanelWidget.cs +++ b/OpenRA.Mods.Common/Widgets/ScrollPanelWidget.cs @@ -374,9 +374,15 @@ namespace OpenRA.Mods.Common.Widgets }); } - void BindingAdd(object item) + void BindingAdd(IObservableCollection col, object item) { - Game.RunAfterTick(() => BindingAddImpl(item)); + Game.RunAfterTick(() => + { + if (collection != col) + return; + + BindingAddImpl(item); + }); } void BindingAddImpl(object item) @@ -393,30 +399,40 @@ namespace OpenRA.Mods.Common.Widgets ScrollToBottom(); } - void BindingRemove(object item) + void BindingRemove(IObservableCollection col, object item) { Game.RunAfterTick(() => { + if (collection != col) + return; + var widget = Children.FirstOrDefault(w => widgetItemEquals(w, item)); if (widget != null) RemoveChild(widget); }); } - void BindingRemoveAt(int index) + void BindingRemoveAt(IObservableCollection col, int index) { Game.RunAfterTick(() => { + if (collection != col) + return; + if (index < 0 || index >= Children.Count) return; + RemoveChild(Children[index]); }); } - void BindingSet(object oldItem, object newItem) + void BindingSet(IObservableCollection col, object oldItem, object newItem) { Game.RunAfterTick(() => { + if (collection != col) + return; + var newWidget = makeWidget(newItem); newWidget.Parent = this; @@ -433,10 +449,13 @@ namespace OpenRA.Mods.Common.Widgets }); } - void BindingRefresh() + void BindingRefresh(IObservableCollection col) { Game.RunAfterTick(() => { + if (collection != col) + return; + RemoveChildren(); foreach (var item in collection.ObservedItems) BindingAddImpl(item);