feat: Add Oodle compression and decompression support#215
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @MRHRTZ, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces comprehensive support for Oodle compression and decompression, specifically targeting PlM formatted save files. It integrates the Oodle SDK's C library into the Python codebase using ctypes, enabling the tool to handle a wider range of save file formats. This enhancement directly addresses the need to process PlM saves, which previously were not supported, by providing new Oodle-specific compression and decompression routines and updating the core palsav module to leverage them alongside existing Zlib functionality.
Highlights
- Oodle Compression/Decompression: Adds full support for Oodle compression and decompression, specifically for save files using the
PlMformat, resolving issues related to handling these types of files. - C Library Integration via ctypes: Integrates the native Oodle C library (
oo2core_9_win64.dllorliboo2corelinux64.so.9) into the Python codebase using thectypesforeign function library, allowing direct calls to Oodle's compression and decompression functions. - Dual Compression Type Support: Refactors the
palsav.pymodule to conditionally support both Oodle (PlM) and existing Zlib (PlZ) compression types for save files, providing flexibility based on the file's magic bytes. - Potential ctypes Struct Mismatch: A potential discrepancy was identified in the
CompressOptionsctypes.Structuredefinition withinoodle_lib.py. Its fields do not fully align with theOodleLZ_CompressOptionsstruct declared in the providedoodle2.hheader, which could lead to incorrect data passing when interacting with the Oodle C library.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces support for Oodle compression and decompression. I've identified several critical issues related to the ctypes definitions that will cause crashes or incorrect behavior. The CompressOptions struct and the function signature for OodleLZ_GetCompressedBufferSizeNeeded do not match the provided C header files, which needs to be corrected. There are also portability issues with data types and platform-specific library loading that should be addressed. Additionally, there are opportunities to improve performance by avoiding repeated loading of the Oodle library.
| class CompressOptions(ctypes.Structure): | ||
| _fields_ = [ | ||
| ("m_verbosity", ctypes.c_uint32), | ||
| ("m_pfnPrintf", ctypes.c_void_p), | ||
| ("m_pPrintfUserData", ctypes.c_void_p), | ||
| ("m_pfnJob", ctypes.c_void_p), | ||
| ("m_pJobUserData", ctypes.c_void_p), | ||
| ("m_jobNum", ctypes.c_int), | ||
| ("m_pfnUserAlloc", ctypes.c_void_p), | ||
| ("m_pUserAllocUserData", ctypes.c_void_p), | ||
| ("m_parent_handles", ctypes.c_void_p), | ||
| ("m_num_parent_handles", ctypes.c_int), | ||
| ("m_http_cache", ctypes.c_void_p), | ||
| ("m_fuzzSafe", ctypes.c_int), | ||
| ("m_checkCRC", ctypes.c_int), | ||
| ("m_dictionarySize", ctypes.c_int), | ||
| ("m_spaceSpeedTradeoffBytes", ctypes.c_int), | ||
| ("m_maxLocalDictionarySize", ctypes.c_int), | ||
| ("m_makeLongRangeMatcher", ctypes.c_int), | ||
| ("m_effort", ctypes.c_int), | ||
| ("m_minMatchLen", ctypes.c_int), | ||
| ("m_seekChunkReset", ctypes.c_bool), # PENTING | ||
| ("m_seekChunkLen", ctypes.c_int), # PENTING | ||
| ("m_profile", ctypes.c_int), | ||
| ("m_sendQuantumCRCs", ctypes.c_int), | ||
| ("m_maxHuffmansPerChunk", ctypes.c_int), | ||
| ("m_fastParser", ctypes.c_void_p), | ||
| ("m_reserved", ctypes.c_ubyte * 24), # Gunakan ubyte untuk memastikan ukuran byte pas | ||
| ] |
There was a problem hiding this comment.
The CompressOptions ctypes Structure does not match the OodleLZ_CompressOptions struct definition in the provided oodle2.h header file. This mismatch will lead to an incorrect memory layout when passing this struct to the Oodle library functions, likely causing memory corruption or crashes.
class CompressOptions(ctypes.Structure):
_fields_ = [
("unused_was_verbosity", ctypes.c_uint32),
("minMatchLen", ctypes.c_int32),
("seekChunkReset", ctypes.c_int32), # OO_BOOL is a 32-bit integer
("seekChunkLen", ctypes.c_int32),
("profile", ctypes.c_int32),
("dictionarySize", ctypes.c_int32),
("spaceSpeedTradeoffBytes", ctypes.c_int32),
("unused_was_maxHuffmansPerChunk", ctypes.c_int32),
("sendQuantumCRCs", ctypes.c_int32), # OO_BOOL is a 32-bit integer
("maxLocalDictionarySize", ctypes.c_int32),
("makeLongRangeMatcher", ctypes.c_int32), # OO_BOOL is a 32-bit integer
("matchTableSizeLog2", ctypes.c_int32),
("jobify", ctypes.c_int32),
("jobifyUserPtr", ctypes.c_void_p),
("farMatchMinLen", ctypes.c_int32),
("farMatchOffsetLog2", ctypes.c_int32),
("reserved", ctypes.c_uint32 * 4),
]| self.oodle_lib.OodleLZ_GetCompressedBufferSizeNeeded.argtypes = ( | ||
| ctypes.c_uint, # rawSize | ||
| ) | ||
| self.oodle_lib.OodleLZ_GetCompressedBufferSizeNeeded.restype = ctypes.c_uint |
There was a problem hiding this comment.
The function signature for OodleLZ_GetCompressedBufferSizeNeeded is incorrect. According to oodle2.h, the function is OodleLZ_GetCompressedBufferSizeNeeded(OodleLZ_Compressor compressor, OO_SINTa rawSize). The current Python definition is missing the compressor argument and uses incorrect types for rawSize and the return value.
self.oodle_lib.OodleLZ_GetCompressedBufferSizeNeeded.argtypes = (
Compressor, # compressor
ctypes.c_ssize_t, # rawSize
)
self.oodle_lib.OodleLZ_GetCompressedBufferSizeNeeded.restype = ctypes.c_ssize_t|
|
||
| src_array = (ctypes.c_char * src_len).from_buffer_copy(gvas_data) | ||
|
|
||
| max_comp_len = self.oodle_lib.OodleLZ_GetCompressedBufferSizeNeeded(src_len) |
There was a problem hiding this comment.
This call to OodleLZ_GetCompressedBufferSizeNeeded is missing the required compressor argument.
max_comp_len = self.oodle_lib.OodleLZ_GetCompressedBufferSizeNeeded(OODLE_COMPRESSOR, src_len)| sys.exit(1) | ||
|
|
||
| try: | ||
| decompressor = OodleDecompressor() |
There was a problem hiding this comment.
The class OodleDecompressor is not defined in this file. The correct class name is OodleLib.
decompressor = OodleLib()| lib_name = "liboo2corelinux64.so.9" | ||
| lib_subdir = "Linux" | ||
| elif sys.platform == "darwin": | ||
| lib_name = "liboo2corelinux64.so.9" |
There was a problem hiding this comment.
The library name liboo2corelinux64.so.9 is being used for the darwin platform (macOS). macOS uses .dylib shared libraries, not .so files. This will prevent the library from loading on macOS.
| self.oodle_lib.OodleLZ_Decompress.argtypes = [ | ||
| ctypes.c_void_p, # compressed buffer | ||
| ctypes.c_long, # compressed size (SINTa) | ||
| ctypes.c_void_p, # raw buffer | ||
| ctypes.c_long, # raw length (SINTa) | ||
| ctypes.c_int, # fuzz_safe | ||
| ctypes.c_int, # check_crc | ||
| ctypes.c_int, # verbosity | ||
| ctypes.c_void_p, # decode buffer base | ||
| ctypes.c_long, # decode buffer size | ||
| ctypes.c_void_p, # fp_callback | ||
| ctypes.c_void_p, # callback userdata | ||
| ctypes.c_void_p, # scratch buffer | ||
| ctypes.c_long, # scratch size | ||
| ctypes.c_int, # thread phase | ||
| ] | ||
| self.oodle_lib.OodleLZ_Decompress.restype = ctypes.c_long # SINTa | ||
|
|
||
| self.oodle_lib.OodleLZ_Compress.argtypes = ( | ||
| Compressor, # compressor | ||
| ctypes.POINTER(ctypes.c_char), # rawBuf | ||
| ctypes.c_long, # rawLen | ||
| ctypes.POINTER(ctypes.c_char), # compBuf | ||
| CompressionLevel, # level | ||
| ctypes.POINTER(CompressOptions), # pOptions | ||
| ctypes.c_void_p, # dictionaryBase | ||
| ctypes.c_void_p, # lrm | ||
| ctypes.c_void_p, # scratchMem | ||
| ctypes.c_long, # scratchSize | ||
| ) | ||
| self.oodle_lib.OodleLZ_Compress.restype = ctypes.c_long |
There was a problem hiding this comment.
The type OO_SINTa from the Oodle C header is intptr_t, which has a size that depends on the architecture. Using ctypes.c_long is not portable across different platforms and architectures.
self.oodle_lib.OodleLZ_Decompress.argtypes = [
ctypes.c_void_p, # compressed buffer
ctypes.c_ssize_t, # compressed size (SINTa)
ctypes.c_void_p, # raw buffer
ctypes.c_ssize_t, # raw length (SINTa)
ctypes.c_int, # fuzz_safe
ctypes.c_int, # check_crc
ctypes.c_int, # verbosity
ctypes.c_void_p, # decode buffer base
ctypes.c_ssize_t, # decode buffer size
ctypes.c_void_p, # fp_callback
ctypes.c_void_p, # callback userdata
ctypes.c_void_p, # scratch buffer
ctypes.c_ssize_t, # scratch size
ctypes.c_int, # thread phase
]
self.oodle_lib.OodleLZ_Decompress.restype = ctypes.c_ssize_t # SINTa
self.oodle_lib.OodleLZ_Compress.argtypes = (
Compressor, # compressor
ctypes.POINTER(ctypes.c_char), # rawBuf
ctypes.c_ssize_t, # rawLen
ctypes.POINTER(ctypes.c_char), # compBuf
CompressionLevel, # level
ctypes.POINTER(CompressOptions), # pOptions
ctypes.c_void_p, # dictionaryBase
ctypes.c_void_p, # lrm
ctypes.c_void_p, # scratchMem
ctypes.c_ssize_t, # scratchSize
)
self.oodle_lib.OodleLZ_Compress.restype = ctypes.c_ssize_t| from enum import IntEnum | ||
| from typing import Tuple, Optional | ||
|
|
||
| # Default compressor dan level |
There was a problem hiding this comment.
The comment is not in English. For clarity and consistency, please use English for all comments.
# Default compressor and level| RuntimeError: If compression fails""" | ||
| src_len = len(gvas_data) | ||
| if src_len == 0: | ||
| raise ValueError("Data input tidak boleh kosong.") |
There was a problem hiding this comment.
The error message is not in English. For consistency, please use English for all user-facing strings.
raise ValueError("Input data cannot be empty.")| if zlib: | ||
| return decompress_sav_to_gvas_with_zlib(data) | ||
|
|
||
| return OodleLib().decompress_sav_to_gvas(data) |
There was a problem hiding this comment.
A new instance of OodleLib is created on every call to this function. The OodleLib constructor loads the native library from disk, which is an expensive operation. To improve performance, you should instantiate OodleLib once at the module level and reuse that instance.
| return OodleLib().decompress_sav_to_gvas(data) | |
| from palworld_save_tools.oodle_lib import OodleLib | |
| _oodle_lib = OodleLib() | |
| def decompress_sav_to_gvas(data: bytes, zlib: bool = False) -> tuple[bytes, int]: | |
| if zlib: | |
| return decompress_sav_to_gvas_with_zlib(data) | |
| return _oodle_lib.decompress_sav_to_gvas(data) |
| if zlib: | ||
| return compress_gvas_to_sav_with_zlib(data, save_type) | ||
|
|
||
| return OodleLib().compress_gvas_to_sav(data, save_type) |
There was a problem hiding this comment.
Similar to decompress_sav_to_gvas, a new instance of OodleLib is created on every call to compress_gvas_to_sav. This is inefficient as it involves reloading the native library from disk repeatedly. Please consider reusing a single module-level instance of OodleLib to improve performance.
| return OodleLib().compress_gvas_to_sav(data, save_type) | |
| from palworld_save_tools.oodle_lib import OodleLib | |
| _oodle_lib = OodleLib() | |
| def compress_gvas_to_sav(data: bytes, save_type: int, zlib: bool = False) -> bytes: | |
| if zlib: | |
| return compress_gvas_to_sav_with_zlib(data, save_type) | |
| return _oodle_lib.compress_gvas_to_sav(data, save_type) |
|
Reopening this PR to switch from UE Oodle DLL to the open-source |
1 participant
Adds Oodle decompression and compression support to handle save files using the PlM format. This change resolves the issues discussed in #214 and further contributions for refinement are welcome.