← Back to team overview

widelands-dev team mailing list archive

[Merge] lp:~kxq/widelands/feature-1656664-scout-improvement into lp:widelands

 

Teppo Mäenpää has proposed merging lp:~kxq/widelands/feature-1656664-scout-improvement into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1656664 in widelands: "Scouts are a sore point of this game."
  https://bugs.launchpad.net/widelands/+bug/1656664

For more details, see:
https://code.launchpad.net/~kxq/widelands/feature-1656664-scout-improvement/+merge/335284

This is a lightweight fix to the issue described verbosely in https://wl.widelands.org/forum/topic/2792 and briefly below:

If player can only build small military huts, and the enemy has a large one, the large may be unattackable as a result of small vision range and long distance. The only way is to use scout to temporarily bring the enemy MS to vision. However, that requires a lot of patience because of reasons discussed in the forum.

With this modification, the switches between random walking and doing an excursion to enemy military sites.

The scout is still in the worker class.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~kxq/widelands/feature-1656664-scout-improvement into lp:widelands.
=== modified file 'data/tribes/workers/atlanteans/scout/init.lua'
--- data/tribes/workers/atlanteans/scout/init.lua	2017-02-12 09:10:57 +0000
+++ data/tribes/workers/atlanteans/scout/init.lua	2017-12-16 17:12:13 +0000
@@ -15,7 +15,7 @@
    descname = pgettext("atlanteans_worker", "Scout"),
    helptext_script = dirname .. "helptexts.lua",
    icon = dirname .. "menu.png",
-   vision_range = 2,
+   vision_range = 3,
 
    buildcost = {
       atlanteans_carrier = 1

=== modified file 'data/tribes/workers/barbarians/scout/init.lua'
--- data/tribes/workers/barbarians/scout/init.lua	2017-02-12 09:10:57 +0000
+++ data/tribes/workers/barbarians/scout/init.lua	2017-12-16 17:12:13 +0000
@@ -17,7 +17,7 @@
    descname = pgettext("barbarians_worker", "Scout"),
    helptext_script = dirname .. "helptexts.lua",
    icon = dirname .. "menu.png",
-   vision_range = 2,
+   vision_range = 3,
 
    buildcost = {
       barbarians_carrier = 1

=== modified file 'data/tribes/workers/empire/scout/init.lua'
--- data/tribes/workers/empire/scout/init.lua	2017-02-12 09:10:57 +0000
+++ data/tribes/workers/empire/scout/init.lua	2017-12-16 17:12:13 +0000
@@ -17,7 +17,7 @@
    descname = pgettext("empire_worker", "Scout"),
    helptext_script = dirname .. "helptexts.lua",
    icon = dirname .. "menu.png",
-   vision_range = 2,
+   vision_range = 3,
 
    buildcost = {
       empire_carrier = 1

=== modified file 'src/logic/findimmovable.cc'
--- src/logic/findimmovable.cc	2017-05-20 22:42:49 +0000
+++ src/logic/findimmovable.cc	2017-12-16 17:12:13 +0000
@@ -21,6 +21,7 @@
 
 #include "base/macros.h"
 #include "economy/flag.h"
+#include "logic/map.h"
 #include "logic/map_objects/immovable.h"
 #include "logic/map_objects/tribes/militarysite.h"
 
@@ -66,6 +67,14 @@
 	}
 	return false;
 }
+// Enemy military sites that cannot be attacked (for scout)
+bool FindForeignMsite::accept(const BaseImmovable& imm) const {
+	if (upcast(MilitarySite const, ms, &imm))
+		{
+			return &ms->owner() != &player;
+		}
+	return false;
+}
 
 bool FindImmovableByDescr::accept(const BaseImmovable& baseimm) const {
 	if (upcast(const Immovable, imm, &baseimm)) {

=== modified file 'src/logic/findimmovable.h'
--- src/logic/findimmovable.h	2017-08-18 16:29:43 +0000
+++ src/logic/findimmovable.h	2017-12-16 17:12:13 +0000
@@ -138,6 +138,13 @@
 
 	bool accept(const BaseImmovable&) const;
 };
+struct FindForeignMsite {
+	explicit FindForeignMsite(const Player& init_player) : player(init_player) {
+	}
+	bool accept(const BaseImmovable&) const;
+	const Player& player;
+};
+
 struct FindImmovableByDescr {
 	explicit FindImmovableByDescr(const ImmovableDescr& init_descr) : descr(init_descr) {
 	}

=== modified file 'src/logic/map_objects/tribes/worker.cc'
--- src/logic/map_objects/tribes/worker.cc	2017-11-12 08:59:45 +0000
+++ src/logic/map_objects/tribes/worker.cc	2017-12-16 17:12:13 +0000
@@ -2551,6 +2551,141 @@
 	state.ivar1 = radius;
 	state.ivar2 = game.get_gametime() + time;
 
+	// Enemy military sites cannot be attacked, if those are not visible.
+	// However, Widelands workers are still somewhat aware of their presence. For example,
+	// Player does not acquire ownership of land, even if a militarysite blocking it is
+	// invisible. Therefore, it is IMO okay for the scout to be aware of those as well.
+	// Scout occasionally pays special attention to enemy military sites, to give the player
+	// an opportunity to attack. This is important, if the player can only build small huts
+	// and the enemy has one of the bigggest ones: without scout, the player has no way of attacking.
+	// The following block switches between two modes of operation:
+	// - Random walk
+	// - Lurking near an enemy military site.
+	// The code keeps track if interesting military sites, so that they all are visited.
+	// When the list of unvisited potential attack targets is exhausted, the list is rebuilt.
+	// The first element in the vector is special: It is used to store the location of the scout's hut
+	// at the moment of creation. If used dismantles the site and builds a new, the old points of interest
+	// are no longer valid and the list is cleared.
+	// Random remarks: Some unattackable military sites are also visited (like one under construction).
+	// Also, dismantled buildings may end up here. I do not consider these bugs, but if somebody reports,
+	// the behavior can always be altered.
+
+	// I assume that this check always matches.
+	if (get_position().field) {
+		// I assume that the following also always matches.
+		// Should I replace these with an assert?
+		if (get_position().field->get_immovable()) {
+			// Yeah -- this gives the location of the flag of the hut, when the scout is inside it.
+			Coords hutpos = get_position().field->get_immovable()->get_positions(game)[0];
+
+			// the first element of poi-vector stores the location
+			// of my hut, at the time of creation.
+			// If the building location changes, then pop the now-obsolete
+			// list of points of interest
+			const Map& map = game.map();
+			if (scout_pois.size()) {
+				if (map.calc_distance(scout_pois[0].poi, hutpos)) {
+					// Hut has been relocated -- clear the POIs.
+					scout_pois.clear();
+				}
+			}
+
+			if (scout_pois.empty()) {
+				// Store the position of homebase
+				struct PlaceToScout home(false, hutpos);
+				scout_pois.push_back(home);
+			}
+			if (1 < scout_pois.size()) {
+				// If there was an old place to visit in queue, remove it.
+				scout_pois.pop_back();
+			}
+
+			// After the pop above, the latest entry of scout_pois is the next MS to visit (if known)
+			// Check whether it is still interesting (=whether it is still invisible)
+			const Player * pptr = get_owner();
+			while (1 < scout_pois.size()) {
+				MapIndex mt = map.get_index(scout_pois.back().poi, map.get_width());
+				if (1 < pptr->vision(mt))
+					// The mil site is now visible. Either player
+					// has acquired possession of more military sites
+					// of own, or own folks are nearby.
+					scout_pois.pop_back();
+				else
+					break;
+			}
+
+
+			if (2 > scout_pois.size()) {
+				// Time to find new places worth visiting.
+				Area<FCoords> revealations (map.get_fcoords(get_position()), state.ivar1);
+				std::vector<ImmovableFound> visit_us;
+				CheckStepWalkOn csteb(MOVECAPS_WALK, true);
+				map.find_reachable_immovables(revealations, &visit_us,
+						csteb,
+						FindFlagOf(FindForeignMsite(*pptr)));
+				// Now, I have a list of military sites.
+				// Regarding haveabreak: If there are many enemy sites, push a random walk request into queue now and then.
+				uint32_t haveabreak = 3;
+				for (const auto& vu : visit_us) {
+					upcast(Flag, aflag, vu.object);
+					Building * abu = aflag->get_building();
+					// Assuming that this always succeeds.
+					if (upcast(MilitarySite const, ms, abu)) {
+						// This should succeed always, too.
+						// Even if not, redundant: Own military sites
+						// are always visible.
+						if (&ms->owner() != pptr) {
+							// Check the visibility
+							MapIndex mx = map.get_index(vu.coords, map.get_width());
+							if (2 > pptr->vision(mx)) {
+								// The find_reachable_immovable sometimes returns multiple instances.
+								// Let's not add duplicates to work list.
+								bool unique = true;
+								unsigned t = 1;
+								unsigned sps = scout_pois.size();
+								while (t < sps) {
+									if (vu.coords.x == scout_pois[t].poi.x && vu.coords.y == scout_pois[t].poi.y)
+										unique = false;
+									t += 1;
+								}
+								if (unique) {
+									haveabreak -= 1 ;
+									if (!haveabreak) {
+										// If there are many MSs to visit,
+										// do a random walk in-between also.
+										haveabreak = 3 ;
+										PlaceToScout gosomewhere(true);
+										scout_pois.push_back(gosomewhere);
+									}
+									// if vision is zero, blacked out.
+									// if vision is one, old info exists; unattackable.
+									// When entering here, the place is worth
+									// scouting.
+									PlaceToScout go_there(false, vu.coords);
+									scout_pois.push_back(go_there);
+								}
+							}
+						}
+					}
+				}
+				// debugging, remove this block
+				for (auto car : scout_pois) {
+					if (car.whereever)
+						std::cout << "TDS: New worklist: mandawander" << std::endl;
+					else
+						std::cout << "TDS: New worklist:            "<<car.poi.x<<" , "<<car.poi.y <<std::endl;
+				}
+				// I suppose that this never triggers.
+				// Anyway. In savegame, I assume that the vector length fits to eight bits. Therefore,
+				while (254 < scout_pois.size())
+					scout_pois.pop_back();
+				// Push a "go-anywhere" -directive into work list
+				PlaceToScout gosomewhere(true);
+				scout_pois.push_back(gosomewhere);
+			}
+		}
+	}
+
 	// first get out
 	push_task(game, taskLeavebuilding);
 	State& stateLeave = top_state();
@@ -2569,8 +2704,11 @@
 
 	const Map& map = game.map();
 
+	struct PlaceToScout scoutat = scout_pois.back(); // do not pop; this function is called many times per run.
+
+	bool do_run = static_cast<int32_t>(state.ivar2 - game.get_gametime()) > 0;
 	// If not yet time to go home
-	if (static_cast<int32_t>(state.ivar2 - game.get_gametime()) > 0) {
+	if (scoutat.whereever && do_run) {
 		std::vector<Coords> list;  //< List of interesting points
 		CheckStepDefault cstep(descr().movecaps());
 		FindNodeAnd ffa;
@@ -2619,7 +2757,6 @@
 			if (oldest_coords != get_position()) {
 				molog("[scout]: All fields discovered. Go to (%i, %i)\n", oldest_coords.x,
 				      oldest_coords.y);
-
 				if (!start_task_movepath(
 				       game, oldest_coords, 0, descr().get_right_walk_anims(does_carry_ware())))
 					molog("[scout]: Failed to reach destination\n");
@@ -2630,7 +2767,38 @@
 		}
 		// No reachable fields found.
 		molog("[scout]: nowhere to go!\n");
+	} else if (do_run) {
+		// The scout shall hang around an enemy military site.
+		std::vector<Coords> list;  // locations near the MS under inspection
+		CheckStepDefault cstep(descr().movecaps());
+		FindNodeAnd ffa;
+		ffa.add(FindNodeImmovableSize(FindNodeImmovableSize::sizeNone), false);
+		Coords current_coords = get_position();
+		// poi points to the enemy military site; walk in random at vicinity.
+		// First try some near-close fields. If no success then try some further off ones.
+		// This code is partially copied from above; I did not check why start_task_movepath
+		// would fail. Therefore, the looping can be a bit silly to more knowledgeable readers.
+		for (unsigned vicinity = 1 ; vicinity < 4 ; vicinity++) {
+			Area<FCoords> exploring_area(map.get_fcoords(scoutat.poi), vicinity);
+			if (map.find_reachable_fields(exploring_area, &list, cstep, ffa) > 0) {
+				unsigned formax = list.size();
+				if (3 + vicinity < formax)
+					formax = 3 + vicinity;
+				for (uint8_t i = 0; i < formax; ++i) {
+					const std::vector<Coords>::size_type lidx = game.logic_rand() % list.size();
+					Coords const coord = list[lidx];
+					list.erase(list.begin() + lidx);
+					std::cout <<"TDS ("<<current_coords.x<<","<<current_coords.y <<") => (" <<coord.x<<","<<coord.y<<")"<<std::endl;
+					if (!start_task_movepath(
+						game, coord, 0, descr().get_right_walk_anims(does_carry_ware())))
+						molog("[scout]: failed to reach destination (x)\n");
+					else
+						return;  // start_task_movepath was successfull.
+				}
+			}
+		}
 	}
+
 	// time to go home or found nothing to go to
 	pop_task(game);
 	schedule_act(game, 10);
@@ -2670,7 +2838,7 @@
 ==============================
 */
 
-constexpr uint8_t kCurrentPacketVersion = 2;
+constexpr uint8_t kCurrentPacketVersion = 3;
 
 Worker::Loader::Loader() : location_(0), carried_ware_(0) {
 }
@@ -2690,6 +2858,24 @@
 				worker.transfer_ = new Transfer(dynamic_cast<Game&>(egbase()), worker);
 				worker.transfer_->read(fr, transfer_);
 			}
+			unsigned veclen = fr.unsigned_8();
+			for (unsigned q = 0 ; q < veclen ; q++)
+			  {
+			    if (fr.unsigned_8())
+			      {
+				PlaceToScout gsw(true);
+				worker.scout_pois.push_back(gsw);
+			      }
+			    else
+			      {
+				int16_t x = fr.signed_16();
+				int16_t y = fr.signed_16();
+				Coords peekpos = Coords(x, y);
+				PlaceToScout gtt(false, peekpos);
+				worker.scout_pois.push_back(gtt);
+			      }
+			  }
+
 		} else {
 			throw UnhandledVersionError("Worker", packet_version, kCurrentPacketVersion);
 		}
@@ -2836,5 +3022,21 @@
 	} else {
 		fw.unsigned_8(0);
 	}
+
+	fw.unsigned_8(scout_pois.size());
+	for (auto p: scout_pois)
+	  {
+	    if (p.whereever)
+	      fw.unsigned_8(1);
+	    else
+	      {
+		fw.unsigned_8(0);
+		// Is there a better way to save Coords? This makes
+		// unnecessary assumptions of the internals of Coords
+		fw.signed_16(p.poi.x);
+		fw.signed_16(p.poi.y);
+	      }
+	  }
+
 }
 }

=== modified file 'src/logic/map_objects/tribes/worker.h'
--- src/logic/map_objects/tribes/worker.h	2017-11-26 14:44:23 +0000
+++ src/logic/map_objects/tribes/worker.h	2017-12-16 17:12:13 +0000
@@ -27,6 +27,7 @@
 #include "logic/map_objects/tribes/productionsite.h"
 #include "logic/map_objects/tribes/worker_descr.h"
 #include "map_io/tribes_legacy_lookup_table.h"
+#include "logic/widelands_geometry.h"
 
 namespace Widelands {
 class Building;
@@ -265,6 +266,16 @@
 	Transfer* transfer_;               ///< where we are currently being sent
 	int32_t current_exp_;              ///< current experience
 
+	// List of places to visit (only if scout), plus a reminder to
+	// occasionally go just somewhere.
+	struct PlaceToScout {
+	PlaceToScout(bool we, Coords pt) : whereever(we), poi(pt) {}
+	PlaceToScout(bool we) : whereever(we) {}
+		const bool whereever;
+		const Coords poi;
+	};
+	std::vector <PlaceToScout> scout_pois;
+
 	// saving and loading
 protected:
 	struct Loader : public Bob::Loader {


Follow ups