widelands-dev team mailing list archive

[Peter Waller <peter.waller@xxxxxxxxx>]

Peter Waller has proposed merging lp:~peter.waller/widelands/fix-unused-vars into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #913369 in widelands: "Warnings at compile-time (clang/llvm)"
  https://bugs.launchpad.net/widelands/+bug/913369

For more details, see:
https://code.launchpad.net/~peter.waller/widelands/fix-unused-vars/+merge/109010

I have fixed the unused variables which I'm convinced are not bugs.

I've also introduced an `is_a(Type, ptr)` macro in upcast.h to complement upcast() which does not have the side-effect of creating a new identifier in the current scope.

I did not touch the one unused variable which I thought could be a bug.
-- 
https://code.launchpad.net/~peter.waller/widelands/fix-unused-vars/+merge/109010
Your team Widelands Developers is requested to review the proposed merge of lp:~peter.waller/widelands/fix-unused-vars into lp:widelands.

=== modified file 'src/economy/flag.cc'
--- src/economy/flag.cc	2012-06-06 17:33:36 +0000
+++ src/economy/flag.cc	2012-06-06 18:11:20 +0000
@@ -317,7 +317,11 @@
 uint8_t Flag::nr_of_roads() const {
 	uint8_t counter = 0;
 	for (uint8_t road_id = 6; road_id; --road_id)
+<<<<<<< TREE
 		if (get_road(road_id) != NULL)
+=======
+		if (/* Road * const road = */ get_road(road_id))
+>>>>>>> MERGE-SOURCE
 			++counter;
 	return counter;
 }

=== modified file 'src/logic/editor_game_base.cc'
--- src/logic/editor_game_base.cc	2012-03-07 10:29:19 +0000
+++ src/logic/editor_game_base.cc	2012-06-06 18:11:20 +0000
@@ -608,7 +608,7 @@
 	//  covered.
 	// SirVer, TODO: In the editor, no buildings should burn down when a military
 	// building is removed. Check this again though
-	if (upcast(Game, game, this)) {
+	if (is_a(Game, this)) {
 		++player_area.radius;
 		player_area.player_number = destroying_player;
 		cleanup_playerimmovables_area(player_area);

=== modified file 'src/logic/game.cc'
--- src/logic/game.cc	2012-04-06 19:26:17 +0000
+++ src/logic/game.cc	2012-06-06 18:11:20 +0000
@@ -480,7 +480,7 @@
 			}
 		} else
 			// Is a scenario!
-			iterate_players_existing(p, nr_players, *this, plr)
+			iterate_players_existing_novar(p, nr_players, *this)
 				if (not map().get_starting_pos(p))
 				throw warning
 					(_("Missing starting position"),

=== modified file 'src/logic/soldier.cc'
--- src/logic/soldier.cc	2012-05-06 16:28:43 +0000
+++ src/logic/soldier.cc	2012-06-06 18:11:20 +0000
@@ -1363,7 +1363,8 @@
 	Map & map = game.map();
 	Soldier & opponent = *m_battle->opponent(*this);
 	if (opponent.get_position() != get_position()) {
-		if (upcast(Building, building, map[get_position()].get_immovable())) {
+		if (is_a(Building, map[get_position()].get_immovable()))
+		{
 			// Note that this does not use the "leavebuilding" task,
 			// because that task is geared towards orderly workers leaving
 			// their location, whereas this case can also happen when

=== modified file 'src/logic/warehouse.cc'
--- src/logic/warehouse.cc	2012-04-06 19:26:17 +0000
+++ src/logic/warehouse.cc	2012-06-06 18:11:20 +0000
@@ -594,8 +594,8 @@
 			clist.pop_back();
 
 			if (upcast(Game, game, &egbase))
-				if (egbase.objects().object_still_available(w))
-					w->reset_tasks(ref_cast<Game, Editor_Game_Base>(egbase));
+				if (game->objects().object_still_available(w))
+					w->reset_tasks(*game);
 		}
 	}
 	m_incorporated_workers.clear();

=== modified file 'src/map_io/widelands_map_building_data_packet.cc'
--- src/map_io/widelands_map_building_data_packet.cc	2012-02-15 21:25:34 +0000
+++ src/map_io/widelands_map_building_data_packet.cc	2012-06-06 18:11:20 +0000
@@ -146,7 +146,7 @@
 			if (!pfb)
 				fw.Unsigned8(0);
 			else {
-				if (upcast(const ConstructionSite, cs, pfb))
+				if (is_a(ConstructionSite, pfb))
 					fw.Unsigned8(1);
 				else // DismantleSite
 					fw.Unsigned8(2);

=== modified file 'src/network/nethost.cc'
--- src/network/nethost.cc	2012-05-02 06:42:13 +0000
+++ src/network/nethost.cc	2012-06-06 18:11:20 +0000
@@ -671,7 +671,7 @@
 void NetHost::initComputerPlayers()
 {
 	const Widelands::Player_Number nr_players = d->game->map().get_nrplayers();
-	iterate_players_existing(p, nr_players, *d->game, plr) {
+	iterate_players_existing_novar(p, nr_players, *d->game) {
 		if (p == d->settings.playernum + 1)
 			continue;
 

=== modified file 'src/upcast.h'
--- src/upcast.h	2012-02-15 21:25:34 +0000
+++ src/upcast.h	2012-06-06 18:11:20 +0000
@@ -19,3 +19,8 @@
 
 #define upcast(type, identifier, source) type * const identifier = \
 dynamic_cast<type *>(source)
+
+// Useful when you want to know if [typeid(source) == typeof(type)*], without
+// the side-effect upcast has of creating a new identifier which won't be used.
+#define is_a(type, source) \
+(dynamic_cast<const type *>(source) != NULL)

=== modified file 'src/wui/general_statistics_menu.cc'
--- src/wui/general_statistics_menu.cc	2012-02-15 21:25:34 +0000
+++ src/wui/general_statistics_menu.cc	2012-06-06 18:11:20 +0000
@@ -139,7 +139,7 @@
 
 	uint32_t plr_in_game = 0;
 	Player_Number const nr_players = game.map().get_nrplayers();
-	iterate_players_existing_const(p, nr_players, game, player) ++plr_in_game;
+	iterate_players_existing_novar(p, nr_players, game) ++plr_in_game;
 
 	iterate_players_existing_const(p, nr_players, game, player) {
 		char buffer[36];
@@ -287,7 +287,7 @@
 		m_my_registry->selected_information = m_selected_information;
 		m_my_registry->time = m_plot.get_time();
 		Player_Number const nr_players = game.map().get_nrplayers();
-		iterate_players_existing_const(p, nr_players, game, player) {
+		iterate_players_existing_novar(p, nr_players, game) {
 			m_my_registry->selected_players[p - 1] = m_cbs[p - 1]->get_perm_pressed();
 		}
 	}