Repository navigation
Conversation
WARNING - Broken UI - Static text
TODO: @bagusnl localization and binding
Also add early error handling to the base function
100% reviewed source file: 'en_US.json' on 'ja_JP'.
+ Fix presence setter concurrency + Fix UI thread hung due to Discord RPC initialization + Cache multiple start offset by caching the offset under PresetConfig's HashID. + Avoid calling multiple App Config key string and instead using its own property instead.
The function should now correctly serialize/deserialize the BinaryFormatter Header as previous implementation was based on a guess and trial&error work. Also efficiently Read/Write with more reduced memory footprint (even though, I know. It's not really a big deal)
This fix, however, isn't yet tested on any monitor other than 16:9 aspect ratio
# Main Goal Add declarative plugin-defined game settings support to Plugin.Core and render those settings in Collapse. Also harden plugin integration by: - Keeping the game-log callback alive throughout asynchronous game sessions. - Hydrating background notifications from an already-running activity’s current progress and status. - Correcting Wuthering Waves preload progress totals. # PR Status - Overall Status: Complete - Commits: Complete - Synced to base (Collapse:main): Yes - Build status: x64 Debug build passed - Crashing: None reported - Bugs found caused by PR: 0 reported # Changelog - [New] Add declarative plugin-defined game settings support to Plugin.Core and render those settings in Collapse. - [Fix] Keep the game-log callback alive throughout asynchronous game sessions. - [Fix] Initialize background notifications using the running activity’s current progress and status. - [Fix] Correct Wuthering Waves preload progress totals. - [Fix] Harden callback lifetimes and native-resource cleanup in plugin integration and background rendering.
For now, just rely on GC to automatically free them, just call Dispose if an object is an IDisposable
# Main Goal Optimizing the Background Video/Image Load routines and provides better memory allocation after dispose/use of each frames. The following PR also provides major improvements on the frame renderer for the background video playback, including direct GPU drawn to the frame's IDirect3DTexture outside of the UI thread, which reduces the VRAM usage and (hopefully) stutters. ## Reduced Memory and VRAM Allocation ### Video Background Playing (Honkai Impact 3rd - Mainland China - Picture 2 1080p) | Mem: Current Release | Mem: This PR | VRAM: Current Release | VRAM: This PR | |----|----|----|----| |  |  |  |  | ### Video Background On-Pause/Window minimized (Custom Video 2160p) | Mem: Current Release | Mem: This PR | VRAM: Current Release | VRAM: This PR | |----|----|----|----| |  |  |  |  | # Other Improvements ### [Fix] Missing Alpha Channel on HEIF/AVIF Encoded Background Image. The Alpha channel went missing while decoding HEIF/AVIF Encoded Images which caused by incorrect decoding request via the `PhotoSauce.MagicScaler` library. A bug fix has been submitted into a [PR here](saucecontrol/PhotoSauce#210) and while waiting for the PR to be merged, we copied our version of the workaround [here](https://github.com/CollapseLauncher/Collapse/tree/feat/mt-bg-renderer/Hi3Helper.MagicScaler.LibheifCL). | Source (AVIF encoded) | Decoded (.png) Before | Decoded (.png) After | |---------------------------|----------------------|---------------------| |  |  |  | ### [Imp] Reencoding Sprites to AVIF Previously, all the static sprites within Collapse assets (including Logo and Default Posters) are encoded using lossless PNG format with reduced palettes (256 colors + dithering). However, the size of the sprites are still considerably huge. So now, we decided to re-encode most of the sprites to AVIF with high bit-depth (up to 16-bit) while maintaining a smaller overall file sizes. The size reduction might be not that noticeable. | Before (Current Release) | After (This PR) | |------------------|-----------------| |  |  | ## PR Status : - Overall Status : Done - Commits : Done - Synced to base (Collapse:main) : Yes - Build status : OK - Crashing : No - Bug found caused by PR : Unknown ### Templates <details> <summary>Changelog Prefixes</summary> ``` **[New]** **[Imp]** **[Fix]** **[Loc]** **[Doc]** ``` </details>
| foreach (GameInstallPackage chunk in ChunkList) | ||
| { | ||
| long id = Http.GetHashNumber(count, i); | ||
| string path = PathOutput + $".{i + 1:000}"; | ||
| string pathLegacy = $"{PathOutput}.{id}"; | ||
| bool isUseLegacy = File.Exists(pathLegacy); | ||
|
|
||
| lastFile = isUseLegacy ? pathLegacy : path; | ||
| fileInfo = new FileInfo(path); | ||
| FileInfo fileInfoLegacy = new FileInfo(pathLegacy); | ||
| if (fileInfo.Exists) | ||
| { | ||
| fileInfo.Delete(); | ||
| } | ||
|
|
||
| if (fileInfoLegacy.Exists) | ||
| { | ||
| fileInfoLegacy.Delete(); | ||
| } | ||
| lastFile = chunk.PathOutput; | ||
| FileInfo fileInfo = new(chunk.PathOutput); | ||
| fileInfo.TryDeleteFile(true); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Bug: Unconditional return; in GameInstallPackage.DeleteFile() causes the loop over ChunkList to exit after deleting only the first chunk, leaving remaining chunks on disk.
Severity: MEDIUM
Suggested Fix
Remove the return; statement inside the foreach (GameInstallPackage chunk in ChunkList) loop so all chunks are iterated over and deleted, returning after the loop completes.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
CollapseLauncher/Classes/InstallManagement/Base/GameInstallPackage.cs#L254-L260
Potential issue: In `GameInstallPackage.DeleteFile()`, the loop over `ChunkList`
contains an unconditional `return;` inside the first iteration. When cleaning up a
multi-chunk package, only the first chunk (`.001`) is deleted, leaving chunks `.002`
through `.N` on disk after installation.
Did we get this right? 👍 / 👎 to inform future reviews.
| { | ||
| return; | ||
| } | ||
|
|
||
| LogType type = e.LogLevel switch | ||
| { | ||
| Verbosity.Verbose => LogType.Debug, | ||
| Verbosity.Debug => LogType.Debug, | ||
| _ => LogType.Default | ||
| }; | ||
| Progress.ProgressAllPercentage = Math.Round(totalWritten / (double)totalSize * 100, 2); | ||
| Progress.ProgressAllTimeLeft = TimeSpan.FromSeconds((totalSize - totalWritten) / speed.UnNanOrInfinity()); | ||
| Progress.ProgressAllSpeed = speed; | ||
| Progress.ProgressAllSizeTotal = totalSize; | ||
| Progress.ProgressAllSizeCurrent = totalWritten; | ||
|
|
||
| LogWriteLine(e.Message, type, true); | ||
| Status.IsProgressAllIndetermined = false; |
There was a problem hiding this comment.
Bug: Division by zero when speed is 0 in DeltaPatchProgress passes infinity to TimeSpan.FromSeconds, causing an OverflowException.
Severity: LOW
Suggested Fix
Guard against speed <= 0 before calculating the remaining time, or use ConverterTool.ToTimeSpanRemain as done in other progress callbacks.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
CollapseLauncher/Classes/InstallManagement/Base/InstallManagerBase.cs#L3349-L3363
Potential issue: `DeltaPatchProgress` calculates remaining time via
`TimeSpan.FromSeconds((totalSize - totalWritten) / speed.UnNanOrInfinity())`. If `speed`
is zero (such as before the initial 1000 ms measurement window closes), dividing by zero
results in infinity, which causes `TimeSpan.FromSeconds` to throw an
`OverflowException`.
Did we get this right? 👍 / 👎 to inform future reviews.
| { | ||
| Console.WriteLine(ex); | ||
| return ex; | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| #endregion | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: GetFromDatabase and PushToDatabase discard returned exceptions from import and export operations, falsely reporting success and leaking temporary files on failure.
Severity: MEDIUM
Suggested Fix
Check and return the Exception? returned by ImportSettings and ExportSettings. Wrap temporary file handling in try...finally blocks to ensure files are deleted regardless of errors.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
CollapseLauncher/Classes/GameManagement/GameSettings/BaseClass/ImportExportBase.cs#L536-L605
Potential issue: In `ImportExportBase`, `GetFromDatabase` and `PushToDatabase` discard
the `Exception?` returned by `ImportSettings` and `ExportSettings`. If settings fail to
import or export, failures are ignored and callers report success to the user.
Additionally, temporary files are not cleaned up in `finally` blocks when exceptions
occur during upload or download.
Did we get this right? 👍 / 👎 to inform future reviews.
Preview 1.84.7 (Codename: Columbina)
What's Changed? - 1.84.7
[New] Game Settings Subsystem Support for Plugin-based Games, by @Cryotechnic
We have implemented a new Update 6 extension for v1 API Standard. The plugin developer could now add customizations to the game into Collapse Launcher. To see the example of the implementation, see the source code of our Wuthering Waves plugin here.
[Imp] Optimizing Background Video/Image Renderer, by @neon-nyan
Optimizing the Background Video/Image Load routines and provides better memory allocation after dispose/use of each frames. The following PR also provides major improvements on the frame renderer for the background video playback, including direct GPU drawn to the frame's IDirect3DTexture outside of the UI thread, which reduces the VRAM usage and (hopefully) stutters.
Video Background Playing (Honkai Impact 3rd - Mainland China - Picture 2 1080p)
Video Background On-Pause/Window minimized (Custom Video 2160p)
Changelog Prefixes