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

CMake Spring Cleaning #514

Merged
merged 61 commits into from
May 17, 2019
Merged

Conversation

jameskr97
Copy link
Member

@jameskr97 jameskr97 commented May 2, 2019

This is my eighth PR on this repository.

Here is a summary of everything to be changed in this pull request.

Required before merge

  • Windows working with the following generators
    • Visual Studio 15 2017
    • Visual Studio 16 2019
    • Ninja
  • Static Analysers and formatters
    • cppcheck
    • clang-tidy
    • clang-format (To be completed in isolated PR due to complexity of solving the issues this project has with consistent formatting, and not wanting to do a Formatting changes commit with a large diff)
    • include_what_you_use (Also to be completed in isolated PR as it requires significant changes)
    • link_what_you_use (lwyu is a simple addition as it's a feature within CMake)
  • Doxygen within CMake
  • LDoc within CMake
  • Resolve Windows error supression (We get too many errors. We only want etterna specific errors.)

Other Fixes

  • libmad configured properly. Previously would stop certain packs from loading when attempting to delete references to audio files. (This specific bug occurred when attempting to delete a RageSoundReader_FileReader object at Song.cpp:706) 15b505d

It has been replaced with a upgraded file.
Header changes, and download shields link to releases page.
Updated image and explanation for macOS Gatekeeper.
Typo fixes were suggested on discord from donte (Providence), Fission, and poco.
The old file is now completely irrelevant, and has been removed, with the new one taking it's place.
Somehow these tags made it through all previous testing. They have now been switched for native compiler comparability.
@jameskr97
Copy link
Member Author

Priority changed to medium. It contains a build fix on windows that could cause both 32bit and 64bit ffmpeg to always be linked, even if the current build architecture does not call for one or the other to be linked.

jameskr97 added 8 commits May 2, 2019 22:39
Details are to tell potential contributions how they could work on something new.
…FORM

CMAKE_GENERATOR while was usable, is not compatible across different Windows generators. CMAKE_GENERATOR_PLATFORM adds usability across all recent CMake generators, and standardizes how the CMake command is used across visual studio generators.
Since some libraries are statically linked, we had to add the logic to tell what to statically link if we're using ninja. Anything built from source does not require this logic. Maybe this can be revisited in the future to simplify it, since it is significant duplicate code, though I believe this is preferable to having a custom `USING_NINJA` variable in the CMake script.
Previously, all warnings produced by any library would be displayed in the console. Now, it has been significantly suppressed by removing the CMake default warning level, and overriding the variable at the directory level in extern/CMakeLists.txt
@jameskr97 jameskr97 self-assigned this May 9, 2019
jameskr97 added 9 commits May 10, 2019 22:21
All information within the Building-Docs.md file has been updated and rewritten in the Building.md file
This directory provided a significant amount of bloat to the repository, and has been removed, since these files are no longer apart of the build toolchain.
This file is not apart of the build toolchain.
@jameskr97 jameskr97 marked this pull request as ready for review May 11, 2019 22:08
@jameskr97 jameskr97 requested review from martensm and poco0317 May 11, 2019 22:08
Docs/Building.md Outdated Show resolved Hide resolved
@poco0317 poco0317 self-requested a review May 12, 2019 10:19
Copy link
Member

@poco0317 poco0317 left a comment

Choose a reason for hiding this comment

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

Still works as intended from nothing through cmake into 64bit vs2017, building Release and RelWithDebug fine. All header/cpp files show up in the project.

@jameskr97 jameskr97 mentioned this pull request May 12, 2019
4 tasks
jameskr97 added 3 commits May 14, 2019 18:25
This commit changes from self-defined to compiler defined preprocessor directives.

CPU_X86 -> __i386__
CPU_X86_64 -> __x86_64__

This change cuts down on PPD's we worry about and leave it to the compiler (which any modern compiler will have them defined).
The commit fixes a critical bug where all audio files would not be properly read, and therefore cause the game on startup when reading from the Songs directory. The config.mad.in.h was restored, and added into the CMakeLists.txt file to be generated into a config.h in the libmad build directory.
The macOS linker "ld" does not seem to have the option "--no-as-needed", so lwyu is disabled on macOS.
@nico-abram nico-abram merged commit a3a3de3 into etternagame:develop May 17, 2019
This was referenced May 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants