Merge pull request #192 from BeatLeader/br-semaphores-fix

Fixed semaphores not being released because of incorrect handling.

Author: NSGolova

Source/2_Core/API/Methods/GetAvatarRequest.cs

@@ -1,12 +1,13 @@
 using System.Net.Http;
+using System.Threading;
 using BeatLeader.Models;
 using BeatLeader.Utils;
 using BeatLeader.WebRequests;
 
 namespace BeatLeader.API {
     internal class GetAvatarRequest : PersistentWebRequestBase<AvatarSettings, JsonResponseParser<AvatarSettings>> {
-        public static IWebRequest<AvatarSettings> Send(string playerId) {
-            return SendRet($"{BLConstants.BEATLEADER_API_URL}/player/{playerId}/ingameavatar", HttpMethod.Get);
+        public static IWebRequest<AvatarSettings> Send(string playerId, CancellationToken token = default) {
+            return SendRet($"{BLConstants.BEATLEADER_API_URL}/player/{playerId}/ingameavatar", HttpMethod.Get, token: token);
         }
     }
 }

Source/2_Core/Models/API/BeatLeader/Player.cs

@@ -6,7 +6,9 @@
 using Newtonsoft.Json;
 using BeatLeader.Themes;
 using BeatLeader.Utils;
+using BeatLeader.WebRequests;
 using BeatSaber.BeatAvatarSDK;
+using UnityEngine;
 
 namespace BeatLeader.Models {
     public class PlayerContextExtension {
@@ -32,20 +34,30 @@ public class Player : IPlayer {
         float IPlayer.PerformancePoints => pp;
         IPlayerProfileSettings? IPlayer.ProfileSettings => profileSettings;
 
-        private static readonly Dictionary<string, AvatarSettings?> avatarSettingsCache = new();
+        private static readonly Dictionary<string, AvatarSettings> avatarSettingsCache = new();
         private static readonly Dictionary<string, SemaphoreSlim?> semaphores = new();
 
         public async Task<AvatarData> GetBeatAvatarAsync(bool bypassCache, CancellationToken token) {
-            var semaphore = semaphores.GetOrAdd(id, new SemaphoreSlim(1, 1))!;
-            await semaphore.WaitAsync(token);
-
-            if (!avatarSettingsCache.TryGetValue(id, out var avatarSettings) || bypassCache) {
-                var request = await GetAvatarRequest.Send(id).Join();
-                avatarSettings = request.Result;
-                avatarSettingsCache[id] = avatarSettings;
+            if (!semaphores.TryGetValue(id, out var semaphore)) {
+                semaphore = new(1, 1);
+                semaphores[id] = semaphore;
             }
 
-            semaphore.Release();
+            await semaphore!.WaitAsync(token);
+
+            AvatarSettings avatarSettings;
+            try {
+                if (!avatarSettingsCache.TryGetValue(id, out avatarSettings) || bypassCache) {
+                    var request = await GetAvatarRequest.Send(id, token).Join();
+
+                    if (request.Result != null) {
+                        avatarSettings = request.Result!;
+                        avatarSettingsCache[id] = avatarSettings;
+                    }
+                }
+            } finally {
+                semaphore.Release();
+            }
 
             return avatarSettings?.ToAvatarData() ?? AvatarUtils.DefaultAvatarData;
         }
@@ -138,11 +150,9 @@ private float? Saturation {
             set => saturation = value ?? 0;
         }
 
-        [JsonIgnore]
-        public int hue;
+        [JsonIgnore] public int hue;
 
-        [JsonIgnore]
-        public float saturation;
+        [JsonIgnore] public float saturation;
 
         public string message;
 

Source/8_UI/FlowCoordinator/Flows/BattleRoyaleFlow/Views/OpponentsView/BattleRoyaleOpponentsList.cs

@@ -137,7 +137,7 @@ protected override void OnUpdate() {
                         _valueAnimator.SetTarget(1f);
                     }
                 }
-                //
+
                 var targetColor = _colorShouldBeSet ? _targetColor : Color.white.ColorWithAlpha(0.3f);
                 var color = Color.Lerp(
                     Color.white.ColorWithAlpha(0.1f),
@@ -178,16 +178,18 @@ protected override void OnInit(BattleRoyaleReplay item) {
                     return;
                 }
 
-                if (_refreshColorTask?.Status is TaskStatus.Running || _refreshPlayerTask?.Status is TaskStatus.Running) {
+                if (_refreshColorTask != null || _refreshPlayerTask != null) {
                     _tokenSource.Cancel();
                     _tokenSource = new();
                 }
 
                 ResetColor();
                 RefreshOtherText();
 
-                _refreshPlayerTask = RefreshPlayer(_tokenSource.Token).RunCatching();
-                _refreshColorTask = RefreshAccentColor(_tokenSource.Token).RunCatching();
+                var token = _tokenSource.Token;
+
+                _refreshPlayerTask = RefreshPlayer(item, token).RunCatching();
+                _refreshColorTask = RefreshAccentColor(item, token).RunCatching();
 
                 _prevReplay = item;
                 _prevRank = item.ReplayRank;
@@ -199,27 +201,28 @@ public void Init(IBattleRoyaleHost battleRoyaleHost) {
                 _navigateButton.Enabled = battleRoyaleHost.CanMutateLobby;
             }
 
-            private async Task RefreshAccentColor(CancellationToken token) {
-                var data = await Item.GetBattleRoyaleDataAsync(false, token);
+            private async Task RefreshAccentColor(BattleRoyaleReplay replay, CancellationToken token) {
+                var data = await replay.GetBattleRoyaleDataAsync(false, token);
 
                 if (token.IsCancellationRequested) {
                     return;
                 }
 
                 var color = data.AccentColor ?? Color.white;
                 SetColor(color);
+                _refreshColorTask = null;
             }
 
-            private async Task RefreshPlayer(CancellationToken token) {
-                var header = Item.ReplayHeader;
-                var player = await header.LoadPlayerAsync(false, token) as IPlayer;
+            private async Task RefreshPlayer(BattleRoyaleReplay replay, CancellationToken token) {
+                var player = await replay.ReplayHeader.LoadPlayerAsync(false, token) as IPlayer;
 
                 if (token.IsCancellationRequested) {
                     return;
                 }
 
                 _playerAvatar.SetAvatar(player);
                 _playerNameText.Text = player.Name;
+                _refreshPlayerTask = null;
             }
 
             private void RefreshOtherText() {
@@ -256,15 +259,15 @@ public void Setup(IBattleRoyaleHost? battleRoyaleHost) {
                 _battleRoyaleHost.ReplayRefreshRequestedEvent -= HandleRefreshRequested;
                 _battleRoyaleHost.CanMutateLobbyStateChangedEvent -= HandleCanMutateLobbyChangedEvent;
             }
-            
+
             _battleRoyaleHost = battleRoyaleHost;
-            
+
             if (_battleRoyaleHost != null) {
                 _battleRoyaleHost.ReplayAddedEvent += HandleReplayAdded;
                 _battleRoyaleHost.ReplayRemovedEvent += HandleReplayRemoved;
                 _battleRoyaleHost.ReplayRefreshRequestedEvent += HandleRefreshRequested;
                 _battleRoyaleHost.CanMutateLobbyStateChangedEvent += HandleCanMutateLobbyChangedEvent;
-                
+
                 HandleCanMutateLobbyChangedEvent(_battleRoyaleHost.CanMutateLobby);
             }
         }

Source/8_UI/Models/FlowCoordinator/BattleRoyale/BattleRoyaleReplay.cs

@@ -21,16 +21,18 @@ public BattleRoyaleReplay(IReplayHeader header) {
         public async Task<BattleRoyaleReplayData> GetBattleRoyaleDataAsync(bool bypassCache, CancellationToken token) {
             if (_replayData == null || bypassCache) {
                 await _semaphoreSlim.WaitAsync(token);
-
-                var player = await ReplayHeader.LoadPlayerAsync(false, token);
-                var avatarData = await player.GetBeatAvatarAsync(false, token);
 
-                var info = ReplayHeader.ReplayInfo;
-                var accentColor = GetReplayColor(info);
+                try {
+                    var player = await ReplayHeader.LoadPlayerAsync(false, token);
+                    var avatarData = await player.GetBeatAvatarAsync(false, token);
 
-                _replayData = new BattleRoyaleReplayData(avatarData, accentColor);
+                    var info = ReplayHeader.ReplayInfo;
+                    var accentColor = GetReplayColor(info);
 
-                _semaphoreSlim.Release();
+                    _replayData = new BattleRoyaleReplayData(avatarData, accentColor);
+                } finally {
+                    _semaphoreSlim.Release();
+                }
             }
 
             return _replayData.Value;