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.

Sign up for GitHub

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

Jump to bottom

Fix DATADIR compile option #1392

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix DATADIR compile option #1392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ option ( ENABLE_CLANG_ANALYSIS "When building with clang, enable the static anal
option ( CHECK_CCACHE "Check if ccache is installed and use it" OFF )
set ( MSVC_WARNING_LEVEL 3 CACHE STRING "Visual Studio warning levels" )
option ( FORCE_INSTALL_DATA_TO_BIN "Force installation of data to binary directory" OFF )
set ( DATADIR "" CACHE STRING "Where to place datafiles" )
set ( DATADIR "" CACHE STRING "Where to search for datafiles" )
set ( OPENXCOM_VERSION_STRING "" CACHE STRING "Version string (after x.x)" )

if ( CHECK_CCACHE )
Expand Down
10 changes: 7 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ User and Config folder:

Data folders:
- C:\Documents and Settings\\\<user\>\My Documents\OpenXcom\data (Windows 2000/XP)
- DATADIR build flag
- C:\Users\\\<user\>\Documents\OpenXcom\data (Windows Vista/7/8)
- \<game directory\>
- . (the current directory)
Expand All @@ -119,8 +120,9 @@ User and Config folder:
Data folders:
- $XDG\_DATA\_HOME/openxcom (if $XDG\_DATA\_HOME is defined)
- $HOME/Library/Application Support/OpenXcom (if $XDG\_DATA\_HOME is not defined)
- DATADIR build flag
- $XDG\_DATA\_DIRS/openxcom (for each directory in $XDG\_DATA\_DIRS if $XDG\_DATA\_DIRS is defined)
- /Users/Shared/OpenXcom
- /Users/Shared/OpenXcom (if $XDG\_DATA\_DIRS is not defined or is empty)
- . (the current directory)

### Linux
Expand All @@ -138,9 +140,11 @@ Config folder:
Data folders:
- $XDG\_DATA\_HOME/openxcom (if $XDG\_DATA\_HOME is defined)
- $HOME/.local/share/openxcom (if $XDG\_DATA\_HOME is not defined)
- DATADIR build flag
- $XDG\_DATA\_DIRS/openxcom (for each directory in $XDG\_DATA\_DIRS if $XDG\_DATA\_DIRS is defined)
- /usr/local/share/openxcom
- /usr/share/openxcom
- /usr/local/share/openxcom (if $XDG\_DATA\_DIRS is not defined or is empty)
- /usr/share/openxcom (if $XDG\_DATA\_DIRS is not defined or is empty)
- the directory data files were installed to
- . (the current directory)

## Configuration
Expand Down
7 changes: 5 additions & 2 deletions src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,9 +431,12 @@ set ( openxcom_src ${root_src} ${basescape_src} ${battlescape_src} ${engine_src}
set ( install_dest RUNTIME )
set ( set_exec_path ON )
set ( install_dest_dir bin )
if ( NOT "${DATADIR}" STREQUAL "" )
add_definitions( -DDATADIR="${DATADIR}" )
endif ()
if ( UNIX AND NOT APPLE )
set ( data_install_dir "${CMAKE_INSTALL_FULL_DATADIR}/openxcom" )
add_definitions( -DDATADIR="${data_install_dir}/" )
add_definitions( -DINSTALLDIR="${data_install_dir}/" )
Copy link
Member

Choose a reason for hiding this comment

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

Why the separate define?

Copy link
Author

Choose a reason for hiding this comment

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

So the executable knows where its data files were installed. Currently passing DATADIR to cmake does nothing and instead CMAKE_INSTALL_FULL_DATADIR is used to pass -DDATADIR to the preprocessor. Instead of removing the DATADIR cmake option I repurposed it to add an entry to the directory list. A possible application is wanting to install the game files to /usr/share but searching for data files in /opt/ as well (e.g. from a commercial copy of xcom)

As for changing the name of the define, I thought it would be confusing if DATADIR (the C #define) would be set by CMAKE_INSTALL_FULL_DATADIR (the cmake variable) while DATADIR (the cmake variable) was used to set a different c define. This way DATADIR sets DATADIR.

endif ()
if ( APPLE )
set ( sdl_src "${MACOS_SDLMAIN_M_PATH}" )
Expand All @@ -460,7 +463,7 @@ if ( APPLE )
endif ()
else ()
set ( data_install_dir "${CMAKE_INSTALL_FULL_DATADIR}/openxcom" )
add_definitions( -DDATADIR="${data_install_dir}/" )
add_definitions( -DINSTALLDIR="${data_install_dir}/" )
endif ()
endif ()
if ( set_exec_path )
Expand Down
21 changes: 14 additions & 7 deletions src/Engine/CrossPlatform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,10 @@ std::vector<std::string> findDataFolders()
PathAppendA(path, "OpenXcom\\");
list.push_back(path);
}

#ifdef DATADIR
snprintf(path, MAX_PATH, "%s\\", DATADIR);
list.push_back(path);
#endif
// Get binary directory
if (GetModuleFileNameA(NULL, path, MAX_PATH) != 0)
{
Expand Down Expand Up @@ -210,9 +213,12 @@ std::vector<std::string> findDataFolders()
#endif
}
list.push_back(path);

#ifdef DATADIR
snprintp(path, MAXPATHLEN, "%s/" DATADIR);
list.push_back(path);
#endif
// Get global data folders
if (char const *const xdg_data_dirs = getenv("XDG_DATA_DIRS"))
if (char const *const xdg_data_dirs = getenv("XDG_DATA_DIRS") && *xdg_data_dirs)
{
char xdg_data_dirs_copy[strlen(xdg_data_dirs)+1];
strcpy(xdg_data_dirs_copy, xdg_data_dirs);
Expand All @@ -224,16 +230,17 @@ std::vector<std::string> findDataFolders()
dir = strtok(0, ":");
}
}
else
{
Copy link
Member

Choose a reason for hiding this comment

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

Won't this break use cases where data is spread among multiple folders?

Copy link
Author

Choose a reason for hiding this comment

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

It will only break use cases where all of the following apply:

  • The data in /usr/(local/)share
  • /usr/(local/)share isn't in $XDG_DATA_DIRS
  • The game wasn't installed to /usr/(local/)share
  • DATADIR was not set to /usr/(local/)share

/usr/(local/)share are supposed to be only a fallback, if XDG_DATA_DIRS isn't set:

$XDG_DATA_DIRS defines the preference-ordered set of base directories to search for data files in addition to the $XDG_DATA_HOME base directory. The directories in $XDG_DATA_DIRS should be seperated with a colon ':'.

If $XDG_DATA_DIRS is either not set or empty, a value equal to /usr/local/share/:/usr/share/ should be used.

(https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html)

#ifdef __APPLE__
list.push_back("/Users/Shared/OpenXcom/");
#else
list.push_back("/usr/local/share/openxcom/");
list.push_back("/usr/share/openxcom/");
#ifdef DATADIR
snprintf(path, MAXPATHLEN, "%s/", DATADIR);
list.push_back(path);
#endif

#ifdef INSTALLDIR
snprintf(path, MAXPATHLEN, "%s", INSTALLDIR);
list.push_back(path);
#endif
// Get working directory
list.push_back("./");
Expand Down