feat&fix: 添加新插件BadApplePlayer,修复#1042#1041
Merged
ACaiCat merged 4 commits intoUnrealMultiple:masterfrom Oct 8, 2025
Merged
Conversation
Contributor
There was a problem hiding this comment.
你好 - 我已审阅了你的更改 - 以下是一些反馈意见:
- 插件在 LoadPositionData/SavePositionData 中使用了同步文件 I/O,这可能会阻塞服务器线程——请考虑使用异步文件操作或将其分载到后台任务。
- 在 InitializePlaybackArea、RenderFirstFrame、RenderDeltaFrame 和 ClearPlaybackArea 中存在大量用于发送瓦片块(嵌套的 CHUNK_SIZE 循环)的重复逻辑——请将其提取到一个共享的辅助函数中以减少代码重复。
- 你将位置数据、临时数据和视频资源的静态和实例状态混合使用,这可能在重新加载或并发会话时导致竞态条件——请将这些封装在线程安全的、实例范围的服务中或添加适当的锁。
供 AI 代理的提示
请处理此代码审查中的评论:
## 总体评论
- 插件在 LoadPositionData/SavePositionData 中使用了同步文件 I/O,这可能会阻塞服务器线程——请考虑使用异步文件操作或将其分载到后台任务。
- 在 InitializePlaybackArea、RenderFirstFrame、RenderDeltaFrame 和 ClearPlaybackArea 中存在大量用于发送瓦片块(嵌套的 CHUNK_SIZE 循环)的重复逻辑——请将其提取到一个共享的辅助函数中以减少代码重复。
- 你将位置数据、临时数据和视频资源的静态和实例状态混合使用,这可能在重新加载或并发会话时导致竞态条件——请将这些封装在线程安全的、实例范围的服务中或添加适当的锁。
## 单独评论
### 评论 1
<位置> `src/BadApplePlayer/Models/BadAppleVideo.cs:41` </位置>
<code_context>
+ public List<(int x, int y, bool isWhite)> GetFrameChanges(int frameIndex, bool[,]? previousFrame)
+ {
+ var changes = new List<(int x, int y, bool isWhite)>();
+ var frameData = this.FramesData[frameIndex];
+
+ if (frameData.Length < 2)
</code_context>
<issue_to_address>
**issue (bug_risk):** GetFrameChanges 中缺少 frameIndex 的边界检查。
在不验证 frameIndex 的情况下访问 FramesData 可能会导致 IndexOutOfRangeException。请为 frameIndex 添加边界检查。
</issue_to_address>
### 评论 2
<位置> `src/BadApplePlayer/BadApplePlayer.cs:198` </位置>
<code_context>
+ return;
+ }
+
+ var frameDelay = 1000 / video.Fps;
+ var currentFrameState = new bool[video.Width, video.Height];
+ var startTime = DateTime.Now;
</code_context>
<issue_to_address>
**issue (bug_risk):** 如果 video.Fps 为零,可能存在除以零的风险。
在此计算之前验证 video.Fps 大于零,以防止 DivideByZeroException。
</issue_to_address>
### 评论 3
<位置> `src/BadApplePlayer/BadApplePlayer.cs:463` </位置>
<code_context>
+ return;
+ }
+
+ Main.tile[x, y].wall = (ushort) (isWhite ? BaseWall : CodeWall);
+ WorldGen.paintWall(x, y, (byte) (isWhite ? BaseColor : CodeColor), true);
+ WorldGen.paintCoatWall(x, y, IsGlowPaintApplied ? (byte) 1 : (byte) 0, true);
</code_context>
<issue_to_address>
**issue (bug_risk):** 直接访问 Main.tile 可能会导致并发问题。
如果 Terraria 的 API 不是线程安全的,在此处更新 Main.tile 可能会导致竞态条件。请确保这些操作在主线程上执行或使用适当的同步机制。
</issue_to_address>
### 评论 4
<位置> `src/BadApplePlayer/CommandHandler/BadAppleCommandHandler.cs:168` </位置>
<code_context>
+ var session = new PlaybackSession(name, position, video, loop, player.Name);
+ sessions[name] = session;
+
+ Task.Run(async () => await plugin.PlayVideoAsync(session));
+
+ player.SendSuccessMessage(GetString($"开始在位置 '{name}' 播放BadApple视频{(loop ? " (循环模式)" : "")}!"));
</code_context>
<issue_to_address>
**suggestion (bug_risk):** “即发即弃”的任务可能会隐藏异常。
考虑处理或记录来自 PlayVideoAsync 的异常,以防止在发生错误时出现静默故障。
```suggestion
Task.Run(async () =>
{
try
{
await plugin.PlayVideoAsync(session);
}
catch (Exception ex)
{
// Log the exception or notify the player/admin
TShock.Log.Error($"Exception in PlayVideoAsync for session '{name}': {ex}");
player.SendErrorMessage(GetString($"播放BadApple视频时发生错误: {ex.Message}"));
}
});
```
</issue_to_address>
### 评论 5
<位置> `src/BadApplePlayer/README.md:39` </位置>
<code_context>
+
+## 反馈
+
+- 优先发issued -> 共同维护的插件库:https://github.com/UnrealMultiple/TShockPlugin
+- 次优先:TShock官方群:816771079
+- 大概率看不到但是也可以:国内社区trhub.cn ,bbstr.net , tr.monika.love
</code_context>
<issue_to_address>
**suggestion (typo):** 将 'issued' 替换为 'issue' 或其对应的中文 '问题' 以提高准确性。
使用 'issue' 或 '问题' 而不是 'issued' 来提高清晰度和正确性。
```suggestion
- 优先发issue -> 共同维护的插件库:https://github.com/UnrealMultiple/TShockPlugin
```
</issue_to_address>帮助我更有用!请在每条评论上点击 👍 或 👎,我将利用您的反馈来改进您的评论。
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- The plugin uses synchronous File I/O in LoadPositionData/SavePositionData which can block the server thread—consider using asynchronous file operations or offloading to a background task.
- There’s a lot of duplicated logic for sending tile chunks (the nested CHUNK_SIZE loops) across InitializePlaybackArea, RenderFirstFrame, RenderDeltaFrame, and ClearPlaybackArea—extract that into a shared helper to reduce code repetition.
- You’re mixing static and instance state for position data, temp data, and the video resource, which may lead to race conditions on reload or concurrent sessions—encapsulate these in thread-safe, instance-scoped services or add proper locking.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The plugin uses synchronous File I/O in LoadPositionData/SavePositionData which can block the server thread—consider using asynchronous file operations or offloading to a background task.
- There’s a lot of duplicated logic for sending tile chunks (the nested CHUNK_SIZE loops) across InitializePlaybackArea, RenderFirstFrame, RenderDeltaFrame, and ClearPlaybackArea—extract that into a shared helper to reduce code repetition.
- You’re mixing static and instance state for position data, temp data, and the video resource, which may lead to race conditions on reload or concurrent sessions—encapsulate these in thread-safe, instance-scoped services or add proper locking.
## Individual Comments
### Comment 1
<location> `src/BadApplePlayer/Models/BadAppleVideo.cs:41` </location>
<code_context>
+ public List<(int x, int y, bool isWhite)> GetFrameChanges(int frameIndex, bool[,]? previousFrame)
+ {
+ var changes = new List<(int x, int y, bool isWhite)>();
+ var frameData = this.FramesData[frameIndex];
+
+ if (frameData.Length < 2)
</code_context>
<issue_to_address>
**issue (bug_risk):** No bounds check for frameIndex in GetFrameChanges.
Accessing FramesData without validating frameIndex may cause an IndexOutOfRangeException. Please add a bounds check for frameIndex.
</issue_to_address>
### Comment 2
<location> `src/BadApplePlayer/BadApplePlayer.cs:198` </location>
<code_context>
+ return;
+ }
+
+ var frameDelay = 1000 / video.Fps;
+ var currentFrameState = new bool[video.Width, video.Height];
+ var startTime = DateTime.Now;
</code_context>
<issue_to_address>
**issue (bug_risk):** Potential division by zero if video.Fps is zero.
Validate that video.Fps is greater than zero before this calculation to prevent a DivideByZeroException.
</issue_to_address>
### Comment 3
<location> `src/BadApplePlayer/BadApplePlayer.cs:463` </location>
<code_context>
+ return;
+ }
+
+ Main.tile[x, y].wall = (ushort) (isWhite ? BaseWall : CodeWall);
+ WorldGen.paintWall(x, y, (byte) (isWhite ? BaseColor : CodeColor), true);
+ WorldGen.paintCoatWall(x, y, IsGlowPaintApplied ? (byte) 1 : (byte) 0, true);
</code_context>
<issue_to_address>
**issue (bug_risk):** Direct access to Main.tile may cause concurrency issues.
If Terraria's API is not thread-safe, updating Main.tile here could cause race conditions. Please ensure these operations occur on the main thread or use proper synchronization.
</issue_to_address>
### Comment 4
<location> `src/BadApplePlayer/CommandHandler/BadAppleCommandHandler.cs:168` </location>
<code_context>
+ var session = new PlaybackSession(name, position, video, loop, player.Name);
+ sessions[name] = session;
+
+ Task.Run(async () => await plugin.PlayVideoAsync(session));
+
+ player.SendSuccessMessage(GetString($"开始在位置 '{name}' 播放BadApple视频{(loop ? " (循环模式)" : "")}!"));
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Fire-and-forget Task may hide exceptions.
Consider handling or logging exceptions from PlayVideoAsync to prevent silent failures when errors occur.
```suggestion
Task.Run(async () =>
{
try
{
await plugin.PlayVideoAsync(session);
}
catch (Exception ex)
{
// Log the exception or notify the player/admin
TShock.Log.Error($"Exception in PlayVideoAsync for session '{name}': {ex}");
player.SendErrorMessage(GetString($"播放BadApple视频时发生错误: {ex.Message}"));
}
});
```
</issue_to_address>
### Comment 5
<location> `src/BadApplePlayer/README.md:39` </location>
<code_context>
+
+## 反馈
+
+- 优先发issued -> 共同维护的插件库:https://github.com/UnrealMultiple/TShockPlugin
+- 次优先:TShock官方群:816771079
+- 大概率看不到但是也可以:国内社区trhub.cn ,bbstr.net , tr.monika.love
</code_context>
<issue_to_address>
**suggestion (typo):** Replace 'issued' with 'issue' or the Chinese equivalent '问题' for accuracy.
Use 'issue' or '问题' instead of 'issued' for clarity and correctness.
```suggestion
- 优先发issue -> 共同维护的插件库:https://github.com/UnrealMultiple/TShockPlugin
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
LaoSparrow
approved these changes
Oct 8, 2025
Closed
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
添加插件
Sourcery 总结
添加 BadApplePlayer 插件,用于在泰拉瑞亚(Terraria)内使用墙砖和涂料播放经典的 Bad Apple 视频,并通过高效的增量渲染和基于命令的控制。
新特性:
/badapple命令,用于设置命名的播放位置和控制会话(播放、暂停、恢复、停止、列出、正在播放、删除、循环)构建:
文档:
Original summary in English
Summary by Sourcery
Add BadApplePlayer plugin to play the iconic Bad Apple video inside Terraria using wall tiles and paint with efficient incremental rendering and command-based controls.
New Features:
Build:
Documentation: