Code Review

This pull request correctly fixes an issue with duplicated resource pack handling by introducing a de-duplication step based on pack hashes. The changes also improve the codebase by removing redundant logging and refactoring the resource pack handling logic to be cleaner. The implementation is solid. I have one suggestion to improve the readability of the logic for determining which resource packs need updating, making it more idiomatic and maintainable.

Code Review

This pull request correctly fixes an issue with duplicated resource packs by filtering them based on their hash. The changes also improve the code structure by centralizing resource pack loading and removing redundant logging. However, I've identified a critical thread-safety issue in the new implementation within ModConfig.kt that could lead to race conditions and application instability. My review includes a detailed explanation and a suggested refactoring to address this.

Code Review

This pull request effectively resolves the issue of handling duplicated resource packs by implementing de-duplication based on pack hashes. The refactoring to use a thread-safe ConfigState class and improving error handling for individual pack loading significantly enhances the robustness and maintainability of the code. The removal of redundant logging is also a welcome cleanup. I have a couple of suggestions to further improve error handling and code clarity.

Code Review

This pull request correctly addresses the issue of duplicated resource packs by using distinctBy on the pack hash. The refactoring to remove redundant logging and centralize resource pack processing in ModConfig using a thread-safe ConfigState is a good improvement. However, I've found a couple of high-severity issues in the new implementation. There's a bug in ModConfig where creating a new config file saves the old state, and the resend functionality in ResourcePackManager is broken. I've also left a medium-severity comment about improving the user experience when a config file is corrupted. Please review the detailed comments.

Code Review

This pull request introduces several important fixes and improvements for resource pack handling. The introduction of a thread-safe ConfigState to manage configuration loading is a great enhancement for stability. The logic for handling duplicated packs and applying diffs to players has also been significantly improved and made more robust. However, I've identified a critical issue in the new error handling logic for configuration loading. When the config file is corrupt, the system falls back to a default configuration but fails to save this valid default back to disk, which could lead to persistent issues across server restarts. My review includes a detailed comment with a suggested fix for this problem.

Code Review

This pull request introduces several significant improvements and bug fixes. The change to ModConfig to use a volatile ConfigState is an excellent fix for a critical thread-safety issue, ensuring atomic updates to the configuration. The error handling during config loading is now more robust and user-friendly. Additionally, the logic for handling duplicate resource packs has been correctly implemented using distinctBy, and a bug in the resource pack diffing logic within updatePlayerResourcePacks has been resolved. The code is also cleaner due to the removal of redundant logging. I have one suggestion to further optimize the resource pack processing.

Code Review

This pull request introduces a series of excellent fixes and refactorings. The correction of the duplicated pack handling, the critical thread-safety improvements for configuration loading using a volatile state object, and the refined error handling are all significant enhancements to the codebase's robustness and correctness. The logic for handling resource pack diffs has also been made more reliable. I have one suggestion to further improve performance by parallelizing resource pack processing, which could be beneficial if many packs are configured.

Code Review

This pull request updates the mod version to 1.0.1 and significantly refactors the resource pack loading and management. The ModConfig class now uses a ConfigState data class and a @Volatile property for thread-safe, immutable configuration, and loadConfig was refactored to handle JsonParseException and IOException more robustly, falling back to default configurations. Resource pack processing was made asynchronous using Kotlin coroutines (async, awaitAll) to improve performance and error handling for individual packs. A new getMappedResourcePacks() function was introduced to retrieve processed packs. Correspondingly, the ResourcePackManager was updated to use this new getMappedResourcePacks() method and its updatePlayerResourcePacks function was refactored to accept a direct list of ResourcePack objects, improving its logic for adding and removing packs. Minor formatting changes were also applied across several files, and redundant logging of loaded resource packs was removed from ServerResourcePackManager and ResourcePackManagerCommand. Review comments suggest refactoring the loadedConfig initialization in ModConfig.kt to use an immutable val with an if-else expression for better idiomatic Kotlin and safety, and optimizing appliedPacks[player] = newPacks.toList() in ResourcePackManager.kt by directly assigning newPacks to avoid an unnecessary list copy.

Code Review

This pull request introduces several significant improvements to the resource pack handling logic. The move to a thread-safe, atomic state object for configuration using @Volatile and a ConfigState class is a great step towards resolving concurrency issues. The parallel loading of resource packs using coroutines should provide a noticeable performance boost, and the enhanced error handling for config parsing makes the system more robust. The fixes for duplicated pack handling and the diff logic for applying packs to players are also solid improvements.

I've added a couple of review comments:

• A high-severity suggestion to make ResourcePackConfig immutable to fully realize the thread-safety benefits of the new state management model.
• A medium-severity suggestion for a minor code cleanup in the pack diffing logic.

Overall, this is a high-quality contribution that significantly improves the stability and performance of the mod. Well done!

Code Review

This pull request introduces several significant improvements and bug fixes. The implementation correctly addresses a critical thread-safety issue in configuration loading by using a volatile state object for atomic updates. It also properly fixes the handling of duplicated resource packs by de-duplicating them by both URL and content hash. Efficiency is enhanced by loading resource packs concurrently using coroutines, and error handling for configuration loading is now more robust. Furthermore, a bug in the resource pack diffing logic has been resolved. Overall, these changes make the code cleaner, more efficient, and more reliable. I have one minor suggestion to further improve performance.

Code Review

This pull request introduces a series of excellent improvements and bug fixes. The changes address a critical thread-safety issue in configuration loading by using a volatile state object, improve performance by loading resource packs concurrently, and fix incorrect handling of duplicate resource packs. The code is also made more robust and maintainable by using immutable data structures and fixing a bug in the resource pack diffing logic. Overall, this is a high-quality contribution. I have one suggestion to improve the automatic recovery from a corrupted configuration file.