Skip to content
This repository has been archived by the owner on Jun 30, 2024. It is now read-only.

Added CMAKE support, tested on MacOS 14 and Windows 10 #6

Merged
merged 39 commits into from
Jun 15, 2024

Conversation

Longwater1234
Copy link
Contributor

  • cmake support with building instructions and requirements
  • tested on macOS 14 Sonoma and Windows 10
  • On macOS , it buils an app Bundle, with assets embedded inside.
  • attaches icon to .EXE on windows, and .app on MacOS
  • Added clang-format according to your code style (Google style)
  • Cross-platform resource loader class, to help get assets from disk or bundle.

@mircea-pavel-anton mircea-pavel-anton added the enhancement New feature or request label Apr 3, 2024
@mircea-pavel-anton mircea-pavel-anton self-assigned this Apr 3, 2024
@mircea-pavel-anton
Copy link
Owner

Hello @Longwater1234 and thanks for your PR!

It's been a while since I had a look at this project, but I'll do my best and review this PR as soon as I possibly can, hopefully this weekend!
I assume since this isn't a draft PR that it is done and ready for review?

@Longwater1234
Copy link
Contributor Author

Longwater1234 commented Apr 3, 2024 via email

@Longwater1234
Copy link
Contributor Author

Longwater1234 commented Apr 4, 2024

@mirceanton One more thing! You may also need to change the github workflows to use Cmake for Multiplatform builds. Cheers😁. Suggestions: Build for 3 platforms: Windows-latest, MacOS latest, Ubuntu latest

@Longwater1234
Copy link
Contributor Author

@mirceanton Hello, just reminding about the review, if it's ok

@mircea-pavel-anton
Copy link
Owner

@mirceanton Hello, just reminding about the review, if it's ok

Hi! I haven't forgotten, I've just been very busy with work. I'll look on this ASAP. In the meantime feel free to take a crack at the pipelines, if that's something you're interested in.

Copy link
Owner

@mircea-pavel-anton mircea-pavel-anton left a comment

Choose a reason for hiding this comment

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

I managed to get CMake working in a Windows 11 environment using VisualStudio 2022 and SFML-2.6.1

After fixing the path for the windows icon, I managed to start the project but I'm having some trouble actually running it.

image

I'll need to look into this some more eventually :)) it's been 3y since I touched C++

Also, I don't think I can really accept and merge these changes without updating the workflows as well, and I am 99% sure I won't really have the time to spend doing that as work has been keeping me rather busy.

Other notes:

  • The comments I added thus far are only in regards to the documentation since that is the only part I managed to focus on to get things going.
  • The code style changes/fixes seem fine.
  • I have no means to test the Mac compatibility which would normally make me want to leave it out since I will not be able to provide any support on this going forward.
  • I would have preferred to keep the .vscode tasks configuration since that was really cross platform assuming you have all of the required tools, but I can't say I am 100% sold on this
  • You should probably ignore all the .DS_Store files via .gitignore since, at least AFAIK, they are not needed

BUILDING.md Outdated
sudo "/Applications/CMake.app/Contents/bin/cmake-gui" --install
```
- This is how the home screen of CMake GUI looks like:
![cmake_screenshot](apple_cmake/cmake_gui.png)

Choose a reason for hiding this comment

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

This hyperlink seems to be broken as that file is not included in this repo/PR Maybe you meant cmake/cmake_gui.png?

BUILDING.md Outdated
Comment on lines 3 to 4
## On Windows
1. Just open this project folder in Visual Studio 2019 (or newer), with

Choose a reason for hiding this comment

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

Markdown syntax requires a newline after titles

BUILDING.md Outdated
Comment on lines 10 to 11
## On MacOS & Linux
1. Open this project directory in your terminal.

Choose a reason for hiding this comment

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

Markdown syntax requires a newline after titles

BUILDING.md Show resolved Hide resolved
README.md Outdated
Comment on lines 32 to 33
### For Linux
- Use your OS package manager to download SFML 2.5 or newer.

Choose a reason for hiding this comment

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

Markdown syntax requires a newline after titles

README.md Show resolved Hide resolved
cmake/README.md Outdated
Comment on lines 5 to 6
### Extra notes
If you installed SFML as "Frameworks", instead of "dylibs", you will need to embed the SFML frameworks (using Xcode) into the App Bundle, if you want to share your app with others. Use CMake GUI to generate an **XCode** project. Open project in XCode. Click menu **Editor** > **Add Build Phase** > **Add Copy Files Build Phase**. Click the `[+]` button at top, and select all SFML frameworks from `/Library/Frameworks` (audio, video, system etc.). Now you can **Build** your project.

Choose a reason for hiding this comment

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

Markdown syntax requires a newline after titles

res/win-icon.rc Outdated
@@ -0,0 +1 @@
IDI_ICON1 ICON DISCARDABLE "windows-icon.ico"

Choose a reason for hiding this comment

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

I think this should be "windows_icon.ico". My VS project would fail at compiling otherwise

@Longwater1234
Copy link
Contributor Author

Longwater1234 commented Apr 25, 2024

@mirceanton Oh, it's sad to see you cannot run it on Win11. I will cross check again on a fresh Windows installation.

  • Concerning workflows, i will add for all 3 main platforms (Win, MacOS, Linux), and run the workflow on my GitHub, so you can see the results.
  • For the .vscode i will restore the folder 📂
  • Sorry about .DS_Store files, i used wildcard to ignore it, but turns out it was already committed to Git. Could not catch them all on my Mac (they are hidden). I will fix this.
  • I will resolve all the issues you listed in the comments this weekend.

@mircea-pavel-anton
Copy link
Owner

@mirceanton Oh, it's sad to see you cannot run it on Win11. I will cross check again on a fresh Windows installation.

  • Concerning workflows, i will add for all 3 main platforms (Win, MacOS, Linux), and run the workflow on my GitHub, so you can see the results.
  • For the .vscode i will restore the folder 📂
  • Sorry about .DS_Store files, i used wildcard to ignore it, but turns out it was already committed to Git. Could not catch them all on my Mac (they are hidden). I will fix this.
  • I will resolve all the issues you listed in the comments this weekend.

That's great! Thank you for your contributions!

@Longwater1234
Copy link
Contributor Author

Longwater1234 commented May 4, 2024

@mirceanton i have fixed all the issues in the comments. The game can now run properly on Windows. Except for restoring .vscode 📁 folder, because i have observed files within vscode folder were specifically for Makefile, while now we have migrated to Cmake build. No worries, the make can still be used with CMake just smoothly, see BUILDING.md 😊

@Longwater1234
Copy link
Contributor Author

@mirceanton Done added workflows for Windows and Ubuntu using Cmake. Both finish successfully. You can view on my github fork, on Actions tab.

.github/workflows/cmake-multi-platform.yml Outdated Show resolved Hide resolved
.github/workflows/cmake-multi-platform.yml Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@mircea-pavel-anton mircea-pavel-anton merged commit 230ea09 into mircea-pavel-anton:master Jun 15, 2024
3 checks passed
@mircea-pavel-anton
Copy link
Owner

@Longwater1234 looks good. I went ahead and got this merged. Thanks for the contribution!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants