Change transactions now properly only finish when all transactions exit, not just the first

IPA.Loader/Config/Stores/GeneratedStoreImpl/IGeneratedStore.cs

@@ -42,8 +42,7 @@ namespace IPA.Config.Stores
         internal class Impl : IConfigStore
         {
             private readonly IGeneratedStore generated;
-            private bool inChangeTransaction = false;
-            //private bool changedInTransaction = false;
+            private long enteredTransactions = 0;
 
             internal static ConstructorInfo Ctor = typeof(Impl).GetConstructor(new[] { typeof(IGeneratedStore) });
             public Impl(IGeneratedStore store) => generated = store;
@@ -104,7 +103,7 @@ namespace IPA.Config.Stores
             public static IDisposable ImplChangeTransaction(IGeneratedStore s, IDisposable nest) => FindImpl(s).ChangeTransaction(nest);
             // TODO: improve trasactionals so they don't always save in every case
             public IDisposable ChangeTransaction(IDisposable nest, bool takeWrite = true)
-                => GetFreeTransaction().InitWith(this, !inChangeTransaction, nest, takeWrite && !WriteSyncObject.IsWriteLockHeld);
+                => GetFreeTransaction().InitWith(this, nest, takeWrite && !WriteSyncObject.IsWriteLockHeld);
 
             private ChangeTransactionObj GetFreeTransaction()
                 => freeTransactionObjs.Count > 0 ? freeTransactionObjs.Pop()
@@ -117,23 +116,21 @@ namespace IPA.Config.Stores
                 private struct Data
                 {
                     public readonly Impl impl;
-                    public readonly bool owns;
                     public readonly bool ownsWrite;
                     public readonly IDisposable nested;
 
-                    public Data(Impl impl, bool owning, bool takeWrite, IDisposable nest)
+                    public Data(Impl impl, bool takeWrite, IDisposable nest)
                     {
-                        this.impl = impl; owns = owning; ownsWrite = takeWrite; nested = nest;
+                        this.impl = impl; ownsWrite = takeWrite; nested = nest;
                     }
                 }
                 private Data data;
 
-                public ChangeTransactionObj InitWith(Impl impl, bool owning, IDisposable nest, bool takeWrite)
+                public ChangeTransactionObj InitWith(Impl impl, IDisposable nest, bool takeWrite)
                 {
-                    data = new Data(impl, owning, takeWrite, nest);
+                    data = new Data(impl, takeWrite, nest);
 
-                    if (data.owns)
-                        impl.inChangeTransaction = true;
+                    _ = Interlocked.Increment(ref impl.enteredTransactions);
                     if (data.ownsWrite)
                         impl.TakeWrite();
 
@@ -143,9 +140,8 @@ namespace IPA.Config.Stores
                 public void Dispose() => Dispose(true);
                 private void Dispose(bool addToStore)
                 {
-                    if (data.owns)
+                    if (data.impl != null && Interlocked.Decrement(ref data.impl.enteredTransactions) == 0)
                     {
-                        data.impl.inChangeTransaction = false;
                         data.impl.InvokeChanged();
                     }
                     data.nested?.Dispose();
@@ -157,6 +153,9 @@ namespace IPA.Config.Stores
                     catch
                     {
                     }
+
+                    data = default;
+
                     if (addToStore)
                         freeTransactionObjs.Push(this);
                 }
@@ -170,7 +169,6 @@ namespace IPA.Config.Stores
                 return store?.Impl;
             }
 
-
             internal static MethodInfo ImplReadFromMethod = typeof(Impl).GetMethod(nameof(ImplReadFrom));
             public static void ImplReadFrom(IGeneratedStore s, ConfigProvider provider) => FindImpl(s).ReadFrom(provider);
             public void ReadFrom(ConfigProvider provider)

IPA.Loader/Config/Stores/GeneratedStoreImpl/MakeCreator.cs

@@ -62,39 +62,6 @@ namespace IPA.Config.Stores
             var implField = typeBuilder.DefineField("<>_impl", typeof(Impl), FieldAttributes.Private | FieldAttributes.InitOnly);
             var parentField = typeBuilder.DefineField("<>_parent", typeof(IGeneratedStore), FieldAttributes.Private | FieldAttributes.InitOnly);
 
-            /*#region Converter fields
-            var uniqueConverterTypes = structure.Where(m => m.HasConverter).Select(m => m.Converter).Distinct().ToArray();
-            var converterFields = new Dictionary<Type, FieldInfo>(uniqueConverterTypes.Length);
-
-            foreach (var convType in uniqueConverterTypes)
-            {
-                var field = typeBuilder.DefineField($"<converter>_{convType}", convType,
-                    FieldAttributes.Private | FieldAttributes.InitOnly | FieldAttributes.Static);
-                converterFields.Add(convType, field);
-
-                foreach (var member in structure.Where(m => m.HasConverter && m.Converter == convType))
-                    member.ConverterField = field;
-            }
-            #endregion
-
-            #region Static constructor
-            var cctor = typeBuilder.DefineConstructor(MethodAttributes.Static, CallingConventions.Standard, Type.EmptyTypes);
-            {
-                var il = cctor.GetILGenerator();
-
-                foreach (var kvp in converterFields)
-                {
-                    var typeCtor = kvp.Key.GetConstructor(Type.EmptyTypes);
-                    il.Emit(OpCodes.Newobj, typeCtor);
-                    il.Emit(OpCodes.Stsfld, kvp.Value);
-                }
-
-                il.Emit(OpCodes.Ret);
-            }
-            #endregion*/
-
-            //CreateAndInitializeConvertersFor(type, structure);
-
             #region Constructor
             var ctor = typeBuilder.DefineConstructor(MethodAttributes.Public, CallingConventions.Standard, new[] { typeof(IGeneratedStore) });
             {
@@ -145,10 +112,13 @@ namespace IPA.Config.Stores
             MethodBuilder notifyChanged = null;
             if (isINotifyPropertyChanged || hasNotifyAttribute)
             {
+                // we don't actually want to notify if the base class implements it
                 if (isINotifyPropertyChanged)
                 {
-                    var ExistingRaisePropertyChanged = type.GetMethod("RaisePropertyChanged", (BindingFlags)int.MaxValue, null, new Type[] { typeof(string) }, Array.Empty<ParameterModifier>());
-                    if (ExistingRaisePropertyChanged != null)
+                    var ExistingRaisePropertyChanged = type.GetMethod("RaisePropertyChanged", 
+                        BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.FlattenHierarchy,
+                        null, new Type[] { typeof(string) }, Array.Empty<ParameterModifier>());
+                    if (ExistingRaisePropertyChanged != null && !ExistingRaisePropertyChanged.IsPrivate)
                     {
                         notifyChanged = typeBuilder.DefineMethod("<>NotifyChanged",
                             MethodAttributes.Public | MethodAttributes.HideBySig | MethodAttributes.Final, null, new[] { typeof(string) });
@@ -164,7 +134,8 @@ namespace IPA.Config.Stores
                     }
                     else
                     {
-                        Logger.log.Critical($"Type '{type.FullName}' implements INotifyPropertyChanged but does not have a 'RaisePropertyChanged(string)' method, automatic raising of PropertyChanged event is disabled.");
+                        Logger.log.Critical($"Type '{type.FullName}' implements INotifyPropertyChanged but does not have an accessible " +
+                            "'RaisePropertyChanged(string)' method, automatic raising of PropertyChanged event is disabled.");
                     }
                 }
                 else