Add DEFAULT_ADDITIONAL_DIRS CMake option
#659
Conversation
BUILD.md
Outdated
| | `CMAKE_BUILD_TYPE` | `Debug` builds are generally larger, slower and contain extra correctness checks that will validate game data and interrupt gameplay when problems are detected.<br>`Release` builds are optimized for size and speed and do not include debugging information, which makes it harder to find problems. | `Debug` | | ||
| | `BUILD_EDITOR` | _(Windows-only)_ Build internal editor. | `OFF` | | ||
| | `BUILD_TESTING` | Enable testing. Requires GTest. | `OFF` | | ||
| | `DEFAULT_ADDITIONAL_DIRS` | A list of directories that Descent 3 will use as read-only base directories unless `-nodefaultadditionaldirs` is used (see [USAGE.md’s Base directories section](./USAGE.md#base-directories)). This value gets interpreted as a C++ expression, so you’ll need to use C++’s syntax. Here’s an example of a valid value: <pre lang=cpp>`{"C:\\Games\\Descent3\\", "D:\\"}`</pre> | `{}` | |
There was a problem hiding this comment.
it would be more idiomatic to provide a list in the CMake sense and then bracket it and quote it automatically
There was a problem hiding this comment.
I tried turning that variable into a list, but I ran into a problem. Consider this list:
C:\Directory 1\C:\Directory 2\
At the moment, it’s not possible to store that list as a CMake list. It’s also not possible to store this next list as a CMake list (although this next list is not a realistic example):
/home/user/[/home/user/\;
What do think I should do about this problem?
There was a problem hiding this comment.
that's annoying indeed. We could forbid trailing backslashes at the end of paths, or have a function to remove them. I could also ask Brad about it.
There was a problem hiding this comment.
That seems like a lot of work to make this value a CMake list instead of a C++ expression, and it might result in a situations where some paths can be used with -additionaldir and cannot be used with DEFAULT_ADDITIONAL_DIRS. Is it worth it?
There was a problem hiding this comment.
I was just thinking, as a packager, is there any situation where you would put static game assets in more than one given directory? Doing so would be confusing and very non-standard. DEFAULT_ADDITIONAL_DIR could simply be a single path value
There was a problem hiding this comment.
I was just thinking, as a packager, is there any situation where you would put static game assets in more than one given directory?
Yes, I outlined one of those situations in this PR’s commit message. Also, Nixpkgs has packages named descent1-assets and descent2-assets. If a descent3-assets package was added to Nixpkgs, then it would be convenient to have Descent3 executables that load some on the game data from the descent3-unwrapped package and some of the game data from the descent3-assets package.
Doing so would be confusing and very non-standard.
If that’s the case, then you should probably go and respond to the comment that I linked to in this PR’s commit message.
Jayman2000
force-pushed
the
default-additional-dirs
branch
from
feb6085 to
c3a45bd
Compare
The main motivation behind this change is to make it easier to package Descent 3 for Linux. For example, let’s say that you’re packaging Descent 3 for Debian and you want to make the package work well with Debian’s game-data-packager. The package for the Descent 3 engine will put d3-linux.hog in one directory, and the package for the proprietary Descent 3 game data will put d3.hog into a different directory [1]. The Descent 3 engine package can use the DEFAULT_ADDITIONAL_DIRS CMake option in order to make sure that both d3-linux.hog and d3.hog are located automatically. For the example value in USAGE.md, I decided to use Windows paths in order to showcase the fact that you have to escape backslashes. [1]: <DescentDevelopers#373 (comment)>
Jayman2000
force-pushed
the
default-additional-dirs
branch
from
c3a45bd to
a2dc357
Compare
Pull Request Type
Description
This pull request adds a
DEFAULT_ADDITIONAL_DIRSCMake option. This new CMake option can be used by packagers in order to help ensure that Descent 3 automatically finds game files that were installed by one or more packages. See the commit message for details.Related Issues
In the pull request description for #471, I had promised that I would open three additional pull requests if #471 got merged. This is one of those pull requests.
Checklist
Additional Comments