Merge lp:~widelands-dev/widelands/ai_remove_iterators into lp:widelands

[GunChleoc]

This isn't quite ready yet, but I wanted to get some feedback before I finish.

I have replaced all easy iterator loops with ranged-based for loops. The remaining ones have erase operations in them for objects that don't have a comparator yet. In order for them to work, I will have to add them to the ai_help_structs.h like this: http://bazaar.launchpad.net/~widelands-dev/widelands/ai_remove_iterators/revision/7241

I think this makes the AI code more readable, but if you think this is overkill, I will leave it.

[SirVer (sirver)]

I think this is a definitive improvement to the readability. I did not carefully review this yet though, I think the seafaring branch should be merged first.

[GunChleoc (gunchleoc)]

Definitely, I just wanted an opinion before I continue :)

=== modified file 'src/ai/ai_help_structs.h'
--- src/ai/ai_help_structs.h	2014-12-16 11:37:49 +0000
+++ src/ai/ai_help_structs.h	2014-12-16 11:37:49 +0000
@@ -189,6 +189,14 @@
 	Widelands::FCoords coords;
 	int32_t blocked_until_;
 
+	bool operator== (const BlockedField& other) const {
+		return coords == other.coords;
+	}
+
+	bool operator!= (const BlockedField& other) const {
+		return coords != other.coords;
+	}
+
 	BlockedField(Widelands::FCoords c, int32_t until) : coords(c), blocked_until_(until) {
 	}
 };
@@ -387,6 +395,15 @@
 };
 
 struct ShipObserver {
+
+	bool operator== (const ShipObserver& other) const {
+		return ship->serial() == other.ship->serial();
+	}
+
+	bool operator!= (const ShipObserver& other) const {
+		return ship->serial() != other.ship->serial();
+	}
+
 	Widelands::Ship* ship;
 	Widelands::Coords expedition_start_point_;
 	std::unordered_set<uint32_t> visited_spots_;

=== modified file 'src/ai/defaultai.cc'
--- src/ai/defaultai.cc	2014-12-16 11:37:49 +0000
+++ src/ai/defaultai.cc	2014-12-16 11:37:49 +0000
@@ -181,19 +181,17 @@
 			   }
 				break;
 
