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

Split more things into libraries #7554

Merged
merged 2 commits into from
Jan 10, 2025
Merged

Split more things into libraries #7554

merged 2 commits into from
Jan 10, 2025

Conversation

glebm
Copy link
Collaborator

@glebm glebm commented Nov 23, 2024

This does not fully address the issue of most everything being a giant library but it's a start.

Note that the libraries are OBJECT libraries, so they may (and currently do) contain references to missing symbols.

Also fixes the implementation of transitive dependency support for OBJECT libraries.

@glebm glebm force-pushed the path-test branch 2 times, most recently from 3d9f1b8 to ff7c974 Compare November 23, 2024 13:15
@glebm glebm marked this pull request as draft November 23, 2024 13:25
@glebm glebm force-pushed the path-test branch 18 times, most recently from eed4c8c to 44af477 Compare November 24, 2024 22:12
@glebm glebm marked this pull request as ready for review November 24, 2024 22:23
@glebm glebm enabled auto-merge (rebase) November 24, 2024 22:23
@glebm glebm force-pushed the path-test branch 3 times, most recently from d96faef to 1f5d04d Compare November 26, 2024 19:18
Copy link
Member

@AJenbo AJenbo left a comment

Choose a reason for hiding this comment

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

Splitting headless from engine and endian_write are the only things I found a bit odd.

Besides unit tests headless would also make sens if we implement a game server, maybe that gives some better context.

@glebm
Copy link
Collaborator Author

glebm commented Dec 1, 2024

  • endian_write: I wanted to split off the functions that depend on SDL
  • Headless: yeah mostly for unit tests. There is a quite a bit of code that only references headless rather than the whole engine

@glebm glebm force-pushed the path-test branch 3 times, most recently from 61c6a4a to 374c6ac Compare January 1, 2025 13:13
@glebm glebm marked this pull request as draft January 7, 2025 20:59
auto-merge was automatically disabled January 7, 2025 20:59

Pull request was converted to draft

@glebm glebm force-pushed the path-test branch 3 times, most recently from 49ed72c to 8b171be Compare January 8, 2025 07:31
@AJenbo
Copy link
Member

AJenbo commented Jan 9, 2025

Headless: yeah mostly for unit tests. There is a quite a bit of code that only references headless rather than the whole engine

sadly a lot graphics related code is spread all over the place

@glebm glebm force-pushed the path-test branch 3 times, most recently from 5871282 to efab121 Compare January 10, 2025 07:55
glebm added 2 commits January 10, 2025 08:02
This does not fully address the issue of most everything
being a giant library but it's a start.

Note that the libraries are OBJECT libraries, so they
may (and currently do) contain references to missing
symbols.

Also fixes the implementation of transitive dependency
support for OBJECT libraries.
Apparently required for `std::visit`
@glebm
Copy link
Collaborator Author

glebm commented Jan 10, 2025

Rebased and improved OBJECT library support so now dependencies are placed neatly next to the target definitions.

This PR no longer contains any source changes, only the build system changes.

@glebm glebm marked this pull request as ready for review January 10, 2025 08:18
@glebm glebm enabled auto-merge (rebase) January 10, 2025 08:18
Copy link
Member

@AJenbo AJenbo left a comment

Choose a reason for hiding this comment

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

A bit annoying having to define the object bounds in a meta language (cmake), but it's nice that we get to carve this up in to more manageable chunks.

@glebm glebm merged commit e5b6120 into diasurgical:master Jan 10, 2025
23 checks passed
@glebm glebm deleted the path-test branch January 10, 2025 15:17
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