Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 53 additions & 3 deletions src/MusicPlayer/SongPlayer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@
public bool Listening { get; private set; }
public TSPlayer Player { get; set; }
public PlaySongInfo? CurrentSong { get; private set; }
public Queue<PlaySongInfo> SongQueue = new();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): 通过方法或只读封装来暴露队列,而不是通过可变的公共字段。

SongQueue 暴露为公共且可变的字段,会让调用方在 SongPlayer 的控制之外执行 Clear/Enqueue/Dequeue,从而可能导致 Listening/CurrentSong 与实际队列状态不同步。更推荐使用 private 队列(或只读视图),并通过 EnqueueSong/PlayNext 风格的方法来完成所有修改,以保持不变式。

建议的实现:

    public bool Listening { get; private set; }
    public TSPlayer Player { get; set; }
    public PlaySongInfo? CurrentSong { get; private set; }

    private readonly Queue<PlaySongInfo> _songQueue = new();
    public IReadOnlyCollection<PlaySongInfo> SongQueue => _songQueue;
    public SongPlayer(TSPlayer ply)
    {
        this.Listening = false;
    }

    public void EnqueueSong(PlaySongInfo playSongInfo)
    {
        if (playSongInfo == null)
            throw new ArgumentNullException(nameof(playSongInfo));

        _songQueue.Enqueue(playSongInfo);
    }

    public bool TryDequeueNext(out PlaySongInfo? nextSong)
    {
        if (_songQueue.Count == 0)
        {
            nextSong = null;
            return false;
        }

        nextSong = _songQueue.Dequeue();
        return true;
    }
  1. 确保文件顶部包含 using System;,以便使用 ArgumentNullException
  2. 将现有任何对 SongQueue.Enqueue/Dequeue/Clear 的外部调用更新为使用 EnqueueSongTryDequeueNext(如有需要,也可以添加 ClearQueue() 等辅助方法),以确保没有代码直接修改 _songQueue
  3. 如果存在依赖清空队列或查看下一首歌曲的逻辑,建议添加 ClearQueue() 和/或 PeekNextSong() 包装方法,而不是直接访问队列。
Original comment in English

suggestion (bug_risk): Expose the queue via methods or a read-only wrapper instead of a mutable public field.

Exposing SongQueue as a public, mutable field lets callers Clear/Enqueue/Dequeue outside SongPlayer’s control, which can desynchronize Listening/CurrentSong from the actual queue. Prefer a private queue (or a read-only view) and route all modifications through EnqueueSong/PlayNext-style methods to preserve invariants.

Suggested implementation:

    public bool Listening { get; private set; }
    public TSPlayer Player { get; set; }
    public PlaySongInfo? CurrentSong { get; private set; }

    private readonly Queue<PlaySongInfo> _songQueue = new();
    public IReadOnlyCollection<PlaySongInfo> SongQueue => _songQueue;
    public SongPlayer(TSPlayer ply)
    {
        this.Listening = false;
    }

    public void EnqueueSong(PlaySongInfo playSongInfo)
    {
        if (playSongInfo == null)
            throw new ArgumentNullException(nameof(playSongInfo));

        _songQueue.Enqueue(playSongInfo);
    }

    public bool TryDequeueNext(out PlaySongInfo? nextSong)
    {
        if (_songQueue.Count == 0)
        {
            nextSong = null;
            return false;
        }

        nextSong = _songQueue.Dequeue();
        return true;
    }
  1. Ensure using System; is present at the top of the file for ArgumentNullException.
  2. Update any existing external usages of SongQueue.Enqueue/Dequeue/Clear to use EnqueueSong and TryDequeueNext (or additional helper methods like ClearQueue() if needed), so no code mutates _songQueue directly.
  3. If there is logic that depends on clearing or peeking at the next song, consider adding ClearQueue() and/or PeekNextSong() wrapper methods instead of accessing the queue directly.


public SongPlayer(TSPlayer ply)
{
this.Player = ply;
this.Listening = false;
}

public bool StartSong(PlaySongInfo? playSongInfo = null)
public bool StartSong(PlaySongInfo? playSongInfo = null, bool fromQueue = false)
{
this.Listening = true;
if (playSongInfo is null)
Expand All @@ -29,20 +30,69 @@
this.CurrentSong.Play();
return true;
}

if (!fromQueue)
{
this.SongQueue.Clear();
}

this.CurrentSong = playSongInfo;
playSongInfo.Performer.Create(this.Player.Index);
playSongInfo.Play();
playSongInfo.OnCompleted += OnSongFinished;

Check failure on line 42 in src/MusicPlayer/SongPlayer.cs

View workflow job for this annotation

GitHub Actions / 构建插件

'PlaySongInfo' does not contain a definition for 'OnCompleted' and no accessible extension method 'OnCompleted' accepting a first argument of type 'PlaySongInfo' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 42 in src/MusicPlayer/SongPlayer.cs

View workflow job for this annotation

GitHub Actions / 构建插件

'PlaySongInfo' does not contain a definition for 'OnCompleted' and no accessible extension method 'OnCompleted' accepting a first argument of type 'PlaySongInfo' could be found (are you missing a using directive or an assembly reference?)
return true;
}

public bool EndSong()
private void OnSongFinished(int index)
{
if (SongQueue.Count > 0)
{
PlayNext();
}
else
{
this.Listening = false;
MusicPlayer.ListeningCheck();
}
}

public void PlayNext()
{
if (SongQueue.Count > 0)
{
var nextSong = SongQueue.Dequeue();
StartSong(nextSong, true);
this.Player.SendInfoMessage(GetString("正在播放队列中的下一首: {0}", nextSong.SongName ?? "未知歌曲"));

Check failure on line 65 in src/MusicPlayer/SongPlayer.cs

View workflow job for this annotation

GitHub Actions / 构建插件

'PlaySongInfo' does not contain a definition for 'SongName' and no accessible extension method 'SongName' accepting a first argument of type 'PlaySongInfo' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 65 in src/MusicPlayer/SongPlayer.cs

View workflow job for this annotation

GitHub Actions / 构建插件

'PlaySongInfo' does not contain a definition for 'SongName' and no accessible extension method 'SongName' accepting a first argument of type 'PlaySongInfo' could be found (are you missing a using directive or an assembly reference?)
}
else
{
this.Listening = false;
MusicPlayer.ListeningCheck();
}
}

public void EnqueueSong(PlaySongInfo songInfo)
{
SongQueue.Enqueue(songInfo);
}

public bool EndSong(bool manual = true)
{
this.Listening = false;
if (this.CurrentSong is null)
{
return false;
}

this.CurrentSong.Stop();
this.CurrentSong.OnCompleted -= OnSongFinished;

Check failure on line 88 in src/MusicPlayer/SongPlayer.cs

View workflow job for this annotation

GitHub Actions / 构建插件

'PlaySongInfo' does not contain a definition for 'OnCompleted' and no accessible extension method 'OnCompleted' accepting a first argument of type 'PlaySongInfo' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 88 in src/MusicPlayer/SongPlayer.cs

View workflow job for this annotation

GitHub Actions / 构建插件

'PlaySongInfo' does not contain a definition for 'OnCompleted' and no accessible extension method 'OnCompleted' accepting a first argument of type 'PlaySongInfo' could be found (are you missing a using directive or an assembly reference?)

if (manual)
{
this.SongQueue.Clear();
}

MusicPlayer.ListeningCheck();
return true;
}
}
}
Loading