Merge lp:~widelands-dev/widelands/compiler-warnings-201908-2 into lp:widelands

[GunChleoc]

Fix compiler warnings and refactor config dir.

Do not send the merge command yet, I want to check the Travis/AppVeyor logs first.

[GunChleoc (gunchleoc)]

I'm done checking the logs, so this is ready for review now.

review: Needs Resubmitting

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5378. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/579360487.
Appveyor build 5148. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_compiler_warnings_201908_2-5148.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5378. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/579360487.
Appveyor build 5148. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_compiler_warnings_201908_2-5148.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5399. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/581705807.
Appveyor build 5169. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_compiler_warnings_201908_2-5169.

[Klaus Halfmann (klaus-halfmann)]

* Replaced:
- widelands_ball_of_mud
+ widelands_options
-> fine

* Codestyle: using this for a member function makes no sense.

* Smuggeled in some USE_XDG handling :-)

Will compile and start this, not doing much testing.

Please: review my refactor_gamehost before merging it becomes to much pain.

review: Approve

[GunChleoc (gunchleoc)]

Thanks for the review!

I already did a code review on your branch, I guess you overlooked it.

@bunnybot merge

[bunnybot (widelandsofficial)]

Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways.

Travis build 5399. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/581705807.

[Klaus Halfmann (klaus-halfmann)]

Locally commpiles without issues. Start upto Lobby was fine.

review: Approve

[Klaus Halfmann (klaus-halfmann)]

Mhh,one test in travis fails

test/maps/plain.wmf/scripting/test_ui.lua ...
  Running Widelands ... FAIL

But only for GCC_VERSION="4.9" BUILD_TYPE="Debug" ?

will run the test loocally, too

[GunChleoc (gunchleoc)]

The UI test sometimes fails because it takes too long for the game to close. I have implemented a fix in one of my other branches. Will rerun the Travis job.

[Klaus Halfmann (klaus-halfmann)]

local tests ok

review: Approve

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5399. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/581705807.
Appveyor build 5169. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_compiler_warnings_201908_2-5169.

[GunChleoc (gunchleoc)]

Thanks again!

@bunnybot merge

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2019-07-20 14:32:57 +0000
+++ src/CMakeLists.txt	2019-09-06 14:10:07 +0000
@@ -92,8 +92,6 @@
     wlapplication.h
     wlapplication_messages.cc
     wlapplication_messages.h
-    wlapplication_options.cc
-    wlapplication_options.h
   USES_SDL2
   USES_SDL2_IMAGE
   USES_SDL2_TTF
@@ -109,7 +107,6 @@
     graphic_fonthandler
     graphic_text
     io_filesystem
-    io_profile
     logic
     logic_exceptions
     logic_filesystem_constants
@@ -129,9 +126,25 @@
     ui_fsmenu_misc
     ui_fsmenu_network
     ui_fsmenu_options
+    widelands_options
     wui
 )
 
+wl_library(widelands_options
+  SRCS
+    wlapplication_options.cc
+    wlapplication_options.h
+  USES_SDL2
+  USES_SDL2_IMAGE
+  USES_SDL2_TTF
+  DEPENDS
+    base_log
+    io_filesystem
+    io_profile
+    logic_filesystem_constants
+)
+
+
 # TODO(sirver): Split into libs with useful names.
 wl_library(helper
   SRCS
 
=== modified file 'src/ai/defaultai.cc'
--- src/ai/defaultai.cc	2019-09-05 19:57:55 +0000
+++ src/ai/defaultai.cc	2019-09-06 14:10:07 +0000
@@ -5601,7 +5601,6 @@
 			inputs[16] = 0;
 			inputs[17] = (inputs_on_stock) ? 0 : -2;
 			inputs[18] = (suppliers_exist) ? 0 : -3;
-			;
 			inputs[19] = (inputs_on_stock) ? 0 : -4;
 			inputs[20] =
 			   (mines_per_type[bo.mines].in_construction + mines_per_type[bo.mines].finished == 1) ?
 
=== modified file 'src/ai/test/test_ai.cc'
--- src/ai/test/test_ai.cc	2019-08-31 20:12:56 +0000
+++ src/ai/test/test_ai.cc	2019-09-06 14:10:07 +0000
@@ -29,6 +29,7 @@
 
 // Triggered by BOOST_AUTO_TEST_CASE
 CLANG_DIAG_OFF("-Wdisabled-macro-expansion")
+CLANG_DIAG_OFF("-Wused-but-marked-unused")
 
 using namespace Widelands;
 
 
=== modified file 'src/economy/test/test_road.cc'
--- src/economy/test/test_road.cc	2019-02-23 11:00:49 +0000
+++ src/economy/test/test_road.cc	2019-09-06 14:10:07 +0000
@@ -33,6 +33,7 @@
 
 // Triggered by BOOST_AUTO_TEST_CASE
 CLANG_DIAG_OFF("-Wdisabled-macro-expansion")
+CLANG_DIAG_OFF("-Wused-but-marked-unused")
 
 namespace Widelands {
 class World;
 
=== modified file 'src/economy/test/test_routing.cc'
--- src/economy/test/test_routing.cc	2019-02-23 11:00:49 +0000
+++ src/economy/test/test_routing.cc	2019-09-06 14:10:07 +0000
@@ -33,6 +33,7 @@
 
 // Triggered by BOOST_AUTO_TEST_CASE
 CLANG_DIAG_OFF("-Wdisabled-macro-expansion")
+CLANG_DIAG_OFF("-Wused-but-marked-unused")
 
 using namespace Widelands;
 
 
=== modified file 'src/editor/CMakeLists.txt'
--- src/editor/CMakeLists.txt	2019-08-27 19:00:30 +0000
+++ src/editor/CMakeLists.txt	2019-09-06 14:10:07 +0000
@@ -115,7 +115,7 @@
     scripting_lua_interface
     scripting_lua_table
     ui_basic
-    widelands_ball_of_mud
+    widelands_options
     wui
     wui_common_mapdetails
     wui_mapview_pixelfunctions
 
=== modified file 'src/graphic/styles/font_style.h'
--- src/graphic/styles/font_style.h	2019-06-01 14:16:25 +0000
+++ src/graphic/styles/font_style.h	2019-09-06 14:10:07 +0000
@@ -67,7 +67,6 @@
 	                       bool init_underline,
 	                       bool init_shadow);
 	explicit FontStyleInfo(const FontStyleInfo& other);
-	FontStyleInfo& operator=(const FontStyleInfo& other) = default;
 
 	std::string as_font_tag(const std::string& text) const;
 
 
=== modified file 'src/graphic/styles/text_panel_style.h'
--- src/graphic/styles/text_panel_style.h	2019-05-26 17:21:15 +0000
+++ src/graphic/styles/text_panel_style.h	2019-09-06 14:10:07 +0000
@@ -38,7 +38,6 @@
 	   : background_(new UI::PanelStyleInfo(other.background())),
 	     font_(new UI::FontStyleInfo(other.font())) {
 	}
-	TextPanelStyleInfo& operator=(const TextPanelStyleInfo& other) = default;
 
 	const UI::FontStyleInfo& font() const {
 		return *font_.get();
 
=== modified file 'src/io/filesystem/test/test_filesystem.cc'
--- src/io/filesystem/test/test_filesystem.cc	2019-04-09 16:43:49 +0000
+++ src/io/filesystem/test/test_filesystem.cc	2019-09-06 14:10:07 +0000
@@ -49,6 +49,7 @@
 
 // Triggered by BOOST_AUTO_TEST_CASE
 CLANG_DIAG_OFF("-Wdisabled-macro-expansion")
+CLANG_DIAG_OFF("-Wused-but-marked-unused")
 
 BOOST_AUTO_TEST_SUITE(FileSystemTests)
 #ifndef _WIN32
 
=== modified file 'src/logic/CMakeLists.txt'
--- src/logic/CMakeLists.txt	2019-08-27 19:00:30 +0000
+++ src/logic/CMakeLists.txt	2019-09-06 14:10:07 +0000
@@ -63,7 +63,7 @@
     logic_commands
     logic_game_settings
     ui_basic
-    widelands_ball_of_mud
+    widelands_options
     wui
 )
 
@@ -258,7 +258,7 @@
     sound
     sound_constants
     ui_basic
-    widelands_ball_of_mud
+    widelands_options
     wui # TODO(GunChleoc): Circular dependency
 )
 
 
=== modified file 'src/logic/map_objects/tribes/productionsite.cc'
--- src/logic/map_objects/tribes/productionsite.cc	2019-08-20 17:35:33 +0000
+++ src/logic/map_objects/tribes/productionsite.cc	2019-09-06 14:10:07 +0000
@@ -1090,7 +1090,7 @@
 	static const uint32_t entire_duration = 10 * 60 * 1000;
 	if (duration > duration_cap) {
 		duration = duration_cap;
-	};
+	}
 	const uint32_t past_duration = entire_duration - duration;
 	crude_percent_ =
 	   (crude_percent_ * past_duration + produced * duration * 10000) / entire_duration;
 
=== modified file 'src/network/CMakeLists.txt'
--- src/network/CMakeLists.txt	2019-08-27 19:00:30 +0000
+++ src/network/CMakeLists.txt	2019-09-06 14:10:07 +0000
@@ -61,5 +61,6 @@
     ui_basic
     ui_fsmenu_gameloading
     widelands_ball_of_mud
+    widelands_options
     wui
 )
 
=== modified file 'src/notifications/test/notifications_test.cc'
--- src/notifications/test/notifications_test.cc	2019-02-23 11:00:49 +0000
+++ src/notifications/test/notifications_test.cc	2019-09-06 14:10:07 +0000
@@ -29,6 +29,7 @@
 
 // Triggered by BOOST_AUTO_TEST_CASE
 CLANG_DIAG_OFF("-Wdisabled-macro-expansion")
+CLANG_DIAG_OFF("-Wused-but-marked-unused")
 
 struct SimpleNote {
 	CAN_BE_SENT_AS_NOTE(100)
 
=== modified file 'src/scripting/CMakeLists.txt'
--- src/scripting/CMakeLists.txt	2019-08-27 19:00:30 +0000
+++ src/scripting/CMakeLists.txt	2019-09-06 14:10:07 +0000
@@ -129,7 +129,7 @@
     scripting_luna
     third_party_eris
     ui_basic
-    widelands_ball_of_mud
+    widelands_options
     wui
     wui_mapview_pixelfunctions
 )
 
=== modified file 'src/scripting/test/test_luna.cc'
--- src/scripting/test/test_luna.cc	2019-02-23 11:00:49 +0000
+++ src/scripting/test/test_luna.cc	2019-09-06 14:10:07 +0000
@@ -30,6 +30,7 @@
 
 // Triggered by BOOST_AUTO_TEST_CASE
 CLANG_DIAG_OFF("-Wdisabled-macro-expansion")
+CLANG_DIAG_OFF("-Wused-but-marked-unused")
 
 #ifndef BEGIN_LUNA_PROPERTIES
 #define BEGIN_LUNA_PROPERTIES(klass) const PropertyType<klass> klass::Properties[] = {
 
=== modified file 'src/sound/CMakeLists.txt'
--- src/sound/CMakeLists.txt	2019-08-27 19:00:30 +0000
+++ src/sound/CMakeLists.txt	2019-09-06 14:10:07 +0000
@@ -34,5 +34,5 @@
     logic_exceptions
     random
     sound_constants
-    widelands_ball_of_mud
+    widelands_options
 )
 
=== modified file 'src/ui_basic/CMakeLists.txt'
--- src/ui_basic/CMakeLists.txt	2019-08-27 19:00:30 +0000
+++ src/ui_basic/CMakeLists.txt	2019-09-06 14:10:07 +0000
@@ -76,4 +76,5 @@
     sound
     sound_constants
     widelands_ball_of_mud
+    widelands_options
 )
 
=== modified file 'src/ui_fsmenu/CMakeLists.txt'
--- src/ui_fsmenu/CMakeLists.txt	2019-08-27 19:00:30 +0000
+++ src/ui_fsmenu/CMakeLists.txt	2019-09-06 14:10:07 +0000
@@ -19,6 +19,7 @@
     ui_fsmenu_base
     sound
     widelands_ball_of_mud
+    widelands_options
     wui_sound_options
 )
 
@@ -81,7 +82,7 @@
     ui_basic
     ui_fsmenu_base
     ui_fsmenu_main_menu
-    widelands_ball_of_mud
+    widelands_options
     wui_common_mapdetails
     wui
 )
@@ -118,7 +119,7 @@
     ui_fsmenu_helpwindow
     ui_fsmenu_loading_common
     ui_fsmenu_maploading
-    widelands_ball_of_mud
+    widelands_options
     wui
     wui_chat_ui
     wui_common_gamedetails
@@ -177,7 +178,7 @@
     sound
     ui_basic
     ui_fsmenu_base
-    widelands_ball_of_mud
+    widelands_options
     wui_chat_ui
 )
 
 
=== modified file 'src/ui_fsmenu/options.cc'
--- src/ui_fsmenu/options.cc	2019-08-25 14:50:16 +0000
+++ src/ui_fsmenu/options.cc	2019-09-06 14:10:07 +0000
@@ -680,5 +680,5 @@
 	g_sh->save_config();
 
 	// Now write to file
-	write_config(WLApplication::get());
+	write_config();
 }
 
=== modified file 'src/wlapplication.cc'
--- src/wlapplication.cc	2019-09-05 20:04:47 +0000
+++ src/wlapplication.cc	2019-09-06 14:10:07 +0000
@@ -228,6 +228,9 @@
 
 // Set up the homedir. Exit 1 if the homedir is illegal or the logger couldn't be initialized for
 // Windows.
+// Also sets the config directory. This defaults to $XDG_CONFIG_HOME/widelands/config on Unix.
+// Defaults to homedir/config everywhere else, if homedir is set manually or if
+// built without XDG-support.
 void WLApplication::setup_homedir() {
 	// Check if we have a command line override
 	if (commandline_.count("homedir")) {
@@ -274,6 +277,12 @@
 		// Homedir is ready, so we can log normally from now on
 		log("Set home directory: %s\n", homedir_.c_str());
 	}
+
+#ifdef USE_XDG
+		set_config_directory(userconfigdir_);
+#else
+		set_config_directory(homedir_);
+#endif
 }
 
 WLApplication* WLApplication::the_singleton = nullptr;
@@ -766,7 +775,7 @@
 bool WLApplication::init_settings() {
 
 	// Read in the configuration file
-	read_config(this);
+	read_config();
 
 	// Then parse the commandline - overwrites conffile settings
 	handle_commandline_parameters();
@@ -846,7 +855,7 @@
 
 	// Save configuration now. Otherwise, the UUID is not saved
 	// when the game crashes, losing part of its advantage
-	write_config(this);
+	write_config();
 
 	return true;
 }
@@ -872,8 +881,7 @@
 void WLApplication::shutdown_settings() {
 	// To be proper, release our textdomain
 	i18n::release_textdomain();
-
-	write_config(this);
+	write_config();
 }
 
 void WLApplication::shutdown_hardware() {
 
=== modified file 'src/wlapplication.h'
--- src/wlapplication.h	2019-07-20 14:32:57 +0000
+++ src/wlapplication.h	2019-09-06 14:10:07 +0000
@@ -196,13 +196,6 @@
 	void replay();
 	static void emergency_save(Widelands::Game&);
 
-#ifdef USE_XDG
-	// this is only for src/wlapplication_options.cc
-	std::string get_userconfigdir() {
-		return userconfigdir_;
-	}
-#endif
-
 private:
 	WLApplication(int argc, char const* const* argv);
 
 
=== modified file 'src/wlapplication_options.cc'
--- src/wlapplication_options.cc	2019-08-31 03:22:10 +0000
+++ src/wlapplication_options.cc	2019-09-06 14:10:07 +0000
@@ -19,14 +19,17 @@
 
 #include "wlapplication_options.h"
 
+#include <cassert>
+#include <memory>
+
 #include "base/log.h"
 #include "io/filesystem/disk_filesystem.h"
-#include "io/profile.h"
 #include "logic/filesystem_constants.h"
-#include "wlapplication.h"
 
 static Profile g_options(Profile::err_log);
 
+static std::unique_ptr<FileSystem> config_dir = nullptr;
+
 void check_config_used() {
 	g_options.check_used();
 }
@@ -110,26 +113,22 @@
 	g_options.pull_section(section.c_str()).set_string(name.c_str(), value.c_str());
 }
 
