add jacketAsMovie post process #33
Open
Conversation
This file contains 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
Sourcery 评审指南此 Pull Request 通过引入异步封面后期处理和重构电影加载工作流程来增强 MovieLoader。它添加了新的配置条目、一个专用的异步后期处理方法，并更新了加载电影的逻辑，以处理各种来源（源电影、mp4 电影和封面），并具有适当的错误处理和回退机制。 封面后期处理流程时序图sequenceDiagram
participant GM as GetMovie (MovieLoader)
participant JP as JacketPostProcess (MovieLoader)
participant FS as FileSystem
participant CMD as cmd.exe (run.bat process)
GM->>GM: Check configuration & load jacket
GM->>JP: Call JacketPostProcess(jacket)
activate JP
JP->>FS: Resolve PostProcessorDir
JP->>FS: Delete old input.png and output.png (if exist)
JP->>JP: Create render texture & copy jacket
JP->>FS: Write input.png file
JP->>FS: Check existence of run.bat
JP->>CMD: Start process (cmd.exe /c run.bat)
CMD-->>JP: Process completes (with exit code)
alt Successful exit (exit code 0)
JP->>FS: Read output.png
JP->>JP: Create processed Texture2D
else Failure
JP->>JP: Log error and return null
end
JP->>FS: Delete input.png and output.png
deactivate JP
JP-->>GM: Return processed jacket texture
GM->>GM: Update movieInfo with processed jacket
更新后的 MovieLoader 类图classDiagram
class MovieLoader {
<<static>> bool jacketAsMovie
<<static>> bool jacketPostProcess
<<static>> readonly string PostProcessorDir
<<static>> (string, string, Texture2D, string) movieInfo
<<static>> VideoPlayer[] _videoPlayers
+async void GetMovie()
+static void LoadLocalBgaAwake(GameObject ____movieMaskObj, int ___monitorIndex)
+async Task<Texture2D> JacketPostProcess(Texture2D jacket)
}
%% Note: GetMovie calls JacketPostProcess internally
MovieLoader : +[HarmonyPatch] GetMovie()
MovieLoader : +[HarmonyPatch] LoadLocalBgaAwake()
文件级别变更
提示和命令与 Sourcery 互动
自定义您的体验访问您的 仪表板 以：
获取帮助Original review guide in EnglishReviewer's Guide by SourceryThis pull request enhances the MovieLoader by introducing asynchronous jacket post-processing and refactoring the movie loading workflow. It adds new configuration entries, a dedicated async method for post-processing, and updates the logic in loading movies to handle various sources (source movie, mp4 movie, and jacket) with proper error handling and fallback mechanisms. Sequence Diagram for Jacket Post-Processing FlowsequenceDiagram
participant GM as GetMovie (MovieLoader)
participant JP as JacketPostProcess (MovieLoader)
participant FS as FileSystem
participant CMD as cmd.exe (run.bat process)
GM->>GM: Check configuration & load jacket
GM->>JP: Call JacketPostProcess(jacket)
activate JP
JP->>FS: Resolve PostProcessorDir
JP->>FS: Delete old input.png and output.png (if exist)
JP->>JP: Create render texture & copy jacket
JP->>FS: Write input.png file
JP->>FS: Check existence of run.bat
JP->>CMD: Start process (cmd.exe /c run.bat)
CMD-->>JP: Process completes (with exit code)
alt Successful exit (exit code 0)
JP->>FS: Read output.png
JP->>JP: Create processed Texture2D
else Failure
JP->>JP: Log error and return null
end
JP->>FS: Delete input.png and output.png
deactivate JP
JP-->>GM: Return processed jacket texture
GM->>GM: Update movieInfo with processed jacket
Class Diagram for Updated MovieLoaderclassDiagram
class MovieLoader {
<<static>> bool jacketAsMovie
<<static>> bool jacketPostProcess
<<static>> readonly string PostProcessorDir
<<static>> (string, string, Texture2D, string) movieInfo
<<static>> VideoPlayer[] _videoPlayers
+async void GetMovie()
+static void LoadLocalBgaAwake(GameObject ____movieMaskObj, int ___monitorIndex)
+async Task<Texture2D> JacketPostProcess(Texture2D jacket)
}
%% Note: GetMovie calls JacketPostProcess internally
MovieLoader : +[HarmonyPatch] GetMovie()
MovieLoader : +[HarmonyPatch] LoadLocalBgaAwake()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @ck2739046 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding error handling or logging for the case where
Singleton<OptionDataManager>.Instance.GetMovieDataPathreturns null or an empty string. - The
movieInfovariable is being reset at the start ofGetMovieand then potentially overwritten multiple times; consider simplifying this logic.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| public static void LoadLocalBgaAwake(GameObject ____movieMaskObj, int ___monitorIndex) | ||
| { | ||
| [HarmonyPatch(typeof(TrackStartProcess), "OnStart")] | ||
| public static async void GetMovie() { |
There was a problem hiding this comment.
suggestion (bug_risk): Use of async void could mask exceptions and complicate error handling.
Consider changing the signature to return a Task to propagate exceptions properly. If this method must be async void due to Harmony patch requirements, ensure that any exceptions are appropriately caught and handled to prevent unobserved exception issues.
Suggested implementation:
[HarmonyPostfix]
[HarmonyPatch(typeof(TrackStartProcess), "OnStart")]
public static async Task GetMovie() {Make sure the project has a reference to System.Threading.Tasks (or any equivalent Task framework library) if not already imported.
Also check that Harmony supports async Task for this patch. If Harmony requires an async void method, then consider wrapping the method body in a try-catch to log and handle exceptions.
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 总结
新功能：
Original summary in English
Summary by Sourcery
New Features: