Skip to content

Use installed SplashKit dependencies - #10

Open
SEBINSHASAMEERSHA wants to merge 1 commit into
thoth-tech:mainfrom
SEBINSHASAMEERSHA:fix-splashkit-dependencies
Open

Use installed SplashKit dependencies#10
SEBINSHASAMEERSHA wants to merge 1 commit into
thoth-tech:mainfrom
SEBINSHASAMEERSHA:fix-splashkit-dependencies

Conversation

@SEBINSHASAMEERSHA

Copy link
Copy Markdown

Summary

  • remove the direct external json.h include
  • use the installed SplashKit header through <splashkit.h>
  • remove the outdated bundled SplashKit headers
  • update VS Code include paths to stop referencing the removed headers

Testing

  • built successfully with: skm clang++ *.cpp -o DXBallGame-test
  • launched DXBallGame-test.exe successfully
  • confirmed the game runs normally

@jankiluitel jankiluitel left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@SEBINSHASAMEERSHA

Copy link
Copy Markdown
Author

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.

@jankiluitel

Copy link
Copy Markdown

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants