Merge lp:~widelands-dev/widelands/bug-1795976-port-attack-crash into lp:widelands

[GunChleoc]

Fix endless loop in DefaultAI::dispensable_road_test

[TiborB (tiborb95)]

Code looks good to me

review: Approve

[TiborB (tiborb95)]

now when I look more deep into the code I think the function has a logical error there, I will investigate and let you know..

[GunChleoc (gunchleoc)]

Do you want to take up this branch and improve it before we merge?

I'll hold off merging for now.

[TiborB (tiborb95)]

OK, good idea... thanks

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 4110. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/440029552.
Appveyor build 3905. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1795976_port_attack_crash-3905.

[TiborB (tiborb95)]

@Gun, can you look at the diff?

I removed your std::set and just look into vector instead. Usually it is <=5 items, so no big overhead

I tested with bug's savegame and with a random game from scratch...

[GunChleoc (gunchleoc)]

Tested, bug is fixed :)

@bunnybot merge

review: Approve

[kaputtnik (franku)]

Great, thanks :)

[GunChleoc (gunchleoc)]

Thanks yourself for the savegame!

=== modified file 'src/ai/defaultai.cc'
--- src/ai/defaultai.cc	2018-09-29 14:35:29 +0000
+++ src/ai/defaultai.cc	2018-10-12 10:03:30 +0000
@@ -3584,19 +3584,18 @@
 	Flag& roadstartflag = road.get_flag(Road::FlagStart);
 	Flag& roadendflag = road.get_flag(Road::FlagEnd);
 
-	// Collecting full path (from crossing/building to another crossing/building)
+	// Calculating full road (from crossing/building to another crossing/building),
+	// this means we calculate vector of all flags of the "full road"
 	std::vector<Widelands::Flag*> full_road;
 	full_road.push_back(&roadstartflag);
 	full_road.push_back(&roadendflag);
 
-	// Making sure it starts with proper flag
 	uint16_t road_length = road.get_path().get_nsteps();
 
 	for (int j = 0; j < 2; ++j) {
 		bool new_road_found = true;
 		while (new_road_found && full_road.back()->nr_of_roads() <= 2 &&
 		       full_road.back()->get_building() == nullptr) {
-			const size_t full_road_size = full_road.size();
 			new_road_found = false;
 			for (uint8_t i = 1; i <= 6; ++i) {
 				Road* const near_road = full_road.back()->get_road(i);
@@ -3613,16 +3612,16 @@
 					other_end = &near_road->get_flag(Road::FlagStart);
 				}
 
-				if (other_end->get_position() == full_road[full_road_size - 2]->get_position()) {
-					continue;
+				// Have we already the end of road in our full_road?
+				if (std::find(full_road.begin(), full_road.end(), other_end) == full_road.end()) {
+					full_road.push_back(other_end);
+					road_length += near_road->get_path().get_nsteps();
+					new_road_found = true;
+					break;
 				}
-				full_road.push_back(other_end);
-				road_length += near_road->get_path().get_nsteps();
-				new_road_found = true;
-				break;
 			}
 		}
-		// we walked to one end, now let revert the concent of full_road and repeat in opposite
+		// we walked to one end, now let revert the content of full_road and repeat in opposite
 		// direction
 		std::reverse(full_road.begin(), full_road.end());
 	}