Merge pull request #145 from poveden/poveden/fix-effect-cleanup

Poveden/fix effect cleanup
poveden · May 22, 2022 · 6578f1e · 6578f1e
2 parents 6dbdf57 + d2dc2f8
commit 6578f1e
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
+## [1.16.3](https://github.com/poveden/EliteChroma/compare/v1.16.2...v1.16.3) — 2022-05-22
+
+### Fixed
+
+- Fix crash caused by cleaning up stale effects on new game runs
+
 ## [1.16.2](https://github.com/poveden/EliteChroma/compare/v1.16.1...v1.16.2) — 2022-05-13
 
 ### Fixed

diff --git a/src/EliteChroma.Core/Chroma/ChromaEffect.cs b/src/EliteChroma.Core/Chroma/ChromaEffect.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Runtime.CompilerServices;
 using ChromaWrapper.Sdk;
 
 namespace EliteChroma.Chroma
@@ -11,7 +12,7 @@ public class ChromaEffect<TState>
         private readonly List<ChromaEffectLayer<TState>> _layers = new List<ChromaEffectLayer<TState>>();
         private readonly ChromaCanvas _canvas = new ChromaCanvas();
 
-        private IReadOnlyCollection<Guid> _activeEffectIds = Array.Empty<Guid>();
+        private readonly ConditionalWeakTable<IChromaSdk, IReadOnlyCollection<Guid>> _activeEffectIds = new ConditionalWeakTable<IChromaSdk, IReadOnlyCollection<Guid>>();
 
         public IReadOnlyList<ChromaEffectLayer<TState>> Layers => _layers;
 
@@ -47,12 +48,15 @@ public void Render(IChromaSdk chroma, TState state)
                 _layers[i].Render(_canvas, state);
             }
 
-            IReadOnlyCollection<Guid> oldEffectIds = _activeEffectIds;
-            _activeEffectIds = _canvas.SetEffect(chroma);
+            _ = _activeEffectIds.TryGetValue(chroma, out IReadOnlyCollection<Guid>? oldEffectIds);
+            _activeEffectIds.AddOrUpdate(chroma, _canvas.SetEffect(chroma));
 
-            foreach (Guid effectId in oldEffectIds)
+            if (oldEffectIds != null)
             {
-                chroma.DeleteEffect(effectId);
+                foreach (Guid effectId in oldEffectIds)
+                {
+                    chroma.DeleteEffect(effectId);
+                }
             }
         }
 

diff --git a/src/EliteChroma/EliteChroma.csproj b/src/EliteChroma/EliteChroma.csproj
@@ -10,7 +10,7 @@
     <PackageProjectUrl>https://github.com/poveden/EliteChroma</PackageProjectUrl>
     <RepositoryUrl>https://github.com/poveden/EliteChroma</RepositoryUrl>
     <Description>A tool to make Razer Chroma devices react to Elite:Dangerous in-game events.</Description>
-    <Version>1.16.2</Version>
+    <Version>1.16.3</Version>
     <RepositoryType></RepositoryType>
     <Authors>Jorge Poveda Coma</Authors>
     <PackageTags>Elite, Dangerous, Razer, Chroma</PackageTags>

diff --git a/test/EliteChroma.Core.Tests/ChromaEffectLayerTests.cs b/test/EliteChroma.Core.Tests/ChromaEffectLayerTests.cs
@@ -4,6 +4,7 @@
 using System.Linq;
 using ChromaWrapper;
 using ChromaWrapper.Keyboard;
+using ChromaWrapper.Sdk;
 using EliteChroma.Chroma;
 using EliteChroma.Core.Internal;
 using EliteChroma.Core.Layers;
@@ -16,6 +17,7 @@
 using EliteFiles.Journal;
 using EliteFiles.Journal.Events;
 using Moq;
+using Moq.Protected;
 using TestUtils;
 using Xunit;
 
@@ -88,6 +90,73 @@ public void LayerBaseThrowsOnInvalidGameState()
             Assert.Throws<ArgumentNullException>(() => le.Render(chroma.Object, null!));
         }
 
+        [Fact]
+        public void ChromaEffectTracksActiveEffectsPerChromaSdkInstance()
+        {
+            var ce = new ChromaEffect<object>();
+
+            var layerMock = new Mock<ChromaEffectLayer<object>>();
+            layerMock.Protected()
+                .Setup("OnRender", ItExpr.IsAny<ChromaCanvas>(), ItExpr.IsAny<object>())
+                .Callback((ChromaCanvas cc, object o) =>
+                {
+                    cc.Keyboard.Key[0] = ChromaColor.Blue;
+                });
+
+            ce.Add(layerMock.Object);
+
+            Mock<IChromaSdk> CreateMockSdk(List<Guid> created, Action<Guid> setDeleted)
+            {
+                var chroma = new Mock<IChromaSdk>();
+                chroma
+                    .Setup(x => x.CreateEffect(It.IsAny<IKeyboardEffect>()))
+                    .Returns(() =>
+                    {
+                        var id = Guid.NewGuid();
+                        created.Add(id);
+                        return id;
+                    });
+                chroma
+                    .Setup(x => x.DeleteEffect(It.IsAny<Guid>()))
+                    .Callback((Guid id) => setDeleted(id));
+
+                return chroma;
+            }
+
+            var idsCreated1 = new List<Guid>();
+            var idsCreated2 = new List<Guid>();
+            var idDeleted1 = Guid.Empty;
+            var idDeleted2 = Guid.Empty;
+
+            var chroma1 = CreateMockSdk(idsCreated1, x => idDeleted1 = x);
+            var chroma2 = CreateMockSdk(idsCreated2, x => idDeleted2 = x);
+
+            ce.Render(chroma1.Object, new object());
+            Assert.Single(idsCreated1);
+            Assert.Empty(idsCreated2);
+            Assert.Equal(Guid.Empty, idDeleted1);
+            Assert.Equal(Guid.Empty, idDeleted2);
+
+            ce.Render(chroma2.Object, new object());
+            Assert.Single(idsCreated1);
+            Assert.Single(idsCreated2);
+            Assert.NotEqual(idsCreated1[0], idsCreated2[0]);
+            Assert.Equal(Guid.Empty, idDeleted1);
+            Assert.Equal(Guid.Empty, idDeleted2);
+
+            ce.Render(chroma1.Object, new object());
+            Assert.Equal(2, idsCreated1.Count);
+            Assert.Single(idsCreated2);
+            Assert.Equal(idsCreated1[0], idDeleted1);
+            Assert.Equal(Guid.Empty, idDeleted2);
+
+            ce.Render(chroma2.Object, new object());
+            Assert.Equal(2, idsCreated1.Count);
+            Assert.Equal(2, idsCreated2.Count);
+            Assert.Equal(idsCreated1[0], idDeleted1);
+            Assert.Equal(idsCreated2[0], idDeleted2);
+        }
+
         [Theory]
         [InlineData(StarClass.O, 0.25, new[] { 0x000000, 0x007F00, 0x00FF00, 0x007F00, 0x000000 })]
         [InlineData(StarClass.HerbigAeBe, 0.25, new[] { 0xFFFF00, 0xFFFF00, 0x000000, 0x000000 })]