Moved multiple header files out of `lib`

[GravisZro]

Pull Request Type

• [x] Build and Dependency changes

Description

I moved several include files out of the lib directory and back to their own modules. The CMakeList.txt files have been updated accordingly.

Some filenames were normalize to match their implementation or header counterparts.

Related Issues

406

[GravisZro]

In my professional opinion we should be including PROJECT_SOURCE_DIR and using "subproject/headerfile.h" for includes. Using a common point of origin (PROJECT_SOURCE_DIR) would eliminate the need for adding target_include_directories directives. As a bonus it makes it clear which subproject the header is being included from.

This PR uses the current method which is to include the directory of the header you need (via target_include_directories) and if it needs a header in a directory that isn't in target_include_directories then you recursively repeat the process until you have finally included all referenced directories. This is a waste of time/effort that could easily be remedied if making this change was allowed.

[Lgt2x]

In my professional opinion we should be including PROJECT_SOURCE_DIR and using "subproject/headerfile.h" for includes. Using a common point of origin (PROJECT_SOURCE_DIR) would eliminate the need for adding target_include_directories directives. As a bonus it makes it clear which subproject the header is being included from.

My experience says otherwise :) It's very tempting to include everything from the project root, but thinking at a "module" level, our approach makes more sense: a CMake-defined library declares public header files. If you want to include them in another module, you need to link against the module that declares them and make it a dependency. This makes module coupling more transparent, which is useful for instance for testing or moving modules around.

With your described approach, how do you know which subproject depends on which? You have to parse every file from the module and extract include prefixes.

[GravisZro]

@Lgt2x I've done everything I can to appease @winterheart but he still insists I remove the listed header files, knowing that this will harm my ability to work on the project. Can you please do something? What happened to compromise?

[GravisZro]

@winterheart @Lgt2x Hey, what's the deal?

[Lgt2x]

@winterheart @Lgt2x Hey, what's the deal?

he's not available this week, I'll take a closer look at the PR whenever I get the chance

[GravisZro]

First pass, some small adjustments to make, but good job otherwise. I'd still like a second pair of eyes to review this thoroughly, changes to the build system are always scary

The requested changes have been made and the branch is now up to date.

[GravisZro]

@Lgt2x After investigating the issue, I'm not seeing how the linux version managed to work before. The static Database pointer value is initialized as NULL and dbase uses that as a pointer to a parent. It seem like there was some dependency on static initialization order and as a result of changing the order, it would fail. Anyway, it now longer uses Database to init dbase which works just fine on Linux. I would make another PR but it's not a big change and it would just cause more conflicts.

[Lgt2x]

That's one reason why static variables suck, and D3 is full of them :( Ok on Linux, I'll test on Windows and merge if nobody opposes

[Lgt2x]

@GravisZro could you run it on Win32 properly? I get an error "Unable to find version key for Descent 3" error on startup, maybe I messed something up (this branch, rebased on latest master)

[GravisZro]

@Lgt2x I got that error too when I tried it (wine). However, I think I've fully corrected the situation (no error box on start) but I need to you to do a full test on actual Windows.

[Lgt2x]

The error seems to persist

[Lgt2x]

Do you need help debugging Windows build?

[Lgt2x]

Do we absolutely need this last commit to fix the windows build? This PR is already big enough, it'd be better to keep modifications minimal

[GravisZro]

Do we absolutely need this last commit to fix the windows build? This PR is already big enough, it'd be better to keep modifications minimal

I'll be splitting this into another PR that will replace some static variable with functions that self-initialize. This will fix the linking order problem and simplify this PR.

[Lgt2x]

outdated, mostly covered by other Pull Requests. CMakeLists now declare headers properly