feat: introduce local patcher dll to cmake #374
Conversation
There was a problem hiding this comment.
Pull request overview
This PR transitions the cslol-patcher dependency from being fetched at build time from GitHub releases to being vendored locally in the repository. The vendored files include the DLL, import library, and header file for the Windows patcher.
- Adds vendor files for cslol-patcher (DLL, lib, and header) to
cslol-tools/vendor/cslol-patcher/ - Updates CMake configuration to use imported library instead of FetchContent
- Modifies
.gitignoreto allow vendor files that would otherwise be excluded
Reviewed changes
Copilot reviewed 2 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cslol-tools/vendor/cslol-patcher/cslol-dll.lib | Import library for the cslol-patcher DLL |
| cslol-tools/vendor/cslol-patcher/cslol-api.h | C API header file defining the patcher interface |
| cslol-tools/CMakeLists.txt | Changes from FetchContent to IMPORTED library targeting local vendor files |
| cslol-tools/dep/CMakeLists.txt | Modifies miniz library setup (appears unrelated to PR purpose) |
| .gitignore | Adds exceptions to allow vendor files and ignores build directory |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cslol-tools/dep/CMakeLists.txt
Outdated
| # add_subdirectory(${miniz_SOURCE_DIR} ${miniz_BINARY_DIR}) | ||
| add_library(miniz STATIC | ||
| ${miniz_SOURCE_DIR}/miniz.c | ||
| ${miniz_SOURCE_DIR}/miniz_zip.c | ||
| ${miniz_SOURCE_DIR}/miniz_tinfl.c | ||
| ${miniz_SOURCE_DIR}/miniz_tdef.c | ||
| ) | ||
| target_include_directories(miniz PUBLIC ${miniz_SOURCE_DIR}) |
There was a problem hiding this comment.
This change to the miniz library setup appears unrelated to introducing the local patcher dll. The modification from using add_subdirectory to manually creating the library should either be explained in the PR description or moved to a separate commit/PR for better change tracking and easier review.
There was a problem hiding this comment.
This should be fixed by bumping the commit at
Newer versions have cmake version bumped, see https://github.com/richgel999/miniz/commits/master/
Should also fix:
The BUILD_COMMAND keyword was followed by an empty string or no value at
all. Policy CMP0174 is not set, so cmake_parse_arguments() will unset the
ARG_BUILD_COMMAND variable rather than setting it to an empty string.
While at it. All of it in a new PR problem.
|
@moonshadow565 I've opened a new pull request, #375, to work on those changes. Once the pull request is ready, I'll request review from you. |
No description provided.