-void read_config(WLApplication* wlapplication) {
-#ifdef USE_XDG
-	RealFSImpl dir(wlapplication->get_userconfigdir());
-	dir.ensure_directory_exists(".");
-	log("Set configuration file: %s/%s\n", wlapplication->get_userconfigdir().c_str(),
-	    kConfigFile.c_str());
-	g_options.read(kConfigFile.c_str(), "global", dir);
-#else
-	g_options.read(kConfigFile.c_str(), "global");
-#endif
-}
-
-void write_config(WLApplication* wlapplication) {
+
+void set_config_directory(const std::string& userconfigdir) {
+	config_dir.reset(new RealFSImpl(userconfigdir));
+	config_dir->ensure_directory_exists(".");
+	log("Set configuration file: %s/%s\n", userconfigdir.c_str(), kConfigFile.c_str());
+}
+
+void read_config() {
+	assert(config_dir != nullptr);
+	g_options.read(kConfigFile.c_str(), "global", *config_dir);
+}
+
+void write_config() {
+	assert(config_dir != nullptr);
 	try {  //  overwrite the old config file
-#ifdef USE_XDG
-		RealFSImpl dir(wlapplication->get_userconfigdir());
-		g_options.write(kConfigFile.c_str(), true, dir);
-#else
-		g_options.write(kConfigFile.c_str(), true);
-#endif
+		g_options.write(kConfigFile.c_str(), true, *config_dir);
 	} catch (const std::exception& e) {
 		log("WARNING: could not save configuration: %s\n", e.what());
 	} catch (...) {
 
=== modified file 'src/wlapplication_options.h'
--- src/wlapplication_options.h	2019-08-28 18:34:58 +0000
+++ src/wlapplication_options.h	2019-09-06 14:10:07 +0000
@@ -21,7 +21,6 @@
 #define WL_WLAPPLICATION_OPTIONS_H
 
 #include "io/profile.h"
-#include "wlapplication.h"
 
 /*
  * Further explanations for all functions and its return values
@@ -77,31 +76,20 @@
                        const std::string& value);
 
 /*
+ * Sets the directory where to read/write kConfigFile.
+ */
+void set_config_directory(const std::string& userconfigdir);
+
+/*
  * Reads the configuration from kConfigFile.
- * Defaults to $XDG_CONFIG_HOME/widelands/config on Unix.
- * Defaults to homedir/config everywhere else, if homedir is set manually or if
- * built without XDG-support.
- *
- * This function needs access to the WLApplication object to distinguish
- * between multiple possible states.
- * While we could use WLApplication::get() this would get us in an infinite loop
- * because this function will be called in the constructor of said object and
- * WLApplication::get() spawns another object if there isn't one yet.
+ * Assumes that set_config_directory has been called.
  */
-void read_config(WLApplication*);
+void read_config();
 
 /*
  * Writes the configuration to kConfigFile.
- * Defaults to $XDG_CONFIG_HOME/widelands/config on Unix.
- * Defaults to homedir/config everywhere else, if homedir is set manually or if
- * built without XDG-support.
- *
- * This function needs access to the WLApplication object to distinguish
- * between multiple possible states.
- * While we could use WLApplication::get() this would get us in an infinite loop
- * because this function will be called in the constructor of said object and
- * WLApplication::get() spawns another object if there isn't one yet.
+ * * Assumes that set_config_directory has been called.
  */
-void write_config(WLApplication*);
+void write_config();
 
 #endif  // end of include guard: WL_WLAPPLICATION_OPTIONS_H
 
=== modified file 'src/wui/CMakeLists.txt'
--- src/wui/CMakeLists.txt	2019-08-27 19:00:30 +0000
+++ src/wui/CMakeLists.txt	2019-09-06 14:10:07 +0000
@@ -33,7 +33,7 @@
     logic
     sound
     ui_basic
-    widelands_ball_of_mud
+    widelands_options
     wui
 )
 
@@ -154,6 +154,7 @@
     logic_widelands_geometry
     ui_basic
     widelands_ball_of_mud
+    widelands_options
     wui_mapview_pixelfunctions
 )
 
@@ -314,7 +315,7 @@
     scripting_lua_table
     sound
     ui_basic
-    widelands_ball_of_mud
+    widelands_options
     wui_chat_ui
     wui_common_gamedetails
     wui_economy_options