Use installed SplashKit dependencies - #10
Conversation
jankiluitel
left a comment
There was a problem hiding this comment.
Thanks for making this update. Moving the project to use the installed SplashKit dependencies instead of maintaining a local copy is a good improvement, as it reduces duplicated files and makes the project easier to maintain.
I have one suggestion before this is merged:
Since this PR removes the entire include/splashkit directory, it would be helpful to confirm that the project has been built successfully on the supported platforms (Windows, macOS, and Linux if applicable). This change affects the project's build configuration, so verifying that a clean checkout still compiles using an installed SplashKit environment would provide confidence that no include paths or configuration issues remain.
Other than that, the include updates look consistent with the goal of the PR.
|
Thanks for the review. I have confirmed the project builds successfully from a clean checkout on Windows using an installed SplashKit environment with skm clang++ *.cpp -o DXBallGame-test, and I also launched the built game successfully. I currently only have access to a Windows environment, so I have not personally verified macOS or Linux builds yet. If cross-platform verification is required before merge, I’m happy to coordinate with another contributor or maintainer who has access to those platforms. |
Thanks for confirming the clean Windows build and successfully launching the game. That addresses the Windows side of my concern. Since this change removes the local SplashKit headers and affects the build configuration, I think it would be worthwhile to get at least one additional verification on macOS/Linux before merging, if those platforms are supported by the project. Once that is confirmed, I’m happy to approve. |
Summary
Testing