Optimize making HOG files

[winterheart]

Pull Request Type

• GitHub Workflow changes
• Documentation or Wiki changes
• Build and Dependency changes
• Runtime changes
  • Render changes
  • Audio changes
  • Input changes
  • Network changes
  • Other changes

Description

Optimizing HOG files building by using add_custom_command instead of add_custom_target, which not enforces always rebuilding HOGs. In result this reduces incremental build time and optimize building process.

Related Issues

Fixes #635.

Screenshots (if applicable)

Checklist

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

Additional Comments

[sebholt]

Nice, that was quick!

It looks good to me in principle but there are some issues when building with Ninja Multi-Config.

As far as I understand the

• build directories and the
• search directories

for the HogMaker calls changed in these patches.

E.g. for d3-${HOG_NAME}.hog the build path changed

• from "$<TARGET_FILE_DIR:Descent3>/d3-${HOG_NAME}.hog" (evals to builds/linux/Descent3/Debug/d3-linux.hog)
• to "${PROJECT_BINARY_DIR}/Descent3/d3-${HOG_NAME}.hog" (evals to builds/linux/Descent3/d3-linux.hog)

It's similar for the search path, it changed

• from "$<TARGET_FILE_DIR:AIGame>" (evals to builds/linux/scripts/Debug)
• to "${CMAKE_CURRENT_BINARY_DIR}" (evals to builds/linux/scripts)

As a result HogMaker can't find any of the .so files.

It's a similar issues for the other hog files, the build and search paths changed there to.

Besides this good work!

[sebholt]

Ah, I see why you changed the build and search paths. The evaluation of the generator expression $<TARGET_FILE_DIR:Descent3> can't find the target inside the scope of the MakeHog function. That's unfortunate.

Maybe a (more verbose) solution would be to drop the MakeHog function and write the add_custom_command in place...

[sebholt]

I've tested writing add_custom_command in place and it fails with the same No target "Descent3" error. So the function scope is not the problem here.

[winterheart]

There still issues on mutli-config environment, I need additional time to fix it.

[winterheart]

