-
Notifications
You must be signed in to change notification settings - Fork 87
[emscripten] Web build #1776
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
base: master
Are you sure you want to change the base?
[emscripten] Web build #1776
Changes from 9 commits
5210d7e
a0601e8
5a27507
b3a76ad
79e8ea5
4d138dc
34b2223
f233505
b3c124f
11f89b6
5a36ad6
deca7c1
98fd863
4ddac87
5443372
666d680
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -298,6 +298,10 @@ include(RttrTestingCfg) | |||||
| set(rttrContribBoostDir ${CMAKE_CURRENT_SOURCE_DIR}/contrib/boost) | ||||||
| if(EXISTS ${rttrContribBoostDir} AND IS_DIRECTORY ${rttrContribBoostDir}) | ||||||
| set(BOOST_ROOT ${rttrContribBoostDir} CACHE PATH "Path to find boost at") | ||||||
| if (${CMAKE_SYSTEM_NAME} MATCHES "Emscripten") | ||||||
| set(Boost_INCLUDE_DIR "${rttrContribBoostDir}/include" CACHE PATH "Path to find boost at") | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not required: |
||||||
| set(Boost_LIBRARY_DIR "${rttrContribBoostDir}/lib" CACHE PATH "Path to find boost at") | ||||||
| endif () | ||||||
| endif() | ||||||
|
|
||||||
| set(BoostPackages filesystem iostreams locale program_options) | ||||||
|
|
@@ -306,7 +310,7 @@ if(BUILD_TESTING) | |||||
| list(APPEND BoostPackages unit_test_framework) | ||||||
| endif() | ||||||
|
|
||||||
| find_package(Boost 1.71 COMPONENTS ${BoostPackages}) | ||||||
| find_package(Boost 1.86 COMPONENTS ${BoostPackages}) | ||||||
| if(NOT Boost_FOUND) | ||||||
| message(FATAL_ERROR "You have to install boost (>=1.71) into contrib/boost or set (as CMake or environment variable) " | ||||||
| "BOOST_ROOT (currently: '${BOOST_ROOT}', Environment: '$ENV{BOOST_ROOT}'), " | ||||||
|
|
@@ -315,7 +319,9 @@ if(NOT Boost_FOUND) | |||||
| endif() | ||||||
|
|
||||||
| option(RTTR_USE_SYSTEM_BOOST_NOWIDE "Use system installed Boost.Nowide. Fails if not found!" "${RTTR_USE_SYSTEM_LIBS}") | ||||||
|
|
||||||
| if (${CMAKE_SYSTEM_NAME} MATCHES "Emscripten") | ||||||
| set(RTTR_USE_SYSTEM_BOOST_NOWIDE ON) | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verify indents
Suggested change
|
||||||
| endif () | ||||||
| if(Boost_VERSION_MINOR GREATER_EQUAL "74" OR RTTR_USE_SYSTEM_BOOST_NOWIDE) | ||||||
| # Boost 1.73 contains Boost.Nowide, 1.74 a required fix | ||||||
| find_package(Boost COMPONENTS nowide) | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -51,7 +51,10 @@ add_subdirectory(liblobby) | |||||||
| add_subdirectory(libsiedler2) | ||||||||
| add_subdirectory(libutil) | ||||||||
| add_subdirectory(mygettext) | ||||||||
| if (${CMAKE_SYSTEM_NAME} MATCHES "Emscripten") | ||||||||
| else () | ||||||||
|
||||||||
| if (${CMAKE_SYSTEM_NAME} MATCHES "Emscripten") | |
| else () | |
| if (NOT CMAKE_SYSTEM_NAME MATCHES "Emscripten") |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -29,7 +29,7 @@ void FreeAudioInstance(driver::IAudioDriver* driver) | |||
| delete driver; | ||||
| } | ||||
|
|
||||
| const char* GetDriverName() | ||||
| const char* GetAudioDriverName() | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This breaks the interface, see
|
||||
| { | ||||
| return "(SDL2) Audio via SDL2_mixer-Library"; | ||||
| } | ||||
|
|
@@ -62,7 +62,7 @@ AudioSDL::~AudioSDL() | |||
| */ | ||||
| const char* AudioSDL::GetName() const | ||||
| { | ||||
| return GetDriverName(); | ||||
| return GetAudioDriverName(); | ||||
| } | ||||
|
|
||||
| static AudioSDL* currentInstance = nullptr; | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,8 +3,11 @@ | |||||
| # SPDX-License-Identifier: GPL-2.0-or-later | ||||||
|
|
||||||
| set(SDL_BUILDING_LIBRARY ON) | ||||||
| if (${CMAKE_SYSTEM_NAME} MATCHES "Emscripten") | ||||||
|
||||||
| if (${CMAKE_SYSTEM_NAME} MATCHES "Emscripten") | |
| if (CMAKE_SYSTEM_NAME MATCHES "Emscripten") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| else () | |
| else() |
Same for endif()
and indent the find_package
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,11 +3,24 @@ | |
| # SPDX-License-Identifier: GPL-2.0-or-later | ||
|
|
||
| set(SDL2_BUILDING_LIBRARY ON) | ||
| if (${CMAKE_SYSTEM_NAME} MATCHES "Emscripten") | ||
|
||
| set(SDL2_FOUND TRUE) | ||
| else () | ||
| find_package(SDL2 2.0.5) | ||
|
|
||
| endif () | ||
| if(SDL2_FOUND) | ||
| add_library(videoSDL2 SHARED ${RTTR_DRIVER_INTERFACE} VideoSDL2.cpp VideoSDL2.h icon.h icon.cpp) | ||
| target_link_libraries(videoSDL2 PRIVATE videodrv s25util::common glad Boost::nowide SDL2::SDL2) | ||
| if (${CMAKE_SYSTEM_NAME} MATCHES "Emscripten") | ||
| add_library(videoSDL2 STATIC ${RTTR_DRIVER_INTERFACE} VideoSDL2.cpp VideoSDL2.h icon.h icon.cpp) | ||
| set(USE_FLAGS "--use-port=sdl2 --use-port=sdl2_image") | ||
| set_target_properties(videoSDL2 PROPERTIES | ||
| COMPILE_FLAGS "${USE_FLAGS}" | ||
| LINK_FLAGS "${USE_FLAGS}" | ||
| ) | ||
| target_link_libraries(videoSDL2 PRIVATE videodrv s25util::common Boost::nowide) | ||
| else () | ||
| add_library(videoSDL2 SHARED ${RTTR_DRIVER_INTERFACE} VideoSDL2.cpp VideoSDL2.h icon.h icon.cpp) | ||
| target_link_libraries(videoSDL2 PRIVATE videodrv s25util::common glad Boost::nowide SDL2::SDL2) | ||
| endif () | ||
| enable_warnings(videoSDL2) | ||
|
|
||
| if(WIN32) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,8 +15,12 @@ AddDirectory(lua) | |
| file(GLOB SOURCES_OTHER *.cpp *.h) | ||
| source_group(other FILES ${SOURCES_OTHER}) | ||
|
|
||
| find_package(Lua 5.1 REQUIRED) | ||
|
|
||
| if (${CMAKE_SYSTEM_NAME} MATCHES "Emscripten") | ||
| set(LUA_LIBRARIES /src/contrib/lua-5.4.7/build/lib/liblua.a) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hard-coded paths are a no-no. Can't the readme just state something like |
||
| set(LUA_INCLUDE_DIR /src/contrib/lua-5.4.7/build/include) | ||
| else () | ||
| find_package(Lua 5.1 REQUIRED) | ||
| endif () | ||
| include(GatherDll) | ||
| gather_dll(Lua) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,7 +19,29 @@ else() | |||||||||
| endif() | ||||||||||
|
|
||||||||||
| add_executable(s25client s25client.cpp commands.cpp ${s25client_RC}) | ||||||||||
| target_link_libraries(s25client PRIVATE s25Main Boost::program_options Boost::nowide rttr::vld) | ||||||||||
| if (${CMAKE_SYSTEM_NAME} MATCHES "Emscripten") | ||||||||||
| set(USE_FLAGS "-fwasm-exceptions \ | ||||||||||
| --use-port=sdl2 --use-port=sdl2_image --use-port=sdl2_mixer -sSDL2_MIXER_FORMATS=ogg \ | ||||||||||
| -O1 -gsource-map") | ||||||||||
|
||||||||||
| --use-port=sdl2 --use-port=sdl2_image --use-port=sdl2_mixer -sSDL2_MIXER_FORMATS=ogg \ | |
| -O1 -gsource-map") | |
| --use-port=sdl2 --use-port=sdl2_image --use-port=sdl2_mixer -sSDL2_MIXER_FORMATS=ogg \ | |
| -O1 -gsource-map") |
Is the -O1 intended? Why not just let CMake handle it via build type or use CXX_FLAGS on the cmake call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is nowide removed?
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,6 +50,13 @@ | |
| # include <csignal> | ||
| #endif | ||
|
|
||
| #ifdef __EMSCRIPTEN__ | ||
| # include <emscripten.h> | ||
| EM_JS(void, main_ready, (), { | ||
| Module?.gameReady?.(); | ||
| }); | ||
| #endif | ||
|
|
||
| namespace bfs = boost::filesystem; | ||
| namespace bnw = boost::nowide; | ||
| namespace po = boost::program_options; | ||
|
|
@@ -435,6 +442,17 @@ bool InitGame(GameManager& gameManager) | |
| return true; | ||
| } | ||
|
|
||
| #ifdef __EMSCRIPTEN__ | ||
| static void mainLoop() | ||
| { | ||
| killme = false; | ||
| if (!GAMEMANAGER.Run()) | ||
| { | ||
| emscripten_cancel_main_loop(); | ||
| } | ||
| } | ||
| #endif | ||
|
|
||
| int RunProgram(po::variables_map& options) | ||
| { | ||
| LOG.write("%1%\n\n", LogTarget::Stdout) % GetProgramDescription(); | ||
|
|
@@ -466,11 +484,10 @@ int RunProgram(po::variables_map& options) | |
| } | ||
| } | ||
|
|
||
| SetGlobalInstanceWrapper<GameManager> gameManager(setGlobalGameManager, LOG, SETTINGS, VIDEODRIVER, AUDIODRIVER, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need that for the regular game. Why the change? |
||
| WINDOWMANAGER); | ||
| setGlobalGameManager(new GameManager(LOG, SETTINGS, VIDEODRIVER, AUDIODRIVER, WINDOWMANAGER)); | ||
| try | ||
| { | ||
| if(!InitGame(gameManager)) | ||
| if(!InitGame(GAMEMANAGER)) | ||
| return 2; | ||
|
|
||
| if(options.count("map")) | ||
|
|
@@ -484,16 +501,19 @@ int RunProgram(po::variables_map& options) | |
| } | ||
|
|
||
| // Hauptschleife | ||
|
|
||
| while(gameManager.Run()) | ||
| #ifdef __EMSCRIPTEN__ | ||
| main_ready(); | ||
| emscripten_set_main_loop(&mainLoop, 0, true); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is blocking, isn't it? So using |
||
| #else | ||
| while(GAMEMANAGER.Run()) | ||
| { | ||
| #ifndef _WIN32 | ||
| killme = false; | ||
| #endif // !_WIN32 | ||
| } | ||
|
|
||
| #endif | ||
| // Spiel beenden | ||
| gameManager.Stop(); | ||
| GAMEMANAGER.Stop(); | ||
| libsiedler2::setAllocator(nullptr); | ||
| } catch(const RTTR_AssertError& error) | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,11 +59,24 @@ target_link_libraries(s25Main PUBLIC | |
| s25Common | ||
| rttrConfig | ||
| gamedata | ||
| glad | ||
| driver | ||
| Boost::filesystem Boost::disable_autolinking | ||
| PRIVATE BZip2::BZip2 Boost::iostreams Boost::locale Boost::nowide samplerate_cpp | ||
| ) | ||
| if (${CMAKE_SYSTEM_NAME} MATCHES "Emscripten") | ||
| target_link_libraries(s25Main PUBLIC videoSDL2 audioSDL) | ||
| set(USE_FLAGS "--use-port=sdl2 --use-port=sdl2_image \ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given the repetition I'm wondering if this can/should be passed to cmake rather than every variable. Especially with the hard-coded path |
||
| --use-port=sdl2_mixer -sSDL2_MIXER_FORMATS=ogg \ | ||
| -I/src/contrib/gl4es/include \ | ||
| -O1 -gsource-map") | ||
| set_target_properties(s25Main PROPERTIES | ||
| COMPILE_FLAGS "${USE_FLAGS}" | ||
| LINK_FLAGS "${USE_FLAGS} -sFULL_ES2 -L/src/contrib/gl4es/lib/ -lgl4es \ | ||
| --profiling-funcs -sASSERTIONS=2 -sLINKABLE" | ||
| ) | ||
| else () | ||
| target_link_libraries(s25Main PUBLIC glad) | ||
| endif () | ||
|
|
||
| if(WIN32 OR CYGWIN) | ||
| include(CheckIncludeFiles) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,7 +41,11 @@ bool GameManager::Start() | |
| settings_.Load(); | ||
|
|
||
| /// Videotreiber laden | ||
| #ifdef __EMSCRIPTEN__ | ||
| if(!videoDriver_.LoadDriver()) | ||
| #else | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the change suggested to this function this change could be removed (it ignores the settings then) |
||
| if(!videoDriver_.LoadDriver(settings_.driver.video)) | ||
| #endif | ||
| { | ||
| s25util::error(_("Video driver couldn't be loaded!")); | ||
| return false; | ||
|
|
@@ -57,7 +61,11 @@ bool GameManager::Start() | |
| videoDriver_.setGuiScalePercent(settings_.video.guiScale); | ||
|
|
||
| /// Audiodriver laden | ||
| #ifdef __EMSCRIPTEN__ | ||
| if(!audioDriver_.LoadDriver()) | ||
| #else | ||
| if(!audioDriver_.LoadDriver(settings_.driver.audio)) | ||
| #endif | ||
| { | ||
| s25util::warning(_("Audio driver couldn't be loaded!")); | ||
| // return false; | ||
|
|
@@ -195,7 +203,7 @@ void GameManager::ResetAverageGFPS() | |
| gfCounter_ = FrameCounter(FrameCounter::clock::duration::max()); // Never update | ||
| } | ||
|
|
||
| static GameManager* globalGameManager = nullptr; | ||
| GameManager* globalGameManager; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this change? This breaks encapsulation as the below function should be used |
||
|
|
||
| GameManager& getGlobalGameManager() | ||
| { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Common pitfall: This could get double-extended. Just omit the dollar sign