Merge pull request #180 from winnerspiros/fix/ci-code-quality-daily-challenge-17383618337664907011

Fix CI and Code Quality issues in Daily Challenge

Author: winnerspiros

debug_ids.py

@@ -0,0 +1,9 @@
+import os
+
+test_path = 'osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlaylist.cs'
+with open(test_path, 'r') as f:
+    lines = f.readlines()
+
+for i, line in enumerate(lines):
+    if 'assertItemInQueueListStep' in line or 'addItemStep' in line:
+        print(f"{i+1}: {line.strip()}")

final_cleanup.py

@@ -0,0 +1,28 @@
+import re
+
+def fix_file(path, pattern, replacement):
+    with open(path, 'r') as f:
+        content = f.read()
+    new_content = re.sub(pattern, replacement, content, flags=re.MULTILINE | re.DOTALL)
+    with open(path, 'w') as f:
+        f.write(new_content)
+
+# 1. Fix TestMultiplayerClient spacing and duplicates
+fix_file('osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs',
+         r'\s+private T clone<T>\(T incoming\).*?return result;\s+\}',
+         '\n\n        private T clone<T>(T incoming)\n        {\n            byte[] serialized = MessagePackSerializer.Serialize(typeof(T), incoming, SignalRUnionWorkaroundResolver.OPTIONS);\n            var result = MessagePackSerializer.Deserialize<T>(serialized, SignalRUnionWorkaroundResolver.OPTIONS);\n\n            if (incoming is MultiplayerRoomUser sourceUser && result is MultiplayerRoomUser targetUser)\n                targetUser.User = sourceUser.User;\n\n            if (incoming is MultiplayerRoom sourceRoom && result is MultiplayerRoom targetRoom)\n            {\n                foreach (var user in targetRoom.Users)\n                    user.User = sourceRoom.Users.FirstOrDefault(u => u.UserID == user.UserID)?.User;\n\n                if (targetRoom.Host != null)\n                    targetRoom.Host.User = sourceRoom.Host?.User;\n            }\n            else if (incoming is MultiplayerRoomUser sourceSingleUser && result is MultiplayerRoomUser targetSingleUser)\n            {\n                targetSingleUser.User = sourceSingleUser.User;\n            }\n\n            return result;\n        }')
+
+# 2. Fix updatePlaylistOrder indentation
+fix_file('osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs',
+         r'orderedActiveItems = itemsByPriority\s+\.OrderBy',
+         'orderedActiveItems = itemsByPriority\n                                         .OrderBy')
+
+# 3. Fix GameplayWarmupScreen unnecessary using
+fix_file('osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/GameplayWarmupScreen.cs',
+         r'using osu\.Framework\.Logging;\s+',
+         '')
+
+# 4. Fix PlayerPanelOverlay null check simplification
+fix_file('osu.Game/Screens/OnlinePlay/Matchmaking/Match/PlayerPanelOverlay.cs',
+         r'if \(panels\.FirstOrDefault\(p => p\.RoomUser\.Equals\(user\)\) is PlayerPanel panel\) panel\.HasQuit = true;',
+         'var panel = panels.FirstOrDefault(p => p.RoomUser.Equals(user));\n            if (panel != null) panel.HasQuit = true;')

final_cleanup_v2.py

@@ -0,0 +1,66 @@
+import re
+
+def fix_file(path, pattern, replacement):
+    with open(path, 'r') as f:
+        content = f.read()
+    new_content = re.sub(pattern, replacement, content, flags=re.MULTILINE | re.DOTALL)
+    if new_content == content:
+        print(f"Warning: No change to {path}")
+    with open(path, 'w') as f:
+        f.write(new_content)
+
+# DailyChallenge.cs cleanup
+# presentScore
+fix_file('osu.Game/Screens/OnlinePlay/DailyChallenge/DailyChallenge.cs',
+         r'private void presentScore\(long id\).*?\{.*?if \(this\.IsCurrentScreen\(\) && playlistItem != null\).*?this\.Push\(new PlaylistItemScoreResultsScreen\(id, room\.RoomID \?\? 0, playlistItem\)\);.*?\}',
+         '''        private void presentScore(long id)
+        {
+            if (this.IsCurrentScreen() && playlistItem != null)
+                this.Push(new PlaylistItemScoreResultsScreen(id, room.RoomID ?? 0, playlistItem));
+        }''')
+
+# updateMods
+fix_file('osu.Game/Screens/OnlinePlay/DailyChallenge/DailyChallenge.cs',
+         r'private void updateMods\(\).*?\{.*?if \(!this\.IsCurrentScreen\(\) \|\| playlistItem == null\).*?return;.*?Mods\.Value = userMods\.Value\.Concat\(playlistItem\.RequiredMods\.Select\(m => m\.ToMod\(Ruleset\.Value\.CreateInstance\(\)\)\)\)\.ToList\(\);.*?\}',
+         '''        private void updateMods()
+        {
+            if (!this.IsCurrentScreen() || playlistItem == null)
+                return;
+
+            Mods.Value = userMods.Value.Concat(playlistItem.RequiredMods.Select(m => m.ToMod(Ruleset.Value.CreateInstance()))).ToList();
+        }''')
+
+# startPlay
+fix_file('osu.Game/Screens/OnlinePlay/DailyChallenge/DailyChallenge.cs',
+         r'private void startPlay\(\).*?\{.*?sampleStart\?\.Play\(\);.*?var item = playlistItem;.*?if \(item == null\) return;.*?this\.Push\(new PlayerLoader\(\(\) => new DailyChallengePlayer\(room, item\).*?\{.*?Exited = \(\) => Scheduler\.AddOnce\(\(\) => leaderboard\.RefetchScores\(\)\).*?\}\)\);.*?\}',
+         '''        private void startPlay()
+        {
+            sampleStart?.Play();
+
+            if (playlistItem == null)
+                return;
+
+            this.Push(new PlayerLoader(() => new DailyChallengePlayer(room, playlistItem)
+            {
+                Exited = () => Scheduler.AddOnce(() => leaderboard.RefetchScores())
+            }));
+        }''')
+
+# PresentBeatmap
+fix_file('osu.Game/Screens/OnlinePlay/DailyChallenge/DailyChallenge.cs',
+         r'public void PresentBeatmap\(WorkingBeatmap beatmap, RulesetInfo ruleset\).*?\{.*?if \(!this\.IsCurrentScreen\(\)\).*?return;.*?var item = playlistItem;.*?if \(item == null\) return;.*?if \(item\.Beatmap\.BeatmapSet != null && beatmap\.BeatmapSetInfo\.OnlineID != item\.Beatmap\.BeatmapSet\.OnlineID\).*?\{.*?this\.Exit\(\);.*?game\?\.PresentBeatmap\(beatmap\.BeatmapSetInfo, b => b\.ID == beatmap\.BeatmapInfo\.ID\);.*?\}.*?\}',
+         '''        public void PresentBeatmap(WorkingBeatmap beatmap, RulesetInfo ruleset)
+        {
+            if (!this.IsCurrentScreen() || playlistItem == null)
+                return;
+
+            // We can only handle the current daily challenge beatmap.
+            // If the import was for a different beatmap, pass the duty off to global handling.
+            if (playlistItem.Beatmap.BeatmapSet != null && beatmap.BeatmapSetInfo.OnlineID != playlistItem.Beatmap.BeatmapSet.OnlineID)
+            {
+                this.Exit();
+                game?.PresentBeatmap(beatmap.BeatmapSetInfo, b => b.ID == beatmap.BeatmapInfo.ID);
+            }
+
+            // And if we're handling, we don't really have much to do here.
+        }''')

final_fix.py

@@ -0,0 +1,31 @@
+import os
+import re
+
+def patch_file(path, old, new):
+    if not os.path.exists(path):
+        return
+    with open(path, 'r') as f:
+        content = f.read()
+    if old in content:
+        with open(path, 'w') as f:
+            f.write(content.replace(old, new))
+    else:
+        # Try regex if literal fails
+        new_content = re.sub(re.escape(old).replace(r'\ ', r'\s+'), new, content, flags=re.MULTILINE | re.DOTALL)
+        if new_content != content:
+            with open(path, 'w') as f:
+                f.write(new_content)
+        else:
+            print(f"Warning: '{old}' not found in {path}")
+
+# 1. Fix GameplayWarmupScreen IDE0074 (compound assignment)
+gw_path = 'osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/GameplayWarmupScreen.cs'
+old_gw = """            if (card == null)
+            {
+                // Played card was not on the screen.
+
+                card = new RankedPlayCard(matchInfo.LastPlayedCard)"""
+new_gw = """            card ??= new RankedPlayCard(matchInfo.LastPlayedCard)
+            {
+                // Played card was not on the screen."""
+# Wait, the braces are different. Let's look at the original code.

fix_bot_feedback.py

@@ -0,0 +1,23 @@
+import sys
+
+# 1. Fix DailyChallenge.cs
+with open('osu.Game/Screens/OnlinePlay/DailyChallenge/DailyChallenge.cs', 'r') as f:
+    content = f.read()
+
+# Fix redundant conditional access
+content = content.replace('if (item?.AllowedMods.Any() == true)', 'if (item.AllowedMods.Any())')
+
+# 2. Fix GameplayWarmupScreen.cs line breaks
+with open('osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/GameplayWarmupScreen.cs', 'r') as f:
+    gw_content = f.read()
+
+old_ternary = 'Children = beatmap == null ? System.Array.Empty<Drawable>() : ['
+new_ternary = 'Children = beatmap == null\n                                        ? System.Array.Empty<Drawable>()\n                                        : ['
+
+gw_content = gw_content.replace(old_ternary, new_ternary)
+
+with open('osu.Game/Screens/OnlinePlay/DailyChallenge/DailyChallenge.cs', 'w') as f:
+    f.write(content)
+
+with open('osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/GameplayWarmupScreen.cs', 'w') as f:
+    f.write(gw_content)

fix_client_and_tests.py

@@ -0,0 +1,54 @@
+import re
+
+# 1. Update MultiplayerClient.cs to use UserID for LocalUser identification
+# Also improve null safety in room setup.
+with open('osu.Game/Online/Multiplayer/MultiplayerClient.cs', 'r') as f:
+    content = f.read()
+
+content = content.replace('public virtual MultiplayerRoomUser? LocalUser => Room?.Users.FirstOrDefault(u => u.UserID == API.LocalUser.Value.Id);',
+                          'public virtual MultiplayerRoomUser? LocalUser => Room?.Users.FirstOrDefault(u => u.UserID == API.LocalUser.Value.OnlineID);')
+
+with open('osu.Game/Online/Multiplayer/MultiplayerClient.cs', 'w') as f:
+    f.write(content)
+
+# 2. Update TestRoomRequestsHandler.cs to preserve RoomID, StartDate, and EndDate
+with open('osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs', 'r') as f:
+    handler_content = f.read()
+
+old_clone_room = """        private Room cloneRoom(Room source)
+        {
+            var result = new Room();
+            result.CopyFrom(source);
+            result.RoomID = source.RoomID;
+            result.StartDate = source.StartDate;
+            result.EndDate = source.EndDate;
+            result.Playlist = source.Playlist.Select(p => p.With()).ToList();
+            return result;
+        }"""
+
+new_clone_room = """        private Room cloneRoom(Room source)
+        {
+            var result = new Room();
+            result.CopyFrom(source);
+            result.RoomID = source.RoomID;
+            result.StartDate = source.StartDate;
+            result.EndDate = source.EndDate;
+            result.Host = source.Host;
+            result.Playlist = source.Playlist.Select(p => p.With()).ToList();
+            return result;
+        }"""
+
+handler_content = handler_content.replace(old_clone_room, new_clone_room)
+
+with open('osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs', 'w') as f:
+    f.write(handler_content)
+
+# 3. Update TestScenePlayerPanelOverlay.cs assertions
+with open('osu.Game.Tests/Visual/Matchmaking/TestScenePlayerPanelOverlay.cs', 'r') as f:
+    test_overlay_content = f.read()
+
+test_overlay_content = test_overlay_content.replace('AddAssert("no panels quit", () => this.ChildrenOfType<PlayerPanel>().Count(p => p.HasQuit), () => Is.EqualTo(0));',
+                                                   'AddAssert("no panels quit", () => list.Panels.Count(p => p.HasQuit), () => Is.EqualTo(0));')
+
+with open('osu.Game.Tests/Visual/Matchmaking/TestScenePlayerPanelOverlay.cs', 'w') as f:
+    f.write(test_overlay_content)

fix_daily_challenge.py

@@ -0,0 +1,120 @@
+import sys
+
+with open('osu.Game/Screens/OnlinePlay/DailyChallenge/DailyChallenge.cs', 'r') as f:
+    content = f.read()
+
+# Fix presentScore
+old_present_score = """        private void presentScore(long id)
+        {
+            if (!this.IsCurrentScreen())
+            var item = playlistItem;
+            if (item == null) return;
+
+            var item = playlistItem;
+            if (item != null)
+                this.Push(new PlaylistItemScoreResultsScreen(id, (room.RoomID ?? 0), item));
+        }"""
+
+new_present_score = """        private void presentScore(long id)
+        {
+            if (!this.IsCurrentScreen())
+                return;
+
+            var item = playlistItem;
+            if (item == null) return;
+
+            this.Push(new PlaylistItemScoreResultsScreen(id, (room.RoomID ?? 0), item));
+        }"""
+
+# Fix updateMods
+old_update_mods = """        private void updateMods()
+        {
+            var item = playlistItem;
+            if (item == null) return;
+                return;
+
+            var item = playlistItem;
+            if (item != null) Mods.Value = userMods.Value.Concat(item.RequiredMods.Select(m => m.ToMod(Ruleset.Value.CreateInstance()))).ToList();
+        }"""
+
+new_update_mods = """        private void updateMods()
+        {
+            if (!this.IsCurrentScreen())
+                return;
+
+            var item = playlistItem;
+            if (item == null) return;
+
+            Mods.Value = userMods.Value.Concat(item.RequiredMods.Select(m => m.ToMod(Ruleset.Value.CreateInstance()))).ToList();
+        }"""
+
+# Fix startPlay
+old_start_play = """        private void startPlay()
+        {
+            sampleStart?.Play();
+            var item = playlistItem; if (item != null) this.Push(new PlayerLoader(() => new DailyChallengePlayer(room, item)
+            {
+                Exited = () => Scheduler.AddOnce(() => leaderboard.RefetchScores())
+            }));
+        }"""
+
+new_start_play = """        private void startPlay()
+        {
+            sampleStart?.Play();
+
+            var item = playlistItem;
+            if (item == null) return;
+
+            this.Push(new PlayerLoader(() => new DailyChallengePlayer(room, item)
+            {
+                Exited = () => Scheduler.AddOnce(() => leaderboard.RefetchScores())
+            }));
+        }"""
+
+# Fix PresentBeatmap
+old_present_beatmap = """        public void PresentBeatmap(WorkingBeatmap beatmap, RulesetInfo ruleset)
+        {
+            var item = playlistItem;
+            if (item == null) return;
+            if (!this.IsCurrentScreen())
+                return;
+
+            var item = playlistItem;
+
+            // We can only handle the current daily challenge beatmap.
+            // If the import was for a different beatmap, pass the duty off to global handling.
+            if (item?.Beatmap.BeatmapSet != null && beatmap.BeatmapSetInfo.OnlineID != item.Beatmap.BeatmapSet.OnlineID)
+            {
+                this.Exit();
+                game?.PresentBeatmap(beatmap.BeatmapSetInfo, b => b.ID == beatmap.BeatmapInfo.ID);
+            }
+
+            // And if we're handling, we don't really have much to do here.
+        }"""
+
+new_present_beatmap = """        public void PresentBeatmap(WorkingBeatmap beatmap, RulesetInfo ruleset)
+        {
+            if (!this.IsCurrentScreen())
+                return;
+
+            var item = playlistItem;
+            if (item == null) return;
+
+            // We can only handle the current daily challenge beatmap.
+            // If the import was for a different beatmap, pass the duty off to global handling.
+            if (item.Beatmap.BeatmapSet != null && beatmap.BeatmapSetInfo.OnlineID == item.Beatmap.BeatmapSet.OnlineID)
+                return;
+
+            this.Exit();
+            game?.PresentBeatmap(beatmap.BeatmapSetInfo, b => b.ID == beatmap.BeatmapInfo.ID);
+
+            // And if we're handling, we don't really have much to do here.
+        }"""
+
+content = content.replace(old_present_score, new_present_score)
+content = content.replace(old_update_mods, new_update_mods)
+content = content.replace(old_start_play, new_start_play)
+content = content.replace(old_present_beatmap, new_present_beatmap)
+
+with open('osu.Game/Screens/OnlinePlay/DailyChallenge/DailyChallenge.cs', 'w') as f:
+    f.write(content)

fix_daily_challenge_final.py

@@ -0,0 +1,45 @@
+with open('osu.Game/Screens/OnlinePlay/DailyChallenge/DailyChallenge.cs', 'r') as f:
+    content = f.read()
+
+# Bot wants null propagation for:
+# var item = playlistItem;
+# if (item == null) return;
+# This pattern is used in: presentScore, updateMods, startPlay, PresentBeatmap.
+
+# Since playlistItem is a field, we can use null-propagation directly in most cases.
+# However, for startPlay and PresentBeatmap we need to perform actions.
+
+content = content.replace('''        private void presentScore(long id)
+        {
+            if (!this.IsCurrentScreen())
+                return;
+
+            var item = playlistItem;
+            if (item == null) return;
+
+            this.Push(new PlaylistItemScoreResultsScreen(id, (room.RoomID ?? 0), item));
+        }''', '''        private void presentScore(long id)
+        {
+            if (this.IsCurrentScreen() && playlistItem != null)
+                this.Push(new PlaylistItemScoreResultsScreen(id, room.RoomID ?? 0, playlistItem));
+        }''')
+
+content = content.replace('''        private void updateMods()
+        {
+            if (!this.IsCurrentScreen())
+                return;
+
+            var item = playlistItem;
+            if (item == null) return;
+
+            Mods.Value = userMods.Value.Concat(item.RequiredMods.Select(m => m.ToMod(Ruleset.Value.CreateInstance()))).ToList();
+        }''', '''        private void updateMods()
+        {
+            if (!this.IsCurrentScreen() || playlistItem == null)
+                return;
+
+            Mods.Value = userMods.Value.Concat(playlistItem.RequiredMods.Select(m => m.ToMod(Ruleset.Value.CreateInstance()))).ToList();
+        }''')
+
+with open('osu.Game/Screens/OnlinePlay/DailyChallenge/DailyChallenge.cs', 'w') as f:
+    f.write(content)