Fixed building on multi-config environment, added support for cross-compiling (#465), disabled rebuilding HogMaker on every commit which cause to rebuild all HOG-files with it.

[sebholt]

Very nice, the build output of repeated builds is very clean now!

I'm a bit uncomfortable with the POST_BUILD copy solution though because now

• one can delete the hog files below "$<TARGET_FILE_DIR:Descent3>" and another build won't copy them
  there again - unless the Descent3 binary is rebuilt.
• Changing e.g. level code or d3-linux-fullhog.txt will update "${HOG_OUTPUT}" but not the hog file below "$<TARGET_FILE_DIR:Descent3>" - unless the Descent3 binary is rebuilt as well.

Here's a proposal for a possible solution:

• Set a DESCENT3_BUILD_DIR somewhere in the root CMakeLists.txt. All shipped binaries should be build there. Can be set to the default build location of the Descent3 binary.
• Pass DESCENT3_BUILD_DIR to the Descent3 target as build directory via set_target_property RUNTIME_OUTPUT_DIRECTORY
• Let MakeHog build all hogs below DESCENT3_BUILD_DIR. Let the OUTPUT parameter of MakeHog be a relative path in DESCENT3_BUILD_DIR.

[Lgt2x]

This is great. I've been trying to get a cross-compiled ARM64 build to validate the workflow. Setting HogMaker_DIR is not enough to find the HogMaker target, we may need to provide a HogMaker-config.cmake giving directions to the executable

[sebholt]

I'm a bit uncomfortable with the POST_BUILD copy solution though

I have (to annoy you with) another proposal. But it's a good one, I've tested it here and it works just fine.

• Instead of adding a POST_BUILD custom command to the Descent3 target, add a POST_BUILD custom command to the ${HOG_TARGET} in the MakeHog function.
• Add a COPY_DIR parameter to the MakeHog function that defines the subdirectory for the copy below $<TARGET_FILE_DIR:Descent3>.
• Update the MakeHog calls with COPY_DIR on demand

The custom command I've added in the MakeHog looks like this.

  add_custom_command(TARGET ${HOG_TARGET}
    POST_BUILD
    COMMAND ${CMAKE_COMMAND} -E make_directory "$<TARGET_FILE_DIR:Descent3>/${HOG_COPY_DIR}"
    COMMAND ${CMAKE_COMMAND} -E copy_if_different "${HOG_OUTPUT}" "$<TARGET_FILE_DIR:Descent3>/${HOG_COPY_DIR}"
    COMMENT "Copying HOG-file into Descent3 work dir"
  )

Note the copy_if_different, it copies the file only if it differs (obviously).

If you don't want to change the current solution, I can prepare another PR as a follow up to this PR with this approach.

[winterheart]

add_custom_command with POST_BUILD and copy_if_different still suffer issues when on unchanged executable user deletes old HOG file. I think DESCENT3_BUILD_DIR proposal is better solution. I'll try to implement this on week-end.

[winterheart]

Reworked project to use "${CMAKE_BINARY_DIR}/build" as directory for built targets.

[sebholt]

That's nice.

Only now all the libraries and executables end up in ${CMAKE_BINARY_DIR}/build.

Using the Makefile generator, the build directory now looks like this

 aigame2.so             BossCamera.so      Descent3              HogMaker          Level12.so   level1.so   level7.so    Merc1.so   myPowerHouse.so       Paranoia.so        testscript.so
 AIGame3.so             CanyonsCTF.so     'Descent3 Online.so'   InfernalBolt.so   level13.so   level2.so   level8.so    Merc3.so   Mysterious_Isle.so    PiccuStation.so    TrainingMission.so
 aigame4.so             CellTestLevel.so  'Direct TCP~IP.so'     Inversion.so      level14.so   level3.so   Level9.so    Merc4.so   netgames              Polaris.so         Y2K.so
 AIGame.so              ChrisTest.so       generic.so            LEVEL0.so         LEVEL15.so   level4.so   LevelS1.so   merc5.so   online                Quadsomniac.so
 barney.so              clutter.so         Geodomes.so           level10.so        Level16.so   level5.so   levelS2.so   Merc6.so   orbital.so            RudeAwakening.so
 BatteriesIncluded.so   d3-linux.hog       HalfPipe.so           level11.so        level17.so   Level6.so   Merc02.so    Merc7.so  'Parallax Online.so'   SewerRat.so

which is bloated.

The culprit is this

# All built targets will placed here
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/build")
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/build")

which is a too broad approach IMO.

IMO only

• Descent3
• *.hog
• netgames/*.d3m
• online/*.d3c

should go into D3_GENERATED_FILES_OUTPUT_DIRECTORY (as it used to be with the Descent3 directory).

For this I wouldn't set the broad CMAKE_{LIBRARY,RUNTIME}_OUTPUT_DIRECTORY variables (as mentioned above) but rather set the target properties

• RUNTIME_OUTPUT_DIRECTORY and LIBRARY_OUTPUT_DIRECTORY

only on the targets that actually should go to the D3_GENERATED_FILES_OUTPUT_DIRECTORY directory.

For the Descent3 target that is this:

set_target_properties(Descent3 PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${D3_GENERATED_FILES_OUTPUT_DIRECTORY}")

and for the netgames targets this:

set_target_properties(${NETGAME_MODULE} PROPERTIES LIBRARY_OUTPUT_DIRECTORY "${D3_GENERATED_FILES_OUTPUT_DIRECTORY}/netgames")

We would also get rid of the $<TARGET_FILE_DIR:Descent3> generator expression this way.

[sebholt]

Umm, I found one more issue.

-add_custom_target(NetgamesDir
-  COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:anarchy> "$<TARGET_FILE_DIR:Descent3>/netgames/Anarchy.d3m"
-  COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:coop> "$<TARGET_FILE_DIR:Descent3>/netgames/Co-op.d3m"
-  COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:ctf> "$<TARGET_FILE_DIR:Descent3>/netgames/CTF.d3m"
-  COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:entropy> "$<TARGET_FILE_DIR:Descent3>/netgames/Entropy.d3m"
-  COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:hoard> "$<TARGET_FILE_DIR:Descent3>/netgames/Hoard.d3m"
-  COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:hyperanarchy> "$<TARGET_FILE_DIR:Descent3>/netgames/Hyper-Anarchy.d3m"
-  COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:monsterball> "$<TARGET_FILE_DIR:Descent3>/netgames/Monsterball.d3m"
-  COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:roboanarchy> "$<TARGET_FILE_DIR:Descent3>/netgames/Robo-Anarchy.d3m"
-  COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:tanarchy> "$<TARGET_FILE_DIR:Descent3>/netgames/Team Anarchy.d3m"
-  COMMENT "Create netgames/ directory"
-)

This removal is fine because we build the libraries in the final location.
The issue is that the copy command renamed the libraries and that rename is lost now.
To restore it I found that is enough to set the LIBRARY_OUTPUT_NAME target property on the given targets.
For example this changes name for the tanarchy target to "Team Anarchy".

set_target_properties(${NETGAME_MODULE} PROPERTIES LIBRARY_OUTPUT_NAME "Team Anarchy")

[winterheart]

Changed set_target_properties only for Descent3 target. Other targets depends on it's location and get placed into correct directory.

Also fixed naming of netgames according to it's names on Steam installation.

[sebholt]

I've tested this with the Makefile and the Ninja Multi-Config generators and it all looks good now.

[sebholt]

Building works fine now (as mentioned above) but I found errors in BUILD.md and in USAGE.md.

• BUILD.md
  For linux it says the build path is builds/linux/Descent3/build/Debug or builds/linux/Descent3/build/Release. This is wrong, the build paths are builds/linux/build/Debug or builds/linux/build/Release.
  I believe for paths for win and mac are also wrong.

• USAGE.md
  The paths mentioned under 8. must be updated

  • builds/<platform>/Descent3/<build-type>/ now is builds/<platform>/build/<build-type>/
  • builds/linux/Descent3/Release now is builds/linux/build/Release

[sebholt]

The formatting is nice!

Unfortunately the paths in USAGE.md are now wrong in an other way.

builds/<platform>/<build-type>/ should be builds/<platform>/build/<build-type>/
and builds/linux/Release should be builds/linux/build/Release.

[Lgt2x]

All good, I made some changes to cross-comp in #642 and included the small USAGE.md fix requested by @sebholt . Nice job!