Include missing headers

[GravisZro]

Pull Request Type

• [x] Build and Dependency changes

Description

I've gotten sick of my IDE not knowing where header files are located. I've added many to the various CMakeList.txt files.

Checklist

• [x] I have tested my changes locally and verified that they work as intended.
• [x] I have documented any new or modified functionality.
• [x] I have reviewed the changes to ensure they do not introduce any unnecessary complexity or duplicate code.
• [x] I understand that by submitting this pull request, I am agreeing to license my contributions under the project's license.

Additional Comments

Please merge this.

[winterheart]

We working on splitting code into submodules where libs includes moved to according submodule (see cfile CMakeLists.txt for example how it's should look). In Cmake headers normally shouldn't be included as part targets. IDEs should detects them as dependent files based on target_include_directories directives.

[Lgt2x]

Seconding this, target_include_directories is much much better than adding headers with relative paths to each target

[GravisZro]

IDEs should detects them as dependent files based on target_include_directories directives.

@winterheart @Lgt2x That's great in theory... but my IDE doesn't do that. I've been told I need to compromise. Compromise requires both sides to make concessions. Are you willing to make a concession that will have no impact on you but will significantly help me?

[winterheart]

I've pointed on what direction we should change project's layout: create submodules that exports own headers via target_include_directories. What is your IDE and why it can't use target_include_directories directives?

[GravisZro]

@winterheart I'm using QtCreator 9.0.2 packed for Debian stable (Bookworm). You would have to ask the developers why it can't/doesn't use the directives.

[winterheart]

I've loaded project (current 5932087) as CMake project into QtCreator and don't see any issues with headers resolving:

On clicking to header with pressed Ctrl you'll open actual header in new tab.

[GravisZro]

I've loaded project ... On clicking to header with pressed Ctrl you'll open actual header in new tab.

Yeah, that's the thing, it's only some headers.

There is also the problem of when I do a search with the "Scope: Project 'Descent 3'" it doesn't search the headers.

Here's the result when you include the headers:

Also, they don't show up in the GUI like they should.

See how there are headers for bitmap but no headers displayed for cfile?

This is what it shows when you actually include them:

Honestly, it's not important because nothing in this PR will harm you but it will help me significantly, unless you are going to be so inflexible that you will refuse to help me despite it not impacting you at all.

[JeodC]

Add cfile.h to cfile/CMakeLists.txt and see if that works. Comparison below:

Cfile CMakeLists.txt

set(CPPS
  cfile.cpp
  hogfile.cpp
  inffile.cpp
)

add_library(cfile STATIC ${CPPS})
target_link_libraries(cfile PRIVATE
  ddio
)
target_include_directories(cfile PUBLIC
  $<BUILD_INTERFACE:
    ${PROJECT_SOURCE_DIR}/cfile
  >
)

Bitmap CMakeLisats.txt

set(HEADERS iff.h)
set(CPPS
  NewBitmap.cpp
  NewBitmap.h

  bitmain.cpp
  bumpmap.cpp
  iff.cpp
  lightmap.cpp
  pcx.cpp
  tga.cpp)

add_library(bitmap STATIC ${HEADERS} ${CPPS})
target_link_libraries(bitmap PRIVATE
  cfile
  ddebug
  ddio
  stb
)
target_include_directories(bitmap PUBLIC
  $<BUILD_INTERFACE:
    ${PROJECT_SOURCE_DIR}/bitmap
  >
)

[GravisZro]

Add cfile.h to cfile/CMakeLists.txt and see if that works.

I did... that's exactly what this PR does.

[JeodC]

Yeah, but it does that in addition to several other changes that may not be necessary. Does just adding cfile.h to cmakelists resolve the problem, or is there some other reason using relative header paths is necessary? It isn't good practice.

Edit: I misread, I see the other changes are all cmakelists files as well. In that case, as @winterheart said they're apparently working on moving all the header files in libs (which should really be called includes) to their own subdirectories.

[GravisZro]

as @winterheart said they're apparently working on moving all the header files in libs to their own subdirectories.

I didn't realize that's what he was trying to say but that's good news. Is there some sort of hold up on doing that? Hell, I could knock that out right now if they want.

[JeodC]

Maybe determining which ones to get rid of entirely via "include what you use" philosophy? I'm sure if it were a simple matter of moving files around they'd have done it already. Since they're all in one place right now it might be the perfect time to do that kind of cleanup.

[winterheart]

I recommend you update your QtCreator to recent version, 9.0.2 is very old. New version shows all needed headers and their usages.

[GravisZro]

I finally realized what target_include_directories actually does. It's not including files for your project, it's adding search paths for #include statements. That is to say that it's translates this:

 target_include_directories(cfile PUBLIC
  $<BUILD_INTERFACE:
    ${PROJECT_SOURCE_DIR}/bitmap
  >
)

into -I/home/gravis/project/Descent3/bitmap for the compiler.

New version shows all needed headers and their usages.

I find that difficult to believe this claim because that would mean it's showing files from /usr/include/. I would be more inclined to believe it if you posted an image of it display an expanded project tree that included all the header files.

[Lgt2x]

I havent gotten to #411 yet but it seems like a much better long-term solution than what you have here. Can we close this one?