-			case NoteShipMessage::Message::kLost:
-			   for (std::list<ShipObserver>::iterator i = allships.begin(); i != allships.end(); ++i) {
-					if (i->ship == note.ship) {
-					   allships.erase(i);
+		   case NoteShipMessage::Message::kLost: {
+				for (const ShipObserver& ship_observer : allships) {
+					if (ship_observer.ship == note.ship) {
+						allships.remove(ship_observer);
 					   break;
-					}
-			   }
-			   break;
-
-			case NoteShipMessage::Message::kWaitingForCommand:
-			   for (std::list<ShipObserver>::iterator i = allships.begin(); i != allships.end(); ++i) {
-					if (i->ship == note.ship) {
-					   i->waiting_for_command_ = true;
+				   }
+				}
+		   case NoteShipMessage::Message::kWaitingForCommand:
+				for (ShipObserver& ship_observer : allships) {
+					if (ship_observer.ship == note.ship) {
+						ship_observer.waiting_for_command_ = true;
 					   break;
 				   }
 			   }
@@ -201,7 +199,8 @@
 		   default:
 				;
 		   }
-		});
+		}
+	});
 }
 
 DefaultAI::~DefaultAI() {
@@ -1032,10 +1031,8 @@
 	for (int32_t i = 0; i < 4; ++i)
 		spots_avail.at(i) = 0;
 
-	for (std::list<BuildableField*>::iterator i = buildable_fields.begin();
-	     i != buildable_fields.end();
-	     ++i)
-		++spots_avail.at((*i)->coords.field->nodecaps() & BUILDCAPS_SIZEMASK);
+	for (const BuildableField* buildable_field : buildable_fields)
+		++spots_avail.at(buildable_field->coords.field->nodecaps() & BUILDCAPS_SIZEMASK);
 
 	spots_ = spots_avail.at(BUILDCAPS_SMALL);
 	spots_ += spots_avail.at(BUILDCAPS_MEDIUM);
@@ -1125,12 +1122,11 @@
 	Coords proposed_coords;
 
 	// Remove outdated fields from blocker list
-	for (std::list<BlockedField>::iterator i = blocked_fields.begin(); i != blocked_fields.end();)
-		if (i->blocked_until_ < game().get_gametime()) {
-			i = blocked_fields.erase(i);
-		} else {
-			++i;
+	for (const BlockedField& blocked_field : blocked_fields) {
+		if (blocked_field.blocked_until_ < game().get_gametime()) {
+			blocked_fields.remove(blocked_field);
 		}
+	}
 
 	// testing big military buildings, whether critical construction
 	// material is available (at least in amount of
@@ -1169,21 +1165,17 @@
 	int16_t max_needed_preciousness = 0;  // preciousness_ of most precious NEEDED output
 
 	// first scan all buildable fields for regular buildings
-	for (std::list<BuildableField*>::iterator i = buildable_fields.begin();
-	     i != buildable_fields.end();
-	     ++i) {
-		BuildableField* const bf = *i;
+	for (const BuildableField* buildable_field : buildable_fields) {
 
-		if (bf->next_update_due_ < gametime - 8000) {
+		if (buildable_field->next_update_due_ < gametime - 8000) {
 			continue;
 		}
 
 		// Continue if field is blocked at the moment
 		field_blocked = false;
 
-		for (std::list<BlockedField>::iterator j = blocked_fields.begin(); j != blocked_fields.end();
-		     ++j) {
-			if (j->coords == bf->coords) {
+		for (const BlockedField& blocked_field : blocked_fields) {
+			if (blocked_field.coords == buildable_field->coords) {
 				field_blocked = true;
 			}
 		}
@@ -1194,7 +1186,7 @@
 		}
 
 		assert(player_);
-		int32_t const maxsize = player_->get_buildcaps(bf->coords) & BUILDCAPS_SIZEMASK;
+		int32_t const maxsize = player_->get_buildcaps(buildable_field->coords) & BUILDCAPS_SIZEMASK;
 
 		// For every field test all buildings
 		for (uint32_t j = 0; j < buildings_.size(); ++j) {
@@ -1215,7 +1207,7 @@
 
 			// testing for reserved ports
 			if (!bo.is_port_) {
-				if (port_reserved_coords.count(coords_hash(bf->coords)) > 0) {
+				if (port_reserved_coords.count(coords_hash(buildable_field->coords)) > 0) {
 					continue;
 				}
 			}
@@ -1251,13 +1243,13 @@
 			if (bo.type == BuildingObserver::PRODUCTIONSITE) {
 
 				// exclude spots on border
-				if (bf->near_border_ && !bo.need_trees_ && !bo.need_stones_ && !bo.is_fisher_) {
+				if (buildable_field->near_border_ && !bo.need_trees_ && !bo.need_stones_ && !bo.is_fisher_) {
 					continue;
 				}
 
 				// this can be only a well (as by now)
 				if (bo.mines_water_) {
-					if (bf->ground_water_ < 2) {
+					if (buildable_field->ground_water_ < 2) {
 						continue;
 					}
 
@@ -1283,8 +1275,8 @@
 					if (bo.stocklevel_ > 50 + productionsites.size() * 5) {
 						continue;
 					}
-					prio += bf->ground_water_ - 2;
-					prio = recalc_with_border_range(*bf, prio);
+					prio += buildable_field->ground_water_ - 2;
+					prio = recalc_with_border_range(*buildable_field, prio);
 
 				} else if (bo.need_trees_) {  // LUMBERJACS
 
@@ -1292,14 +1284,14 @@
 					   3 + static_cast<int32_t>(mines_.size() + productionsites.size()) / 15;
 
 					if (bo.total_count() == 0) {
-						prio = 500 + bf->trees_nearby_;
+						prio = 500 + buildable_field->trees_nearby_;
 					}
 
 					else if (bo.total_count() == 1) {
-						prio = 400 + bf->trees_nearby_;
+						prio = 400 + buildable_field->trees_nearby_;
 					}
 
-					else if (bf->trees_nearby_ < 2) {
+					else if (buildable_field->trees_nearby_ < 2) {
 						continue;
 					}
 
@@ -1311,15 +1303,15 @@
 							prio = 0;
 						}
 
-						if (bf->producers_nearby_.at(bo.outputs_.at(0)) > 1) {
+						if (buildable_field->producers_nearby_.at(bo.outputs_.at(0)) > 1) {
 							continue;
 						}
 
-						prio += 2 * bf->trees_nearby_ - 10 -
-						        bf->producers_nearby_.at(bo.outputs_.at(0)) * 5 -
+						prio += 2 * buildable_field->trees_nearby_ - 10 -
+								  buildable_field->producers_nearby_.at(bo.outputs_.at(0)) * 5 -
 						        new_buildings_stop_ * 15;
 
-						if (bf->near_border_) {
+						if (buildable_field->near_border_) {
 							prio = prio / 2;
 						}
 					}
@@ -1332,7 +1324,7 @@
 					if (bo.cnt_under_construction_ > 0) {
 						continue;
 					}
-					prio = bf->stones_nearby_;
+					prio = buildable_field->stones_nearby_;
 
 					if (prio <= 0) {
 						continue;
@@ -1352,14 +1344,14 @@
 					}
 
 					// to prevent to many quaries on one spot
-					prio = prio - 50 * bf->producers_nearby_.at(bo.outputs_.at(0));
+					prio = prio - 50 * buildable_field->producers_nearby_.at(bo.outputs_.at(0));
 
-					if (bf->near_border_) {
+					if (buildable_field->near_border_) {
 						prio = prio / 2;
 					}
 
 				} else if (bo.is_hunter_) {
-					if (bf->critters_nearby_ < 5) {
+					if (buildable_field->critters_nearby_ < 5) {
 						continue;
 					}
 
@@ -1368,7 +1360,7 @@
 					}
 
 					prio +=
-					   (bf->critters_nearby_ * 2) - 8 - 5 * bf->producers_nearby_.at(bo.outputs_.at(0));
+						(buildable_field->critters_nearby_ * 2) - 8 - 5 * buildable_field->producers_nearby_.at(bo.outputs_.at(0));
 
 				} else if (bo.is_fisher_) {  // fisher
 
@@ -1381,7 +1373,7 @@
 						continue;
 					}
 
-					if (bf->water_nearby_ < 2) {
+					if (buildable_field->water_nearby_ < 2) {
 						continue;
 					}
 
@@ -1400,11 +1392,11 @@
 						continue;
 					}
 
-					if (bf->producers_nearby_.at(bo.outputs_.at(0)) >= 1) {
+					if (buildable_field->producers_nearby_.at(bo.outputs_.at(0)) >= 1) {
 						continue;
 					}
 
-					prio = bf->fish_nearby_ - new_buildings_stop_ * 15 * bo.total_count();
+					prio = buildable_field->fish_nearby_ - new_buildings_stop_ * 15 * bo.total_count();
 
 				} else if (bo.production_hint_ >= 0) {
 					// first setting targets (needed also for dismantling)
@@ -1423,7 +1415,7 @@
 					if (bo.plants_trees_) {  // RANGERS
 
 						// if there are too many trees nearby
-						if (bf->trees_nearby_ > 25 && bo.total_count() >= 1) {
+						if (buildable_field->trees_nearby_ > 25 && bo.total_count() >= 1) {
 							continue;
 						}
 
@@ -1451,12 +1443,12 @@
 						}
 
 						// considering near trees and producers
-						prio += (30 - bf->trees_nearby_) * 2 +
-						        bf->producers_nearby_.at(bo.production_hint_) * 5 -
+						prio += (30 - buildable_field->trees_nearby_) * 2 +
+								  buildable_field->producers_nearby_.at(bo.production_hint_) * 5 -
 						        new_buildings_stop_ * 15;
 
 						// considering space consumers nearby
-						prio -= bf->space_consumers_nearby_ * 5;
+						prio -= buildable_field->space_consumers_nearby_ * 5;
 
 					} else {  // FISH BREEDERS and GAME KEEPERS
 						if (new_buildings_stop_ && bo.total_count() > 0) {
@@ -1464,11 +1456,11 @@
 						}
 
 						// especially for fish breeders
-						if (bo.need_water_ && bf->water_nearby_ < 2) {
+						if (bo.need_water_ && buildable_field->water_nearby_ < 2) {
 							continue;
 						}
 						if (bo.need_water_) {
-							prio += bf->water_nearby_ / 5;
+							prio += buildable_field->water_nearby_ / 5;
 						}
 
 						if (bo.total_count() > bo.cnt_target_) {
@@ -1485,14 +1477,14 @@
 						}
 
 						if (bo.total_count() == 0 && gametime > 45 * 1000) {
-							prio += 100 + bf->producers_nearby_.at(bo.production_hint_) * 10;
-						} else if (bf->producers_nearby_.at(bo.production_hint_) == 0) {
+							prio += 100 + buildable_field->producers_nearby_.at(bo.production_hint_) * 10;
+						} else if (buildable_field->producers_nearby_.at(bo.production_hint_) == 0) {
 							continue;
 						} else {
-							prio += bf->producers_nearby_.at(bo.production_hint_) * 10;
+							prio += buildable_field->producers_nearby_.at(bo.production_hint_) * 10;
 						}
 
-						if (bf->enemy_nearby_) {
+						if (buildable_field->enemy_nearby_) {
 							prio -= 10;
 						}
 					}
@@ -1536,34 +1528,34 @@
 						prio += max_needed_preciousness + kDefaultPrioBoost;
 
 						if (bo.space_consumer_) {  // need to consider trees nearby
-							prio += 20 - (bf->trees_nearby_ / 3);
+							prio += 20 - (buildable_field->trees_nearby_ / 3);
 						}
 
 						// we attempt to cluster space consumers together
 						if (bo.space_consumer_) {  // need to consider trees nearby
-							prio += bf->space_consumers_nearby_ * 2;
+							prio += buildable_field->space_consumers_nearby_ * 2;
 						}
 
-						if (bo.space_consumer_ && !bf->water_nearby_) {  // not close to water
+						if (bo.space_consumer_ && !buildable_field->water_nearby_) {  // not close to water
 							prio += 1;
 						}
 
 						if (bo.space_consumer_ &&
-						    !bf->unowned_mines_pots_nearby_) {  // not close to mountains
+							 !buildable_field->unowned_mines_pots_nearby_) {  // not close to mountains
 							prio += 1;
 						}
 
 						if (!bo.space_consumer_) {
-							prio -= bf->producers_nearby_.at(bo.outputs_.at(0)) * 20;
+							prio -= buildable_field->producers_nearby_.at(bo.outputs_.at(0)) * 20;
 						}  // leave some free space between them
 
-						prio -= bf->space_consumers_nearby_ * 3;
+						prio -= buildable_field->space_consumers_nearby_ * 3;
 					}
 
 					else if (bo.is_shipyard_) {
 						// for now AI builds only one shipyard
-						if (bf->water_nearby_ > 3 && bo.total_count() == 0 && seafaring_economy) {
-							prio += kDefaultPrioBoost + productionsites.size() * 5 + bf->water_nearby_;
+						if (buildable_field->water_nearby_ > 3 && bo.total_count() == 0 && seafaring_economy) {
+							prio += kDefaultPrioBoost + productionsites.size() * 5 + buildable_field->water_nearby_;
 						}
 
 					} else if (!bo.inputs_.empty()) {
@@ -1584,7 +1576,7 @@
 					consumers_nearby_count = 0;
 
 					for (size_t k = 0; k < bo.outputs_.size(); ++k)
-						consumers_nearby_count += bf->consumers_nearby_.at(bo.outputs_.at(k));
+						consumers_nearby_count += buildable_field->consumers_nearby_.at(bo.outputs_.at(k));
 
 					if (consumers_nearby_count > 0) {
 						prio += 1;
@@ -1595,11 +1587,11 @@
 
 				// we allow 1 exemption from big buildings prohibition
 				if (bo.build_material_shortage_ &&
-				    (bo.cnt_under_construction_ > 0 || !(bf->enemy_nearby_))) {
+					 (bo.cnt_under_construction_ > 0 || !(buildable_field->enemy_nearby_))) {
 					continue;
 				}
 
-				if (!bf->unowned_land_nearby_) {
+				if (!buildable_field->unowned_land_nearby_) {
 					continue;
 				}
 
@@ -1611,21 +1603,21 @@
 					continue;
 				}
 
-				if (expansion_mode == kDefenseOnly && !bf->enemy_nearby_) {
+				if (expansion_mode == kDefenseOnly && !buildable_field->enemy_nearby_) {
 					continue;
 				}
 
-				if (bf->enemy_nearby_ && bo.fighting_type_) {
+				if (buildable_field->enemy_nearby_ && bo.fighting_type_) {
 					;
 				}  // it is ok, go on
-				else if (bf->unowned_mines_pots_nearby_ > 2 &&
+				else if (buildable_field->unowned_mines_pots_nearby_ > 2 &&
 				         (bo.mountain_conqueror_ || bo.expansion_type_)) {
 					;
 				}  // it is ok, go on
-				else if (bf->unowned_land_nearby_ && bo.expansion_type_ &&
+				else if (buildable_field->unowned_land_nearby_ && bo.expansion_type_ &&
 				         num_milit_constructionsites <= 1) {
 					;  // we allow big buildings now
-				} else if (bf->unowned_land_nearby_ &&
+				} else if (buildable_field->unowned_land_nearby_ &&
 				           bo.expansion_type_) {  // decreasing probability for big buidlings
 					if (bo.desc->get_size() == 2 && gametime % 15 >= 1) {
 						continue;
@@ -1640,8 +1632,8 @@
 				}  // the building is not suitable for situation
 
 				// not to build so many military buildings nearby
-				if (!bf->enemy_nearby_ &&
-				    (bf->military_in_constr_nearby_ + bf->military_unstationed_) > 0) {
+				if (!buildable_field->enemy_nearby_ &&
+					 (buildable_field->military_in_constr_nearby_ + buildable_field->military_unstationed_) > 0) {
 					continue;
 				}
 
@@ -1651,14 +1643,14 @@
 					local_boost = 200;
 				}
 
-				prio = (bf->unowned_land_nearby_ * 2 * resource_necessity_territory_ / 255 +
-				        bf->unowned_mines_pots_nearby_ * resource_necessity_mines_ / 255 +
-				        bf->stones_nearby_ / 2 + bf->military_loneliness_ / 10 - 60 + local_boost +
-				        bf->water_nearby_ * resource_necessity_water_ / 255);
+				prio = (buildable_field->unowned_land_nearby_ * 2 * resource_necessity_territory_ / 255 +
+						  buildable_field->unowned_mines_pots_nearby_ * resource_necessity_mines_ / 255 +
+						  buildable_field->stones_nearby_ / 2 + buildable_field->military_loneliness_ / 10 - 60 + local_boost +
+						  buildable_field->water_nearby_ * resource_necessity_water_ / 255);
 
 				// special bonus due to remote water for atlanteans
 				if (resource_necessity_water_needed_)
-					prio += bf->distant_water_ * resource_necessity_water_ / 255;
+					prio += buildable_field->distant_water_ * resource_necessity_water_ / 255;
 
 				if (bo.desc->get_size() < maxsize) {
 					prio = prio - 5;
@@ -1671,22 +1663,22 @@
 				// for expansion)
 				const int16_t bottom_treshold =
 				   15 - ((virtual_mines <= 4) ? (5 - virtual_mines) * 2 : 0);
-				if (bf->enemy_nearby_ && bf->military_capacity_ < bottom_treshold) {
-					prio += 50 + (bottom_treshold - bf->military_capacity_) * 20;
+				if (buildable_field->enemy_nearby_ && buildable_field->military_capacity_ < bottom_treshold) {
+					prio += 50 + (bottom_treshold - buildable_field->military_capacity_) * 20;
 				}
 
-				if (bf->enemy_nearby_ && bf->military_capacity_ > bottom_treshold + 4) {
-					prio -= (bf->military_capacity_ - (bottom_treshold + 4)) * 5;
+				if (buildable_field->enemy_nearby_ && buildable_field->military_capacity_ > bottom_treshold + 4) {
+					prio -= (buildable_field->military_capacity_ - (bottom_treshold + 4)) * 5;
 				}
 
 			} else if (bo.type == BuildingObserver::WAREHOUSE) {
 
 				// exclude spots on border
-				if (bf->near_border_ && !bo.is_port_) {
+				if (buildable_field->near_border_ && !bo.is_port_) {
 					continue;
 				}
 
-				if (!bf->is_portspace_ && bo.is_port_) {
+				if (!buildable_field->is_portspace_ && bo.is_port_) {
 					continue;
 				}
 
@@ -1714,7 +1706,7 @@
 
 				// special boost for first port
 				if (bo.is_port_ && bo.total_count() == 0 && productionsites.size() > 5 &&
-				    !bf->enemy_nearby_ && bf->is_portspace_ && seafaring_economy) {
+					 !buildable_field->enemy_nearby_ && buildable_field->is_portspace_ && seafaring_economy) {
 					prio += kDefaultPrioBoost + productionsites.size();
 					warehouse_needed = true;
 				}
@@ -1728,7 +1720,7 @@
 				uint16_t nearest_distance = std::numeric_limits<uint16_t>::max();
 				for (const WarehouseSiteObserver& wh_obs : warehousesites) {
 					const uint16_t actual_distance =
-					   map.calc_distance(bf->coords, wh_obs.site->get_position());
+						map.calc_distance(buildable_field->coords, wh_obs.site->get_position());
 					nearest_distance = std::min(nearest_distance, actual_distance);
 				}
 				//but limit to 15
@@ -1737,11 +1729,11 @@
 				prio += nearest_distance;
 
 				// take care about and enemies
-				if (bf->enemy_nearby_) {
+				if (buildable_field->enemy_nearby_) {
 					prio /= 2;
 				}
 
-				if (bf->unowned_land_nearby_ && !bo.is_port_) {
+				if (buildable_field->unowned_land_nearby_ && !bo.is_port_) {
 					prio /= 2;
 				}
 
@@ -1752,7 +1744,7 @@
 				}
 
 				// exclude spots on border
-				if (bf->near_border_) {
+				if (buildable_field->near_border_) {
 					continue;
 				}
 
@@ -1767,17 +1759,17 @@
 				}
 
 				// take care about borders and enemies
-				if (bf->enemy_nearby_) {
+				if (buildable_field->enemy_nearby_) {
 					prio /= 2;
 				}
 
-				if (bf->unowned_land_nearby_) {
+				if (buildable_field->unowned_land_nearby_) {
 					prio /= 2;
 				}
 			}
 
 			// think of space consuming buildings nearby like farms or vineyards
-			prio -= bf->space_consumers_nearby_ * 10;
+			prio -= buildable_field->space_consumers_nearby_ * 10;
 
 			// Stop here, if priority is 0 or less.
 			if (prio <= 0) {
@@ -1786,25 +1778,25 @@
 
 			// testing also vicinity
 			if (!bo.is_port_) {
-				if (port_reserved_coords.count(coords_hash(bf->coords)) > 0) {
+				if (port_reserved_coords.count(coords_hash(buildable_field->coords)) > 0) {
 					continue;
 				}
 			}
 
 			// Prefer road side fields
-			prio += bf->preferred_ ? 1 : 0;
+			prio += buildable_field->preferred_ ? 1 : 0;
 			// don't waste good land for small huts
 			prio -= (maxsize - bo.desc->get_size()) * 5;
 
 			// prefer vicinity of ports (with exemption of warehouses)
-			if (bf->port_nearby_ && bo.type == BuildingObserver::MILITARYSITE) {
+			if (buildable_field->port_nearby_ && bo.type == BuildingObserver::MILITARYSITE) {
 				prio *= 2;
 			}
 
 			if (prio > proposed_priority) {
 				best_building = &bo;
 				proposed_priority = prio;
-				proposed_coords = bf->coords;
+				proposed_coords = buildable_field->coords;
 			}
 		}  // ending loop over buildings
 	}     // ending loop over fields
@@ -1862,18 +1854,16 @@
 				}
 
 				// iterating over fields
-				for (std::list<MineableField*>::iterator j = mineable_fields.begin();
-				     j != mineable_fields.end();
-				     ++j) {
+				for (const MineableField* mineable_field : mineable_fields) {
 
-					if ((*j)->coords.field->get_resources() != bo.mines_) {
+					if (mineable_field->coords.field->get_resources() != bo.mines_) {
 						continue;
 					}
 
-					int32_t prio = (*j)->coords.field->get_resources_amount();
+					int32_t prio = mineable_field->coords.field->get_resources_amount();
 
 					// applying nearnes penalty
-					prio = prio - (*j)->mines_nearby_ * nearness_penalty;
+					prio = prio - mineable_field->mines_nearby_ * nearness_penalty;
 
 					// Only build mines_ on locations where some material can be mined
 					if (prio < 2) {
@@ -1883,10 +1873,8 @@
 					// Continue if field is blocked at the moment
 					bool blocked = false;
 
-					for (std::list<BlockedField>::iterator k = blocked_fields.begin();
-					     k != blocked_fields.end();
-					     ++k)
-						if ((*j)->coords == k->coords) {
+					for (const BlockedField& blocked_field :blocked_fields)
+						if (mineable_field->coords == blocked_field.coords) {
 							blocked = true;
 							break;
 						}
@@ -1897,13 +1885,13 @@
 					}
 
 					// Prefer road side fields
-					prio += (*j)->preferred_ ? 1 : 0;
+					prio += mineable_field->preferred_ ? 1 : 0;
 
 					if (prio > proposed_priority) {
 						// proposed_building = bo.id;
 						best_building = &bo;
 						proposed_priority = prio;
-						proposed_coords = (*j)->coords;
+						proposed_coords = mineable_field->coords;
 						mine = true;
 					}
 				}  // end of evaluation of field
@@ -2098,10 +2086,10 @@
 			continue;
 		}
 
-		std::vector<NearFlag>::iterator f =
+		std::vector<NearFlag>::iterator near_flag_it =
 		   find(reachableflags.begin(), reachableflags.end(), queue.top().flag);
 
-		if (f != reachableflags.end()) {
+		if (near_flag_it != reachableflags.end()) {
 			queue.pop();
 			continue;
 		}
@@ -2280,10 +2268,10 @@
 
 	// algorithm to walk on roads
 	while (!queue.empty()) {
-		std::vector<NearFlag>::iterator f =
+		std::vector<NearFlag>::iterator near_flag_it =
 		   find(nearflags_tmp.begin(), nearflags_tmp.end(), queue.top().flag);
 
-		if (f != nearflags_tmp.end()) {
+		if (near_flag_it != nearflags_tmp.end()) {
 			queue.pop();
 			continue;
 		}
@@ -2318,19 +2306,16 @@
 	// iterating over nearflags_tmp, each item in nearflags_tmp should be contained also in nearflags
 	// so for each corresponding field in nearflags we update "cost" (distance on existing roads)
 	// to actual value
-	for (std::vector<NearFlag>::iterator nf_walk_it = nearflags_tmp.begin();
-	     nf_walk_it != nearflags_tmp.end();
-	     ++nf_walk_it) {
-		uint32_t const hash_walk = coords_hash(nf_walk_it->flag->get_position());
+	for (const NearFlag& temp_near_flag : nearflags_tmp) {
+		uint32_t const hash_walk = coords_hash(temp_near_flag.flag->get_position());
 		if (lookuptable.count(hash_walk) > 0) {
 			// iterating over nearflags
-			for (std::vector<NearFlag>::iterator nf_it = nearflags.begin(); nf_it != nearflags.end();
-			     ++nf_it) {
-				uint32_t const hash = coords_hash(nf_it->flag->get_position());
+			for (NearFlag& near_flag : nearflags) {
+				uint32_t const hash = coords_hash(near_flag.flag->get_position());
 				if (hash == hash_walk) {
 					// decreasing "cost" (of walking via roads)
-					if (nf_it->cost_ > nf_walk_it->cost_) {
-						nf_it->cost_ = nf_walk_it->cost_;
+					if (near_flag.cost_ > temp_near_flag.cost_) {
+						near_flag.cost_ = temp_near_flag.cost_;
 					}
 				}
 			}
@@ -2403,26 +2388,21 @@
 		get_economy_observer(flag.economy())->flags.push_back(&flag);
 	}
 
-	for (std::list<EconomyObserver*>::iterator obs_iter = economies.begin();
-	     obs_iter != economies.end();
-	     ++obs_iter) {
+	for (EconomyObserver* economy_observer : economies) {
 		// check if any flag has changed its economy
-		std::list<Flag const*>& fl = (*obs_iter)->flags;
+		std::list<Flag const*>& fl = economy_observer->flags;
 
-		for (std::list<Flag const*>::iterator j = fl.begin(); j != fl.end();) {
-			if (&(*obs_iter)->economy != &(*j)->economy()) {
-				get_economy_observer((*j)->economy())->flags.push_back(*j);
-				j = fl.erase(j);
-			} else {
-				++j;
+		for (const Flag* flag : fl) {
+			if (&economy_observer->economy != &flag->economy()) {
+				get_economy_observer(flag->economy())->flags.push_back(flag);
+				fl.remove(flag);
 			}
 		}
 
 		// if there are no more flags in this economy,
 		// we no longer need it's observer
-		if ((*obs_iter)->flags.empty()) {
-			delete *obs_iter;
-			economies.erase(obs_iter);
+		if (economy_observer->flags.empty()) {
+			economies.remove(economy_observer);
 			return true;
 		}
 	}
@@ -2800,20 +2780,19 @@
 	}
 
 	// and now over ships
-	for (std::list<ShipObserver>::iterator sp_iter = allships.begin(); sp_iter != allships.end();
-	     ++sp_iter) {
-		if (sp_iter->ship->state_is_expedition()) {
+	for (const ShipObserver& ship_observer : allships) {
+		if (ship_observer.ship->state_is_expedition()) {
 			expeditions_in_progress += 1;
 		}
 	}
 
 	// we must verify that all remote ports are still ours (and exists at all)
 	bool still_ours;
-	for (std::unordered_set<uint32_t>::iterator ports_iter = remote_ports_coords.begin();
-	     ports_iter != remote_ports_coords.end();
-	     ++ports_iter) {
+	for (std::unordered_set<uint32_t>::iterator ports_it = remote_ports_coords.begin();
+		  ports_it != remote_ports_coords.end();
+		  ++ports_it) {
 		still_ours = false;
-		FCoords fcoords = game().map().get_fcoords(coords_unhash(*ports_iter));
+		FCoords fcoords = game().map().get_fcoords(coords_unhash(*ports_it));
 		if (fcoords.field->get_owned_by() == player_number()) {
 			if (upcast(PlayerImmovable, imm, fcoords.field->get_immovable())) {
 				still_ours = true;
@@ -2821,7 +2800,7 @@
 		}
 
 		if (!still_ours) {
-			remote_ports_coords.erase(*ports_iter);
+			remote_ports_coords.erase(*ports_it);
 			break;
 		}
 	}
@@ -2892,26 +2871,25 @@
 	}
 
 	if (!allships.empty()) {
-		// iterating over ships and executing what is needed
-		for (std::list<ShipObserver>::iterator i = allships.begin(); i != allships.end(); ++i) {
-
-			// only two states need an attention
-			if ((i->ship->get_ship_state() == Widelands::Ship::EXP_WAITING ||
-			     i->ship->get_ship_state() == Widelands::Ship::EXP_FOUNDPORTSPACE) &&
-			    !i->waiting_for_command_) {
-				if (gametime - i->last_command_time > 180 * 1000) {
-					i->waiting_for_command_ = true;
+		// Iterating over ships and executing what is needed.
+		for (ShipObserver& ship_observer : allships) {
+			// Only two states need attention.
+			if ((ship_observer.ship->get_ship_state() == Widelands::Ship::EXP_WAITING ||
+				  ship_observer.ship->get_ship_state() == Widelands::Ship::EXP_FOUNDPORTSPACE) &&
+				 !ship_observer.waiting_for_command_) {
+				if (gametime - ship_observer.last_command_time > 180 * 1000) {
+					ship_observer.waiting_for_command_ = true;
 					log("  %1d: last command for ship at %3dx%3d was %3d seconds ago, something wrong "
 					    "here?...\n",
 					    player_number(),
-					    i->ship->get_position().x,
-					    i->ship->get_position().y,
-					    (gametime - i->last_command_time) / 1000);
+						 ship_observer.ship->get_position().x,
+						 ship_observer.ship->get_position().y,
+						 (gametime - ship_observer.last_command_time) / 1000);
 				}
 			}
-			// if ships is waiting for command
-			if (i->waiting_for_command_) {
-				expedition_management(*i);
+			// If ship is waiting for command.
+			if (ship_observer.waiting_for_command_) {
+				expedition_management(ship_observer);
 			}
 		}
 	}
@@ -2920,33 +2898,27 @@
 	while (!marineTaskQueue_.empty()) {
 		if (marineTaskQueue_.back() == kStopShipyard) {
 			// iterate over all production sites searching for shipyard
-			for (std::list<ProductionSiteObserver>::iterator site = productionsites.begin();
-			     site != productionsites.end();
-			     ++site) {
-				if (site->bo->is_shipyard_) {
-					if (!site->site->is_stopped()) {
-						game().send_player_start_stop_building(*site->site);
+			for (const ProductionSiteObserver& prod_site_observer : productionsites) {
+				if (prod_site_observer.bo->is_shipyard_) {
+					if (!prod_site_observer.site->is_stopped()) {
+						game().send_player_start_stop_building(*prod_site_observer.site);
 					}
 				}
 			}
 		}
 
 		if (marineTaskQueue_.back() == kReprioritize) {
-			for (std::list<ProductionSiteObserver>::iterator site = productionsites.begin();
-			     site != productionsites.end();
-			     ++site) {
-				if (site->bo->is_shipyard_) {
-					for (uint32_t k = 0; k < site->bo->inputs_.size(); ++k) {
+			for (const ProductionSiteObserver& prod_site_observer : productionsites) {
+				if (prod_site_observer.bo->is_shipyard_) {
+					for (uint32_t k = 0; k < prod_site_observer.bo->inputs_.size(); ++k) {
 						game().send_player_set_ware_priority(
-						   *site->site, wwWARE, site->bo->inputs_.at(k), HIGH_PRIORITY);
+							*prod_site_observer.site, wwWARE, prod_site_observer.bo->inputs_.at(k), HIGH_PRIORITY);
 					}
 				}
 			}
 		}
-
 		marineTaskQueue_.pop_back();
 	}
-
 	return true;
 }
 
@@ -3127,13 +3099,9 @@
 // perhaps it will be able to replace get_stocklevel
 uint32_t DefaultAI::get_warehoused_stock(WareIndex wt) {
 	uint32_t count = 0;
-
-	for (std::list<WarehouseSiteObserver>::iterator i = warehousesites.begin();
-	     i != warehousesites.end();
-	     ++i) {
-		count += i->site->get_wares().stock(wt);
+	for (const WarehouseSiteObserver& warehouse_observer :  warehousesites) {
+		count += warehouse_observer.site->get_wares().stock(wt);
 	}
-
 	return count;
 }
 
@@ -3155,13 +3123,10 @@
 	} else {
 		new_priority = DEFAULT_PRIORITY;
 	}
-	for (std::list<TrainingSiteObserver>::iterator site = trainingsites.begin();
-	     site != trainingsites.end();
-	     ++site) {
-
-		for (uint32_t k = 0; k < site->bo->inputs_.size(); ++k) {
+	for (const TrainingSiteObserver& trainingsite_observer :trainingsites) {
+		for (uint32_t k = 0; k < trainingsite_observer.bo->inputs_.size(); ++k) {
 			game().send_player_set_ware_priority(
-			   *site->site, wwWARE, site->bo->inputs_.at(k), new_priority);
+				*trainingsite_observer.site, wwWARE, trainingsite_observer.bo->inputs_.at(k), new_priority);
 		}
 	}
 	return true;
@@ -3186,10 +3151,8 @@
 	// construction
 	unstationed_milit_buildings_ = 0;
 
-	for (std::list<MilitarySiteObserver>::iterator it = militarysites.begin();
-	     it != militarysites.end();
-	     ++it) {
-		if (it->site->stationed_soldiers().size() == 0) {
+	for (const MilitarySiteObserver& militarysite_observer : militarysites) {
+		if (militarysite_observer.site->stationed_soldiers().size() == 0) {
 			unstationed_milit_buildings_ += 1;
 		}
 	}
@@ -3387,9 +3350,11 @@
 
 /// \returns the economy observer containing \arg economy
 EconomyObserver* DefaultAI::get_economy_observer(Economy& economy) {
-	for (std::list<EconomyObserver*>::iterator i = economies.begin(); i != economies.end(); ++i)
-		if (&(*i)->economy == &economy)
-			return *i;
+	for (EconomyObserver* economy_observer : economies) {
+		if (&economy_observer->economy == &economy) {
+			return economy_observer;
+		}
+	}
 
 	economies.push_front(new EconomyObserver(economy));
 	return economies.front();
@@ -3427,20 +3392,16 @@
 		lose_building(*building);
 	} else if (upcast(Flag const, flag, &pi)) {
 		for (EconomyObserver* eco_obs : economies) {
-			for (std::list<Flag const*>::iterator flag_iter = eco_obs->flags.begin();
-			     flag_iter != eco_obs->flags.end();
-			     ++flag_iter) {
-				if (*flag_iter == flag) {
-					eco_obs->flags.erase(flag_iter);
+			for (const Flag* economy_flag : eco_obs->flags) {
+				if (economy_flag == flag) {
+					eco_obs->flags.remove(economy_flag);
 					return;
 				}
 			}
 		}
-		for (std::list<Flag const*>::iterator flag_iter = new_flags.begin();
-		     flag_iter != new_flags.end();
-		     ++flag_iter) {
-			if (*flag_iter == flag) {
-				new_flags.erase(flag_iter);
+		for (const Flag* new_flag : new_flags) {
+			if (new_flag == flag) {
+				new_flags.remove(new_flag);
 				return;
 			}
 		}
@@ -3453,9 +3414,9 @@
 void DefaultAI::out_of_resources_site(const ProductionSite& site) {
 
 	// we must identify which mine matches the productionsite a note reffers to
-	for (std::list<ProductionSiteObserver>::iterator i = mines_.begin(); i != mines_.end(); ++i)
-		if (i->site == &site) {
-			i->no_resources_count += 1;
+	for (ProductionSiteObserver& mine_observer : mines_)
+		if (mine_observer.site == &site) {
+			mine_observer.no_resources_count += 1;
 			break;
 		}
 }
@@ -3752,11 +3713,11 @@
 
 		if (bo.type == BuildingObserver::PRODUCTIONSITE) {
 
-			for (std::list<ProductionSiteObserver>::iterator i = productionsites.begin();
-			     i != productionsites.end();
-			     ++i)
-				if (i->site == &b) {
-					productionsites.erase(i);
+			for (std::list<ProductionSiteObserver>::iterator prod_site_observer_it = productionsites.begin();
+				  prod_site_observer_it != productionsites.end();
+				  ++prod_site_observer_it)
+				if (prod_site_observer_it->site == &b) {
+					productionsites.erase(prod_site_observer_it);
 					break;
 				}
 
@@ -3768,10 +3729,10 @@
 				--wares.at(bo.inputs_.at(i)).consumers_;
 			}
 		} else if (bo.type == BuildingObserver::MINE) {
-			for (std::list<ProductionSiteObserver>::iterator i = mines_.begin(); i != mines_.end();
-			     ++i) {
-				if (i->site == &b) {
-					mines_.erase(i);
+			for (std::list<ProductionSiteObserver>::iterator mines_observer_it = mines_.begin(); mines_observer_it != mines_.end();
+				  ++mines_observer_it) {
+				if (mines_observer_it->site == &b) {
+					mines_.erase(mines_observer_it);
 					break;
 				}
 			}
@@ -3785,21 +3746,21 @@
 			}
 		} else if (bo.type == BuildingObserver::MILITARYSITE) {
 
-			for (std::list<MilitarySiteObserver>::iterator i = militarysites.begin();
-			     i != militarysites.end();
-			     ++i) {
-				if (i->site == &b) {
-					militarysites.erase(i);
+			for (std::list<MilitarySiteObserver>::iterator militarysite_observer_it = militarysites.begin();
+				  militarysite_observer_it != militarysites.end();
+				  ++militarysite_observer_it) {
+				if (militarysite_observer_it->site == &b) {
+					militarysites.erase(militarysite_observer_it);
 					break;
 				}
 			}
 		} else if (bo.type == BuildingObserver::TRAININGSITE) {
 
-			for (std::list<TrainingSiteObserver>::iterator i = trainingsites.begin();
-			     i != trainingsites.end();
-			     ++i) {
-				if (i->site == &b) {
-					trainingsites.erase(i);
+			for (std::list<TrainingSiteObserver>::iterator trainingsite_observer_it = trainingsites.begin();
+				  trainingsite_observer_it != trainingsites.end();
+				  ++trainingsite_observer_it) {
+				if (trainingsite_observer_it->site == &b) {
+					trainingsites.erase(trainingsite_observer_it);
 					break;
 				}
 			}
@@ -3810,11 +3771,11 @@
 				--num_ports;
 			}
 
-			for (std::list<WarehouseSiteObserver>::iterator i = warehousesites.begin();
-			     i != warehousesites.end();
-			     ++i) {
-				if (i->site == &b) {
-					warehousesites.erase(i);
+			for (std::list<WarehouseSiteObserver>::iterator warehouse_observer_it = warehousesites.begin();
+				  warehouse_observer_it != warehousesites.end();
+				  ++warehouse_observer_it) {
+				if (warehouse_observer_it->site == &b) {
+					warehousesites.erase(warehouse_observer_it);
 					break;
 				}
 			}
@@ -3982,12 +3943,12 @@
 	for (uint32_t position = gametime % test_every; position < militarysites.size();
 	     position += test_every) {
 
-		std::list<MilitarySiteObserver>::iterator mso = militarysites.begin();
-		std::advance(mso, position);
-
-		MilitarySite* ms = mso->site;
-
-		if (!mso->enemies_nearby_) {
+		std::list<MilitarySiteObserver>::iterator militarysite_observer_it = militarysites.begin();
+		std::advance(militarysite_observer_it, position);
+
+		MilitarySite* ms = militarysite_observer_it->site;
+
+		if (!militarysite_observer_it->enemies_nearby_) {
 			continue;
 		}