lethal-guitar / RigelEngine

A modern re-implementation of the classic DOS game Duke Nukem II
GNU General Public License v2.0
921 stars 60 forks source link

Cannot build on macOS #360

Closed Kyle-Falconer closed 5 years ago

Kyle-Falconer commented 5 years ago

I'm following the instructions in the readme for macOS and I get the following error about a redundant semicolon in glm:

[  2%] Built target dbopl
[  6%] Built target entityx
[  8%] Built target glad
[ 10%] Built target speex_resampler
[ 10%] Building CXX object src/CMakeFiles/rigel_core.dir/engine/debugging_system.cpp.o
In file included from /Users/kylefalconer/sources/RigelEngine/src/engine/debugging_system.cpp:17:
In file included from /Users/kylefalconer/sources/RigelEngine/src/engine/debugging_system.hpp:22:
In file included from /Users/kylefalconer/sources/RigelEngine/src/engine/renderer.hpp:24:
In file included from /Users/kylefalconer/sources/RigelEngine/src/engine/shader.hpp:23:
In file included from /Users/kylefalconer/sources/RigelEngine/3rd_party/glm/glm/gtc/type_ptr.hpp:37:
In file included from /Users/kylefalconer/sources/RigelEngine/3rd_party/glm/glm/gtc/../gtc/quaternion.hpp:18:
In file included from /Users/kylefalconer/sources/RigelEngine/3rd_party/glm/glm/gtc/../gtc/matrix_transform.hpp:24:
In file included from /Users/kylefalconer/sources/RigelEngine/3rd_party/glm/glm/gtc/../mat4x4.hpp:5:
In file included from /Users/kylefalconer/sources/RigelEngine/3rd_party/glm/glm/gtc/.././ext/matrix_double4x4.hpp:5:
In file included from /Users/kylefalconer/sources/RigelEngine/3rd_party/glm/glm/gtc/../ext/../detail/type_mat4x4.hpp:188:
In file included from /Users/kylefalconer/sources/RigelEngine/3rd_party/glm/glm/gtc/../ext/../detail/type_mat4x4.inl:1:
In file included from /Users/kylefalconer/sources/RigelEngine/3rd_party/glm/glm/gtc/../ext/../detail/../matrix.hpp:161:
In file included from /Users/kylefalconer/sources/RigelEngine/3rd_party/glm/glm/gtc/../detail/func_matrix.inl:1:
In file included from /Users/kylefalconer/sources/RigelEngine/3rd_party/glm/glm/gtc/../ext/../detail/../geometric.hpp:116:
In file included from /Users/kylefalconer/sources/RigelEngine/3rd_party/glm/glm/gtc/../detail/func_geometric.inl:2:
In file included from /Users/kylefalconer/sources/RigelEngine/3rd_party/glm/glm/gtc/../ext/../detail/../common.hpp:532:
In file included from /Users/kylefalconer/sources/RigelEngine/3rd_party/glm/glm/gtc/../detail/func_common.inl:785:
In file included from /Users/kylefalconer/sources/RigelEngine/3rd_party/glm/glm/gtc/../ext/../detail/func_common_simd.inl:6:
/Users/kylefalconer/sources/RigelEngine/3rd_party/glm/glm/gtc/../ext/../detail/../simd/common.h:106:45: error: empty expression statement
      has no effect; remove unnecessary ';' to silence this warning [-Werror,-Wextra-semi-stmt]
        glm_vec4 const or0 = _mm_or_ps(and0, and1);;
                                                   ^
1 error generated.
make[2]: *** [src/CMakeFiles/rigel_core.dir/engine/debugging_system.cpp.o] Error 1
make[1]: *** [src/CMakeFiles/rigel_core.dir/all] Error 2
make: *** [all] Error 2

Once I modify the glm code to fix the issue, I get a new error:

...
[ 49%] Building CXX object src/CMakeFiles/rigel_core.dir/game_logic/game_world.cpp.o
In file included from /Users/kylefalconer/sources/RigelEngine/src/game_logic/game_world.cpp:28:
In file included from /Users/kylefalconer/sources/RigelEngine/src/loader/resource_loader.hpp:21:
/Users/kylefalconer/sources/RigelEngine/src/data/movie.hpp:27:3: error: explicitly defaulted default constructor is implicitly deleted
      [-Werror,-Wdefaulted-function-deleted]
  MovieFrame() = default;
  ^
/Users/kylefalconer/sources/RigelEngine/src/data/movie.hpp:34:9: note: default constructor of 'MovieFrame' is implicitly deleted because
      field 'mReplacementImage' has no default constructor
  Image mReplacementImage;
        ^
1 error generated.
make[2]: *** [src/CMakeFiles/rigel_core.dir/game_logic/game_world.cpp.o] Error 1
make[1]: *** [src/CMakeFiles/rigel_core.dir/all] Error 2
make: *** [all] Error 2
lethal-guitar commented 5 years ago

Interesting, which version of Clang (or Xcode) are you using? It looks like some more warnings were added that didn't exist in older versions. I'll take a look later today

avioli commented 5 years ago

Same thing here, although I didn't touch the 3rd_party lib.

MacOS High Sierra (not Mojave), thus Xcode is Version 10.1 (10B61).

> clang --version
Apple LLVM version 10.0.0 (clang-1000.11.45.5)
Target: x86_64-apple-darwin17.7.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
> brew info llvm
llvm: stable 8.0.0 (bottled), HEAD [keg-only]
Next-gen compiler infrastructure
https://llvm.org/
/usr/local/Cellar/llvm/8.0.0_1 (6,807 files, 3.4GB)

Can I silence/ignore the extra-semi-stmt?

lethal-guitar commented 5 years ago

Can I silence/ignore the extra-semi-stmt?

Yeah, my solution would be to add that warning to the list of ignored warnings in the top-level CMake file (see here ). Any warnings that only occur in 3rd-party code can also be disabled in src/base/warnings.hpp.

The "defaulted constructor deleted" sounds more like I should fix it in the code itself

Kyle-Falconer commented 5 years ago

When I fixed the constructor issue and semicolons, the build did complete.

However, I also had issues unzipping the 4duke.zip (from 3drealms.com) file using macOS. I had to use Windows to extract both files, then copy the extracted files back to my mac.

With all this done, the program was able to run.

avioli commented 5 years ago

