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.

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

Lot of small codefixes related to configure #7363

Merged
merged 10 commits into from Mar 11, 2019

Conversation

@TrueBrain
Copy link
Member

TrueBrain commented Mar 10, 2019

During the development of cmake, a few small things were noticed and fixed. This PR already brings that to master, to both ease up the diff the cmake branch is creating, as to reduce conflicts over time.

Please let me know if you want any of these taken out in a separate PR to talk about it in more detail; I felt most are so innocent, they are fine in a single PR.

Many individual commits have a commit body explaining their existence.

@TrueBrain TrueBrain force-pushed the TrueBrain:cmake_preflight branch from df30f0a to 7850330 Mar 10, 2019
src/music/dmusic.cpp Outdated Show resolved Hide resolved
@TrueBrain TrueBrain force-pushed the TrueBrain:cmake_preflight branch from 7850330 to 68d7b5f Mar 10, 2019
It is the only library we use that calls itself with 'lib' in the
name. This might be confusing, but with the arrival of cmake a lot
of these things are automated. And detection will find 'liblzma',
not 'lzma', like with 'lzo', 'zlib', ..
@TrueBrain TrueBrain force-pushed the TrueBrain:cmake_preflight branch from 68d7b5f to 3c1ef31 Mar 10, 2019
@LordAro
Copy link
Member

LordAro commented Mar 10, 2019

I'm not really a fan of ICU_lx & ICU_i18n defines, they're a bit ...lowercase

TrueBrain added 6 commits Mar 10, 2019
… files)

By naming it in a different way, things get a bit confusing.
Especially if we are switching to CMake, which autodetects these
things, we need to use the name the authors of ICU gave it; not
our interpertation of that name.
… files)

By naming it in a different way, things get a bit confusing.
Especially if we are switching to CMake, which autodetects these
things, we need to use the name the authors of ICU gave it; not
our interpertation of that name.
config.lib happens to set GLOBAL_DATA_DIR in case it is not DOS
and not OS2, but this kind of deduction is annoying to maintain.
It is better to just check if the define you want to use is set,
and leave it to config.lib to set it or not depending on the OS.
…viour

If it was compiled with MingW, both / and \ were accepted as
path separator. On MSVC, only \ was. This is an unexpected
difference between binaries for the same platform. Remove this
discrepancy by accepting both / and \ on all platforms.
This is more in trend with other files, where if the driver is not
selected, we don't even attempt to compile it.
@TrueBrain TrueBrain force-pushed the TrueBrain:cmake_preflight branch from 3c1ef31 to 3bdd904 Mar 10, 2019
@TrueBrain
Copy link
Member Author

TrueBrain commented Mar 10, 2019

I'm not really a fan of ICU_lx & ICU_i18n defines, they're a bit ...lowercase

Convention dictates uppercase, but it looked silly. Either way, fixed.

@PeterN
PeterN approved these changes Mar 10, 2019
@TrueBrain TrueBrain merged commit 6a897a2 into OpenTTD:master Mar 11, 2019
8 checks passed
8 checks passed
OpenTTD CI Build #20190310.51 succeeded
Details
OpenTTD CI (Linux commit-checker) Linux commit-checker succeeded
Details
OpenTTD CI (Linux linux-amd64-clang-3.8) Linux linux-amd64-clang-3.8 succeeded
Details
OpenTTD CI (Linux linux-amd64-gcc-6) Linux linux-amd64-gcc-6 succeeded
Details
OpenTTD CI (Linux linux-i386-gcc-6) Linux linux-i386-gcc-6 succeeded
Details
OpenTTD CI (MacOS) MacOS succeeded
Details
OpenTTD CI (Windows Win32) Windows Win32 succeeded
Details
OpenTTD CI (Windows Win64) Windows Win64 succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.