From ebe9ea47be2f7825097f81072503d7e59808d8b1 Mon Sep 17 00:00:00 2001 From: rootdarkarchon Date: Tue, 28 Feb 2023 22:45:00 +0100 Subject: [PATCH] potentially fixes an issue with cache creation, do not remove mediator on failure --- MareAPI | 2 +- .../Factories/CharacterDataFactory.cs | 23 +++------- .../Managers/CacheCreationService.cs | 44 +++++++++++-------- MareSynchronos/Managers/CachedPlayer.cs | 7 +-- .../Managers/OnlinePlayerManager.cs | 2 +- MareSynchronos/MareSynchronos.csproj | 2 +- MareSynchronos/Mediator/MareMediator.cs | 9 ++-- MareSynchronos/Mediator/Messages.cs | 2 +- MareSynchronos/Models/CharacterData.cs | 11 +---- MareSynchronos/UI/SettingsUi.cs | 2 +- 10 files changed, 45 insertions(+), 59 deletions(-) diff --git a/MareAPI b/MareAPI index 381f9a4..85bedb4 160000 --- a/MareAPI +++ b/MareAPI @@ -1 +1 @@ -Subproject commit 381f9a48085c7feb4acb7d00658d06d555c1dd96 +Subproject commit 85bedb49e3b5d9fbccbb1f81929e366d515ca1a6 diff --git a/MareSynchronos/Factories/CharacterDataFactory.cs b/MareSynchronos/Factories/CharacterDataFactory.cs index f40c394..2fb557f 100644 --- a/MareSynchronos/Factories/CharacterDataFactory.cs +++ b/MareSynchronos/Factories/CharacterDataFactory.cs @@ -25,7 +25,6 @@ public class CharacterDataFactory : MediatorSubscriberBase private readonly TransientResourceManager _transientResourceManager; private readonly FileCacheManager _fileCacheManager; private readonly PerformanceCollector _performanceCollector; - private readonly ConcurrentQueue> _processingQueue = new(); public CharacterDataFactory(ILogger logger, DalamudUtil dalamudUtil, IpcManager ipcManager, TransientResourceManager transientResourceManager, FileCacheManager fileReplacementFactory, MareMediator mediator, @@ -37,13 +36,6 @@ public class CharacterDataFactory : MediatorSubscriberBase _transientResourceManager = transientResourceManager; _fileCacheManager = fileReplacementFactory; _performanceCollector = performanceCollector; - Mediator.Subscribe(this, (_) => - { - while (_processingQueue.TryDequeue(out var result)) - { - result.RunSynchronously(); - } - }); } private unsafe bool CheckForNullDrawObject(IntPtr playerPointer) @@ -51,14 +43,14 @@ public class CharacterDataFactory : MediatorSubscriberBase return ((Character*)playerPointer)->GameObject.DrawObject == null; } - public async Task BuildCharacterData(CharacterData previousData, GameObjectHandler playerRelatedObject, CancellationToken token) + public async Task BuildCharacterData(CharacterData previousData, GameObjectHandler playerRelatedObject, CancellationToken token) { if (!_ipcManager.Initialized) { throw new InvalidOperationException("Penumbra is not connected"); } - if (playerRelatedObject == null) return previousData; + if (playerRelatedObject == null) return; bool pointerIsZero = true; try @@ -84,7 +76,7 @@ public class CharacterDataFactory : MediatorSubscriberBase _logger.LogTrace("Pointer was zero for {objectKind}", playerRelatedObject.ObjectKind); previousData.FileReplacements.Remove(playerRelatedObject.ObjectKind); previousData.GlamourerString.Remove(playerRelatedObject.ObjectKind); - return previousData; + return; } var previousFileReplacements = previousData.FileReplacements.ToDictionary(d => d.Key, d => d.Value); @@ -92,15 +84,14 @@ public class CharacterDataFactory : MediatorSubscriberBase try { - _processingQueue.Clear(); - return await _performanceCollector.LogPerformance(this, "CreateCharacterData>" + playerRelatedObject.ObjectKind, async () => + await _performanceCollector.LogPerformance(this, "CreateCharacterData>" + playerRelatedObject.ObjectKind, async () => { - return await CreateCharacterData(previousData, playerRelatedObject, token).ConfigureAwait(false); + await CreateCharacterData(previousData, playerRelatedObject, token).ConfigureAwait(false); }).ConfigureAwait(true); + return; } catch (OperationCanceledException) { - _processingQueue.Clear(); _logger.LogDebug("Cancelled creating Character data for {object}", playerRelatedObject); throw; } @@ -111,7 +102,7 @@ public class CharacterDataFactory : MediatorSubscriberBase previousData.FileReplacements = previousFileReplacements; previousData.GlamourerString = previousGlamourerData; - return previousData; + return; } private async Task CreateCharacterData(CharacterData previousData, GameObjectHandler playerRelatedObject, CancellationToken token) diff --git a/MareSynchronos/Managers/CacheCreationService.cs b/MareSynchronos/Managers/CacheCreationService.cs index fe23e6e..06572b2 100644 --- a/MareSynchronos/Managers/CacheCreationService.cs +++ b/MareSynchronos/Managers/CacheCreationService.cs @@ -1,11 +1,9 @@ -using FFXIVClientStructs.FFXIV.Client.Game.Character; -using MareSynchronos.API.Data.Enum; +using MareSynchronos.API.Data.Enum; using MareSynchronos.Factories; using MareSynchronos.Mediator; using MareSynchronos.Models; using MareSynchronos.Utils; using Microsoft.Extensions.Logging; -using System.Collections; namespace MareSynchronos.Managers; @@ -14,7 +12,7 @@ public class CacheCreationService : MediatorSubscriberBase, IDisposable private readonly CharacterDataFactory _characterDataFactory; private Task? _cacheCreationTask; private readonly Dictionary _cachesToCreate = new(); - private readonly CharacterData _lastCreatedData = new(); + private readonly CharacterData _playerData = new(); private readonly CancellationTokenSource _cts = new(); private readonly List _playerRelatedObjects = new(); private CancellationTokenSource _palettePlusCts = new(); @@ -43,9 +41,9 @@ public class CacheCreationService : MediatorSubscriberBase, IDisposable Task.Run(() => { var actualMsg = (ClearCacheForObjectMessage)msg; - _lastCreatedData.FileReplacements.Remove(actualMsg.ObjectToCreateFor.ObjectKind); - _lastCreatedData.GlamourerString.Remove(actualMsg.ObjectToCreateFor.ObjectKind); - Mediator.Publish(new CharacterDataCreatedMessage(_lastCreatedData)); + _playerData.FileReplacements.Remove(actualMsg.ObjectToCreateFor.ObjectKind); + _playerData.GlamourerString.Remove(actualMsg.ObjectToCreateFor.ObjectKind); + Mediator.Publish(new CharacterDataCreatedMessage(_playerData.ToAPI())); }); }); Mediator.Subscribe(this, (msg) => ProcessCacheCreation()); @@ -57,9 +55,9 @@ public class CacheCreationService : MediatorSubscriberBase, IDisposable private void PalettePlusChanged(PalettePlusMessage msg) { - if (!string.Equals(msg.Data, _lastCreatedData.PalettePlusPalette, StringComparison.Ordinal)) + if (!string.Equals(msg.Data, _playerData.PalettePlusPalette, StringComparison.Ordinal)) { - _lastCreatedData.PalettePlusPalette = msg.Data ?? string.Empty; + _playerData.PalettePlusPalette = msg.Data ?? string.Empty; _palettePlusCts?.Cancel(); _palettePlusCts?.Dispose(); @@ -69,26 +67,26 @@ public class CacheCreationService : MediatorSubscriberBase, IDisposable Task.Run(async () => { await Task.Delay(TimeSpan.FromSeconds(1), token).ConfigureAwait(false); - Mediator.Publish(new CharacterDataCreatedMessage(_lastCreatedData)); + Mediator.Publish(new CharacterDataCreatedMessage(_playerData.ToAPI())); }, token); } } private void HeelsOffsetChanged(HeelsOffsetMessage msg) { - if (msg.Offset != _lastCreatedData.HeelsOffset) + if (msg.Offset != _playerData.HeelsOffset) { - _lastCreatedData.HeelsOffset = msg.Offset; - Mediator.Publish(new CharacterDataCreatedMessage(_lastCreatedData)); + _playerData.HeelsOffset = msg.Offset; + Mediator.Publish(new CharacterDataCreatedMessage(_playerData.ToAPI())); } } private void CustomizePlusChanged(CustomizePlusMessage msg) { - if (!string.Equals(msg.Data, _lastCreatedData.CustomizePlusScale, StringComparison.Ordinal)) + if (!string.Equals(msg.Data, _playerData.CustomizePlusScale, StringComparison.Ordinal)) { - _lastCreatedData.CustomizePlusScale = msg.Data ?? string.Empty; - Mediator.Publish(new CharacterDataCreatedMessage(_lastCreatedData)); + _playerData.CustomizePlusScale = msg.Data ?? string.Empty; + Mediator.Publish(new CharacterDataCreatedMessage(_playerData.ToAPI())); } } @@ -104,9 +102,18 @@ public class CacheCreationService : MediatorSubscriberBase, IDisposable { foreach (var obj in toCreate) { - var data = await _characterDataFactory.BuildCharacterData(_lastCreatedData, obj.Value, _cts.Token).ConfigureAwait(false); + await _characterDataFactory.BuildCharacterData(_playerData, obj.Value, _cts.Token).ConfigureAwait(false); } - Mediator.Publish(new CharacterDataCreatedMessage(_lastCreatedData)); + + int maxWaitingTime = 10000; + while (!_playerData.IsReady && maxWaitingTime > 0) + { + await Task.Delay(100).ConfigureAwait(false); + maxWaitingTime -= 100; + _logger.LogTrace("Waiting for Cache to be ready"); + } + + Mediator.Publish(new CharacterDataCreatedMessage(_playerData.ToAPI())); } catch (Exception ex) { @@ -115,7 +122,6 @@ public class CacheCreationService : MediatorSubscriberBase, IDisposable finally { _logger.LogDebug("Cache Creation complete"); - } }, _cts.Token); } diff --git a/MareSynchronos/Managers/CachedPlayer.cs b/MareSynchronos/Managers/CachedPlayer.cs index 932d6bb..d20a020 100644 --- a/MareSynchronos/Managers/CachedPlayer.cs +++ b/MareSynchronos/Managers/CachedPlayer.cs @@ -378,6 +378,7 @@ public class CachedPlayer : MediatorSubscriberBase, IDisposable if (downloadToken.IsCancellationRequested) { _logger.LogTrace("Detected cancellation"); + _apiController.CancelDownload(downloadId); return; } @@ -415,12 +416,6 @@ public class CachedPlayer : MediatorSubscriberBase, IDisposable _logger.LogDebug("[{applicationId}] Application finished", _applicationId); }); - - _downloadCancellationTokenSource = null; - - _logger.LogDebug("Download was cancelled"); - _apiController.CancelDownload(downloadId); - }, downloadToken); } diff --git a/MareSynchronos/Managers/OnlinePlayerManager.cs b/MareSynchronos/Managers/OnlinePlayerManager.cs index 1fc425c..fefee9e 100644 --- a/MareSynchronos/Managers/OnlinePlayerManager.cs +++ b/MareSynchronos/Managers/OnlinePlayerManager.cs @@ -28,7 +28,7 @@ public class OnlinePlayerManager : MediatorSubscriberBase, IDisposable Mediator.Subscribe(this, (_) => FrameworkOnUpdate()); Mediator.Subscribe(this, (msg) => { - var newData = ((CharacterDataCreatedMessage)msg).CharacterData.ToAPI(); + var newData = ((CharacterDataCreatedMessage)msg).CharacterData; if (_lastSentData == null || _lastSentData != null && !string.Equals(newData.DataHash.Value, _lastSentData.DataHash.Value, StringComparison.Ordinal)) { _logger.LogDebug("Pushing data for visible players"); diff --git a/MareSynchronos/MareSynchronos.csproj b/MareSynchronos/MareSynchronos.csproj index b82c3c5..fffa024 100644 --- a/MareSynchronos/MareSynchronos.csproj +++ b/MareSynchronos/MareSynchronos.csproj @@ -3,7 +3,7 @@ - 0.7.39 + 0.7.40 https://github.com/Penumbra-Sync/client diff --git a/MareSynchronos/Mediator/MareMediator.cs b/MareSynchronos/Mediator/MareMediator.cs index 41c54fb..81c9af4 100644 --- a/MareSynchronos/Mediator/MareMediator.cs +++ b/MareSynchronos/Mediator/MareMediator.cs @@ -22,6 +22,7 @@ public class MareMediator : IDisposable private readonly ILogger _logger; private readonly PerformanceCollector _performanceCollector; private readonly object _addRemoveLock = new(); + private readonly Dictionary _lastErrorTime = new(); public MareMediator(ILogger logger, PerformanceCollector performanceCollector) { @@ -69,11 +70,13 @@ public class MareMediator : IDisposable } catch (Exception ex) { - lock (_addRemoveLock) + if (_lastErrorTime.TryGetValue(subscriber, out var lastErrorTime)) { - var removed = _subscriberDict[message.GetType()].RemoveWhere(s => s == subscriber); - _logger.LogCritical(ex, "Error executing {type} for subscriber {subscriber}, removed from Mediator: {removeCount}", message.GetType().Name, subscriber.Subscriber.GetType().Name, removed); + if (lastErrorTime.Add(TimeSpan.FromSeconds(10)) > DateTime.UtcNow) continue; } + + _logger.LogCritical(ex, "Error executing {type} for subscriber {subscriber}", message.GetType().Name, subscriber.Subscriber.GetType().Name); + _lastErrorTime[subscriber] = DateTime.UtcNow; } } }); diff --git a/MareSynchronos/Mediator/Messages.cs b/MareSynchronos/Mediator/Messages.cs index 1a50369..fc2234e 100644 --- a/MareSynchronos/Mediator/Messages.cs +++ b/MareSynchronos/Mediator/Messages.cs @@ -40,7 +40,7 @@ public record NotificationMessage (string Title, string Message, NotificationType Type, uint TimeShownOnScreen = 3000) : IMessage; public record CreateCacheForObjectMessage(GameObjectHandler ObjectToCreateFor) : IMessage; public record ClearCacheForObjectMessage(GameObjectHandler ObjectToCreateFor) : IMessage; -public record CharacterDataCreatedMessage(CharacterData CharacterData) : IMessage; +public record CharacterDataCreatedMessage(API.Data.CharacterData CharacterData) : IMessage; public record PenumbraStartRedrawMessage(IntPtr Address) : IMessage; public record PenumbraEndRedrawMessage(IntPtr Address) : IMessage; public record HubReconnectingMessage(Exception? Exception) : IMessage; diff --git a/MareSynchronos/Models/CharacterData.cs b/MareSynchronos/Models/CharacterData.cs index 32c482e..5eb43df 100644 --- a/MareSynchronos/Models/CharacterData.cs +++ b/MareSynchronos/Models/CharacterData.cs @@ -1,31 +1,22 @@ -using Newtonsoft.Json; -using System.Text; +using System.Text; using MareSynchronos.API.Data.Enum; using MareSynchronos.API.Data; namespace MareSynchronos.Models; -[JsonObject(MemberSerialization.OptIn)] public class CharacterData { - [JsonProperty] public Dictionary> FileReplacements { get; set; } = new(); - [JsonProperty] public Dictionary GlamourerString { get; set; } = new(); public bool IsReady => FileReplacements.SelectMany(k => k.Value).All(f => f.Computed); - - [JsonProperty] public string ManipulationString { get; set; } = string.Empty; - [JsonProperty] public float HeelsOffset { get; set; } = 0f; - [JsonProperty] public string CustomizePlusScale { get; set; } = string.Empty; - [JsonProperty] public string PalettePlusPalette { get; set; } = string.Empty; public API.Data.CharacterData ToAPI() diff --git a/MareSynchronos/UI/SettingsUi.cs b/MareSynchronos/UI/SettingsUi.cs index 8a661d5..32fc7f9 100644 --- a/MareSynchronos/UI/SettingsUi.cs +++ b/MareSynchronos/UI/SettingsUi.cs @@ -62,7 +62,7 @@ public class SettingsUi : WindowMediatorSubscriberBase, IDisposable Mediator.Subscribe(this, (_) => IsOpen = false); Mediator.Subscribe(this, (_) => UiShared_GposeStart()); Mediator.Subscribe(this, (_) => UiShared_GposeEnd()); - Mediator.Subscribe(this, (msg) => LastCreatedCharacterData = ((CharacterDataCreatedMessage)msg).CharacterData.ToAPI()); + Mediator.Subscribe(this, (msg) => LastCreatedCharacterData = ((CharacterDataCreatedMessage)msg).CharacterData); windowSystem.AddWindow(this); }