Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Cubos compatible with CMake find_package #1327

Merged
merged 8 commits into from
Nov 22, 2024

Conversation

RiscadoA
Copy link
Member

@RiscadoA RiscadoA commented Sep 27, 2024

Description

I tried to quickly give this a shot for 0.4 but it was way more complex than I anticipated.
I ended up needing to refactor pretty much the whole CMake configuration, as dealing with two libs at once (engine and core), plus their dependencies, of which some are obtained from add_subdirectory instead of find_package, was a big pain.

Sorry for not breaking this PR up further. I didn't work on this in a very structured way because the solution wasn't clear from the beginning. To sum it up, this PR achieves the following:

  • Makes Cubos find_package compatible. This means that now, Cubos can be installed by building the 'install' target, and can then be used in other projects (e.g. cubos-demo) through find_package(cubos), which is great, and way more standard!

  • Replaces all git submodules by CMake calls to FetchContent. Internally, FetchContent may end up creating git submodules, but the whole process is hidden from the user. I decided to change to this system because submodule integrate very badly with find_package, and I wasn't able to get it working. Also, I think everyone is tired of messing around with submodules, so this is a big QOL plus.

  • Make the process of deploying Cubos applications way easier. Tesseratos is an example of this - it can now be installed with the 'install' target! Basically I just setup some 'wiring' to make it possible to either pick the assets folder from the development source directory, or from the current directory. This can be toggled with the TESSERATOS_DISTRIBUTE option. The latter is what we want when we're deploying.

  • A very big cleanup on the whole CMake config, making it nicer to read and more organized, with proper comments. This was pretty much necessary to get the other things done.

One final thing: to put this all to test, I packaged Cubos with Nix! Maybe we can get it on nixpkgs on the next update 👀

Checklist

  • Self-review changes.
  • Evaluate impact on the documentation.
  • Add entry to the changelog's unreleased section.

@RiscadoA RiscadoA added C-Code-Quality A section of code that is hard to understand or change A-Meta B-Build Related to the build configuration labels Sep 27, 2024
@RiscadoA RiscadoA added this to the 0.5 milestone Sep 27, 2024
@RiscadoA RiscadoA self-assigned this Sep 27, 2024
@RiscadoA RiscadoA linked an issue Sep 27, 2024 that may be closed by this pull request
Copy link
Contributor

github-actions bot commented Sep 27, 2024

PR Preview Action v1.4.8
🚀 Deployed preview to https://GameDevTecnico.github.io/cubos/preview/pr-1327/
on branch gh-pages at 2024-11-22 09:56 UTC

Copy link

codecov bot commented Sep 27, 2024

Codecov Report

Attention: Patch coverage is 43.47826% with 13 lines in your changes missing coverage. Please review.

Project coverage is 54.23%. Comparing base (dcb021c) to head (67a2023).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
engine/src/assets/plugin.cpp 40.00% 12 Missing ⚠️
core/src/al/miniaudio_context.cpp 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1327       +/-   ##
===========================================
+ Coverage   41.94%   54.23%   +12.29%     
===========================================
  Files         441      438        -3     
  Lines       33129    25338     -7791     
  Branches     3872     2347     -1525     
===========================================
- Hits        13895    13742      -153     
+ Misses      19234    11596     -7638     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@RiscadoA RiscadoA changed the title Make Cubos usable CMake find_package compatible Make Cubos CMake find_package compatible Sep 29, 2024
@RiscadoA RiscadoA changed the title Make Cubos CMake find_package compatible Make Cubos compatible with CMake find_package Sep 29, 2024
@RiscadoA RiscadoA force-pushed the 1326-make-cubos-usable-in-cmake-through-find_package branch from 528aa87 to 1a572b1 Compare October 7, 2024 11:12
@RiscadoA RiscadoA force-pushed the 1326-make-cubos-usable-in-cmake-through-find_package branch from 1a572b1 to 0e47178 Compare November 5, 2024 13:35
@RiscadoA RiscadoA force-pushed the 1326-make-cubos-usable-in-cmake-through-find_package branch 2 times, most recently from b079992 to 903fb18 Compare November 16, 2024 16:22
@RiscadoA RiscadoA requested review from a team, diogomsmiranda and SrGesus and removed request for a team November 16, 2024 16:23
@RiscadoA RiscadoA force-pushed the 1326-make-cubos-usable-in-cmake-through-find_package branch 4 times, most recently from 1b25f44 to fc4ca73 Compare November 16, 2024 16:42
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

@RiscadoA RiscadoA marked this pull request as ready for review November 16, 2024 17:15
@RiscadoA RiscadoA requested review from a team as code owners November 16, 2024 17:15
@RiscadoA RiscadoA requested review from jdbaracho and removed request for a team November 16, 2024 17:15
@RiscadoA RiscadoA enabled auto-merge (rebase) November 16, 2024 17:15
@RiscadoA RiscadoA force-pushed the 1326-make-cubos-usable-in-cmake-through-find_package branch from fc4ca73 to c9db46e Compare November 16, 2024 17:21
Copy link
Contributor

@jdbaracho jdbaracho left a comment

Choose a reason for hiding this comment

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

I don't understand much about CMake so I can't really comment on the code itself, however by the description seems like a great QoL update for both devs and users!

Copy link
Contributor

@SrGesus SrGesus left a comment

Choose a reason for hiding this comment

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

Works on my machine LGTM

@RiscadoA RiscadoA linked an issue Nov 21, 2024 that may be closed by this pull request
@RiscadoA RiscadoA mentioned this pull request Nov 21, 2024
3 tasks
Copy link
Contributor

@diogomsmiranda diogomsmiranda left a comment

Choose a reason for hiding this comment

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

LGTM!!

This commit is a big one, and it does a lot of stuff. Sorry for not breaking it up further. I didn't work on this in a very structured way because the solution wasn't clear at the beginning.

This commit does the following things:
- Makes Cubos find_package compatible. This means that now, Cubos can be installed by building the 'install' target, and can then be used in other projects (e.g. cubos-demo) through find_package(cubos), which is great, and way more standard!
- Replaces all git submodules by CMake calls to FetchContent. Internally, FetchContent may end up creating git submodules, but the whole process is hidden from the user. I decided to change to this system because submodule integrate very badly with find_package, and I wasn't able to get it working. Also, I think everyone is tired of messing around with submodules, so this is a big QOL plus.
- Make the process of deploying Cubos applications way easier. Tesseratos is an example of this - it can now be installed with the 'install' target! Basically I just setup some 'wiring' to make it possible to either pick the assets folder from the development source directory, or from the current directory. The latter is what we want when we're deploying.
- A very big cleanup on the whole CMake config, making it nicer to read and more organized, with proper comments. This was pretty much necessary to get the other things done.
@RiscadoA RiscadoA force-pushed the 1326-make-cubos-usable-in-cmake-through-find_package branch from 5153fc8 to 67a2023 Compare November 22, 2024 09:54
@RiscadoA RiscadoA merged commit e32490e into main Nov 22, 2024
9 checks passed
@RiscadoA RiscadoA deleted the 1326-make-cubos-usable-in-cmake-through-find_package branch November 22, 2024 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Meta B-Build Related to the build configuration C-Code-Quality A section of code that is hard to understand or change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Cubos usable in CMake through find_package Make it easy to deploy games through CMake
4 participants