Code review

Setlist/BeatLeaderPlaylistOwnership.cs

@@ -5,38 +5,26 @@ using System.Linq;
 using System.Reflection;
 using System.Threading;
 using System.Threading.Tasks;
+using UnityEngine;
+#if false
 using BeatLeader.Utils;
 using Newtonsoft.Json;
-using UnityEngine;
 using UnityEngine.Networking;
+#endif
 using IPALogger = IPA.Logging.Logger;
 
 namespace Setlist
 {
     /// <summary>
-    /// Parses BeatLeader sync URLs and checks ownership via GET /user/playlists.
+    /// Parses BeatLeader-style sync URLs for logging. Ownership is determined in
-    /// Reuses BeatLeader's sign-in by piggy-backing on Unity's process-wide
+    /// <see cref="Plugin.FormatPlaylistLogLine"/> from playlist JSON only (no network).
-    /// <see cref="UnityWebRequest"/> cookie cache (the shipped 0.9.x BeatLeader
-    /// signs in with <see cref="UnityWebRequest"/>; cookies are global per host).
-    /// We block on BeatLeader's <c>Authentication._signedIn</c> via reflection so
-    /// we don't fire the request before the cookie is in the cache.
     /// </summary>
     internal static class BeatLeaderPlaylistOwnership
     {
-        private const string AuthenticationTypeName = "BeatLeader.API.Authentication";
-        private const string SignedInFieldName = "_signedIn";
-        private const float LoginWaitTimeoutSeconds = 90f;
         private const float PlatformUserPollStepSeconds = 0.5f;
         /// <summary>How long to wait for <see cref="PlatformLeaderboardsModel"/> to appear and populate <c>playerId</c> (plugin runs before menu init).</summary>
         private const float PlatformUserWaitTimeoutSeconds = 30f;
         private const float PlatformUserGetUserInfoRetrySeconds = 3f;
-        private const int RequestTimeoutSeconds = 30;
-
-        private sealed class UserPlaylistSummary
-        {
-            [JsonProperty("guid")]
-            public string Guid { get; set; }
-        }
 
         internal static bool TryExtractBeatLeaderPlaylistGuid(string syncUrl, out string guid)
         {
@@ -79,9 +67,8 @@ namespace Setlist
         }
 
         /// <summary>
-        /// Spawns a hidden coroutine runner that waits for BeatLeader sign-in,
+        /// Spawns a hidden coroutine runner that resolves the platform user id, then logs per-playlist
-        /// fetches <c>/user/playlists</c>, then logs the per-playlist ownership.
+        /// ownership from playlist JSON. Must be called from the Unity main thread (BSIPA <c>OnApplicationStart</c> is fine).
-        /// Must be called from the Unity main thread (BSIPA's <c>OnApplicationStart</c> is fine).
         /// </summary>
         internal static void ScheduleVerifyAndLog(
             List<(string Title, bool HasSyncUrl, string BeatLeaderGuid, string OwnerId)> entries,
@@ -95,7 +82,7 @@ namespace Setlist
         }
 
         /// <summary>
-        /// Component that drives the verification coroutine; self-destroys when finished.
+        /// Component that drives the logging coroutine; self-destroys when finished.
         /// </summary>
         private sealed class OwnershipRunner : MonoBehaviour
         {
@@ -119,54 +106,6 @@ namespace Setlist
                     ? "platformUserId=(unknown)"
                     : $"platformUserId={platformUserId}");
 
-                HashSet<string> owned = null;
-                string failure = null;
-
-                if (_entries.Any(e => e.BeatLeaderGuid != null))
-                {
-                    FieldInfo signedInField;
-                    try
-                    {
-                        signedInField = ResolveSignedInField(_log);
-                    }
-                    catch (Exception ex)
-                    {
-                        signedInField = null;
-                        failure = "reflecting BeatLeader Authentication failed: " + ex.Message;
-                    }
-
-                    if (signedInField != null)
-                    {
-                        var waitedSeconds = 0f;
-                        while (!IsSignedIn(signedInField))
-                        {
-                            if (waitedSeconds >= LoginWaitTimeoutSeconds)
-                            {
-                                failure = $"BeatLeader login did not complete within {LoginWaitTimeoutSeconds:F0}s; "
-                                          + "is the BeatLeader mod actually signing in (check BeatLeader log lines)?";
-                                break;
-                            }
-                            yield return new WaitForSeconds(1f);
-                            waitedSeconds += 1f;
-                        }
-
-                        if (failure == null)
-                        {
-                            var fetchEnumerator = FetchOwnedGuids(result =>
-                            {
-                                owned = result.OwnedGuids;
-                                failure = result.Failure;
-                            });
-                            yield return StartCoroutine(fetchEnumerator);
-                        }
-                    }
-                }
-
-                if (owned == null && failure != null)
-                {
-                    _log.Info("BeatLeader /user/playlists: " + failure);
-                }
-
                 foreach (var e in _entries)
                 {
                     _log.Info(Plugin.FormatPlaylistLogLine(
@@ -174,8 +113,7 @@ namespace Setlist
                         e.HasSyncUrl,
                         e.BeatLeaderGuid,
                         e.OwnerId,
-                        platformUserId,
+                        platformUserId));
-                        owned));
                 }
 
                 Destroy(gameObject);
@@ -287,6 +225,21 @@ namespace Setlist
                 return last;
             }
 
+            // SETLIST: Remove the entire #if false region below once we no longer need the old
+            // BeatLeader GET /user/playlists + login-wait verification path for reference.
+
+#if false
+            private const string AuthenticationTypeName = "BeatLeader.API.Authentication";
+            private const string SignedInFieldName = "_signedIn";
+            private const float LoginWaitTimeoutSeconds = 90f;
+            private const int RequestTimeoutSeconds = 30;
+
+            private sealed class UserPlaylistSummary
+            {
+                [JsonProperty("guid")]
+                public string Guid { get; set; }
+            }
+
             private struct FetchResult
             {
                 public HashSet<string> OwnedGuids;
@@ -384,6 +337,7 @@ namespace Setlist
                 var asm = typeof(BeatLeaderPlaylistOwnership).Assembly;
                 return asm.GetName().Version?.ToString() ?? "0.0.0";
             }
+#endif
         }
     }
 }

Setlist/Plugin.cs

@@ -1,6 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using IPA;
 using IPALogger = IPA.Logging.Logger;
 
@@ -86,45 +85,50 @@ namespace Setlist
         {
         }
 
+        /// <summary>
+        /// One-line log for a playlist. Ownership is inferred only from playlist JSON customData
+        /// (<c>owner</c> vs the game's platform user id). No network verification.
+        /// </summary>
         internal static string FormatPlaylistLogLine(
             string title,
             bool hasSyncUrl,
-            string beatLeaderGuid,
+            string beatLeaderPlaylistGuid,
             string ownerId,
-            string platformUserId,
+            string platformUserId)
-            HashSet<string> ownedGuids)
         {
             var ownerToken = string.IsNullOrEmpty(ownerId) ? "owner=n/a" : $"owner={ownerId}";
 
-            string confirmPart;
+            string ownerMatchesPlatformPart;
             if (!hasSyncUrl)
             {
-                confirmPart = "beatLeaderOwnerConfirmed=n/a (no sync URL)";
+                ownerMatchesPlatformPart = "ownerMatchesPlatform=n/a (no sync URL)";
             }
-            else if (string.IsNullOrEmpty(beatLeaderGuid))
+            else if (string.IsNullOrEmpty(beatLeaderPlaylistGuid))
             {
-                confirmPart = "beatLeaderOwnerConfirmed=n/a (sync URL is not a BeatLeader playlist)";
+                ownerMatchesPlatformPart =
+                    "ownerMatchesPlatform=n/a (sync URL is not an api.beatleader.com /playlist/guid/… URL)";
             }
-            else if (!string.IsNullOrEmpty(ownerId)
+            else if (string.IsNullOrEmpty(platformUserId))
-                     && !string.IsNullOrEmpty(platformUserId)
-                     && string.Equals(ownerId, platformUserId, StringComparison.Ordinal))
             {
-                confirmPart = "beatLeaderOwnerConfirmed=true (owner matches platform user id)";
+                ownerMatchesPlatformPart =
+                    "ownerMatchesPlatform=unknown (platform user id not available yet; cannot compare to owner)";
             }
-            else if (ownedGuids == null)
+            else if (string.IsNullOrEmpty(ownerId))
             {
-                confirmPart = "beatLeaderOwnerConfirmed=unknown (BeatLeader /user/playlists did not succeed; see prior log line)";
+                ownerMatchesPlatformPart =
+                    "ownerMatchesPlatform=false (BeatLeader playlist URL but no owner field in playlist JSON)";
             }
-            else if (ownedGuids.Contains(beatLeaderGuid))
+            else if (string.Equals(ownerId, platformUserId, StringComparison.Ordinal))
             {
-                confirmPart = "beatLeaderOwnerConfirmed=true";
+                ownerMatchesPlatformPart = "ownerMatchesPlatform=true (playlist owner field equals platform user id)";
             }
             else
             {
-                confirmPart = "beatLeaderOwnerConfirmed=false";
+                ownerMatchesPlatformPart =
+                    "ownerMatchesPlatform=false (playlist owner field differs from platform user id)";
             }
 
-            return $"Playlist \"{title}\": hasSyncUrl={hasSyncUrl}, {ownerToken}, {confirmPart}";
+            return $"Playlist \"{title}\": hasSyncUrl={hasSyncUrl}, {ownerToken}, {ownerMatchesPlatformPart}";
         }
     }
 }

Setlist/Properties/AssemblyInfo.cs

@@ -11,5 +11,5 @@ using System.Runtime.InteropServices;
 [assembly: AssemblyCulture("")]
 [assembly: ComVisible(false)]
 [assembly: Guid("50F53E6E-21D5-4780-8E67-273877DAA28C")]
-[assembly: AssemblyVersion("0.0.5.0")]
+[assembly: AssemblyVersion("0.0.6.0")]
-[assembly: AssemblyFileVersion("0.0.5.0")]
+[assembly: AssemblyFileVersion("0.0.6.0")]

Setlist/Setlist.csproj

@@ -10,6 +10,7 @@
     <AppDesignerFolder>Properties</AppDesignerFolder>
     <RootNamespace>Setlist</RootNamespace>
     <AssemblyName>Setlist</AssemblyName>
+    <!-- BSIPA plugins often target net472; this project uses net48. Todo: investigate why (SDK refs, BCL, host). -->
     <TargetFrameworkVersion>v4.8</TargetFrameworkVersion>
     <FileAlignment>512</FileAlignment>
     <DebugSymbols>true</DebugSymbols>

Setlist/manifest.json

@@ -3,7 +3,7 @@
   "id": "Setlist",
   "name": "Setlist",
   "author": "",
-  "version": "0.0.5",
+  "version": "0.0.6",
   "description": "Syncs playlists with external sources.",
   "gameVersion": "1.40.8",
   "dependsOn": {

docs/playlistmanager-add-map.md

@@ -80,3 +80,38 @@ So:
 - **Library code**: `selectedPlaylist.Add(...)` — implementation of how the entry is stored lives in **BeatSaberPlaylistsLib** (`IPlaylist`), not in this repository.
 
 There is no other `.Add(` on a playlist for this flow in the grep results; removing a song is the parallel path in `LevelDetailButtonsViewController.RemoveSong()`.
+
+## hooking into the process
+
+### 1. Subscribe to PlaylistManager’s public event (simplest)
+
+After a successful add from the **Add to playlist** UI, PlaylistManager raises a **public static** event:
+
+```16:18:PlaylistManager/Utilities/Events.cs
+        /// <summary>
+        /// Raised when an <see cref="BeatSaberPlaylistsLib.Types.IPlaylistSong"/> is added to an <see cref="BeatSaberPlaylistsLib.Types.IPlaylist"/>
+        /// </summary>
+        public static event Action<BeatSaberPlaylistsLib.Types.IPlaylistSong, BeatSaberPlaylistsLib.Types.IPlaylist> playlistSongAdded;
+```
+
+It is invoked **after** `RaisePlaylistChanged()` and `StorePlaylist()` succeed:
+
+```187:194:PlaylistManager/UI/ViewControllers/AddPlaylistModalController.cs
+                try
+                {
+                    selectedPlaylist.RaisePlaylistChanged();
+                    parentManager.StorePlaylist(selectedPlaylist);
+                    popupModalsController.ShowOkModal(modalTransform, string.Format("Song successfully added to {0}", selectedPlaylist.Title), null, animateParentCanvas: false);
+                    // TODO: Doesn't refresh the sprite.
+                    Events.RaisePlaylistSongAdded(playlistSong, selectedPlaylist);
+                }
+```
+
+In your plugin: add a **reference to `PlaylistManager.dll`**, a **manifest dependency** on PlaylistManager, then subscribe in `OnEnable` (or menu init) and unsubscribe in `OnDisable`:
+
+- Namespace: `PlaylistManager.Utilities`
+- Type: `Events`
+- Event: `playlistSongAdded`
+- Handler signature: `(IPlaylistSong song, IPlaylist playlist)` from `BeatSaberPlaylistsLib.Types`
+
+**Caveat:** This is only raised for adds that go through **this** code path. It is the **only** `RaisePlaylistSongAdded` call site in the repo, so adds done only via BeatSaberPlaylistsLib (or another mod) will **not** fire this event.