Adding -Wextra-semi-stmt to the src/base/warnings.hpp helped go past the warning, but another occurred, before reaching 49%, which mentions macOS 10.14 (I'm on 10.13.6).

I'm guessing Mojave is a minimum requirement then.

...
[ 32%] Building CXX object src/CMakeFiles/rigel_core.dir/game_logic/ai/bomber_plane.cpp.o
In file included from /Users/avioli/repos/RigelEngine/src/game_logic/ai/bomber_plane.cpp:19:
/Users/avioli/repos/RigelEngine/src/base/match.hpp:40:10: error: call to unavailable function 'visit': introduced in macOS 10.14
  return std::visit(
         ^~~~~~~~~~
/Users/avioli/repos/RigelEngine/src/game_logic/ai/bomber_plane.cpp:108:9: note: in instantiation of function template
      specialization 'rigel::base::match<std::__1::variant<rigel::game_logic::behaviors::BomberPlane::FlyingIn,
      rigel::game_logic::behaviors::BomberPlane::DroppingBomb, rigel::game_logic::behaviors::BomberPlane::FlyingOut> &, (lambda at
      /Users/avioli/repos/RigelEngine/src/game_logic/ai/bomber_plane.cpp:109:5), (lambda at
      /Users/avioli/repos/RigelEngine/src/game_logic/ai/bomber_plane.cpp:129:5), (lambda at
      /Users/avioli/repos/RigelEngine/src/game_logic/ai/bomber_plane.cpp:144:5)>' requested here
  base::match(mState,
        ^
/usr/local/Cellar/llvm/8.0.0_1/bin/../include/c++/v1/variant:1535:26: note: candidate function [with _Visitor =
      rigel::base::detail::overloaded<(lambda at /Users/avioli/repos/RigelEngine/src/game_logic/ai/bomber_plane.cpp:109:5),
      (lambda at /Users/avioli/repos/RigelEngine/src/game_logic/ai/bomber_plane.cpp:129:5), (lambda at
      /Users/avioli/repos/RigelEngine/src/game_logic/ai/bomber_plane.cpp:144:5)>, _Vs =
      <std::__1::variant<rigel::game_logic::behaviors::BomberPlane::FlyingIn,
      rigel::game_logic::behaviors::BomberPlane::DroppingBomb, rigel::game_logic::behaviors::BomberPlane::FlyingOut> &>] has been
      explicitly made unavailable
constexpr decltype(auto) visit(_Visitor&& __visitor, _Vs&&... __vs) {
                         ^
1 error generated.
make[2]: *** [src/CMakeFiles/rigel_core.dir/game_logic/ai/bomber_plane.cpp.o] Error 1
make[1]: *** [src/CMakeFiles/rigel_core.dir/all] Error 2
make: *** [all] Error 2

Re unzipping - you can open a terminal in macOS and do unzip 4duke.zip and it will unzip it just fine.

lethal-guitar commented 5 years ago

@avioli

I'm guessing Mojave is a minimum requirement then.

Only if you want to build with Xcode's clang. I was building it successfully on Sierra when using Clang 7.0.0 from Homebrew.

But you need to make sure CMake picks up the right Clang and standard library, by doing the following before you run CMake:

export CC=/usr/local/opt/llvm/bin/clang
export CXX="$CC++"

export CPPFLAGS="-I/usr/local/opt/llvm/include"
export LDFLAGS="-L/usr/local/opt/llvm/lib -Wl,-rpath,/usr/local/opt/llvm/lib"
AntonioND commented 5 years ago

Rather than disabling warnings, you should stop using -Werror in your default build target and leave it for a development build target. If someone is using a different compiler than yours they'll always get one or two warnings that you didn't and they won't be able to build it. I get that you don't want to have warnings in your code, but this is counterproductive.

lethal-guitar commented 5 years ago

@AntonioND I disagree, warnings on other compilers are a sign for me that my warning coverage is incomplete, and I'd prefer to know about new warnings so I can either fix my code (like I did in https://github.com/lethal-guitar/RigelEngine/pull/366/commits/998440ded4b93472d3cf5cad467c8a3b9e32ee23) or decide to ignore them.

That being said, I do see that it would be useful to optionally compile without -Werror. It should be fairly straight-forward to add an option to CMake to do that - I'll look into it.

AntonioND commented 5 years ago

If you leave the project unattended for a few weeks, GCC or Clang are updated with new warnings, and I try to compile your game with the newest version, I won't be able to do it because of -Werror. That alone is a good enough reason to not enable it by default.

If there are new warnings, it's great to know it so that they can be fixed. But what if I don't have a GitHub account and I can't report the issue here? What if I have no idea about programming and I just want to play, and I follow your instructions only to see that it doesn't build? There are just to many situations in which this can be a problem. It's better to leave the option disabled by default.

lethal-guitar commented 5 years ago

What if I have no idea about programming and I just want to play, and I follow your instructions only to see that it doesn't build?

For that situation, pre-compiled binaries are the solution. I already do that for Windows (see Releases), but still need to do it for Mac and Linux.

Kyle-Falconer commented 5 years ago

Yeah the lack of pre-compiled binaries for macOS is what made me try building it myself.

AntonioND commented 5 years ago

But you can't easily create Linux binaries that run everywhere.

Kyle-Falconer commented 5 years ago

But you can't easily create Linux binaries that run everywhere.

No, but you can provide one for the most common distros, like Debian and Fedora. It just takes time and a bit of effort to set this up using CI/CD. However, if someone's using Linux, surely they'd be able to follow the instructions needed to compile their own.

AntonioND commented 5 years ago

Exactly, it is a pain to maintain binaries for different distros, and that's why it should be as easy as possible to build it. -Werror gets in the way of that.