Suppress recruit/recall commands from the context menu...

...for shrouded and visibly occupied hexes.

Part of a fix for bug #19844.

changelog

@@ -209,6 +209,8 @@ Version 1.11.0-svn:
      finished no longer unselects the current unit (bug #19734). (The new
      movement command is still deliberately ignored.)
    * Fix income changes during unit movement (bug #19788).
+   * The recruit and recall commands no longer appear in the context menu
+     for shrouded or (visibly) occupied hexes.
  * Whiteboard:
    * Fixed bug #19626: segfaults on window resize
    * Fixed bug #19369: Using planning mode can cause losing ability to move my units

players_changelog

@@ -117,6 +117,8 @@ Version 1.11.0-svn:
    * Trying to initiate movement (or an attack) before previous movement is
      finished no longer unselects the current unit (bug #19734). (The new
      movement command is still deliberately ignored.)
+   * The recruit and recall commands no longer appear in the context menu
+     for shrouded or (visibly) occupied hexes.
 
  * Whiteboard:
    * Fixed bug #19369: Using planning mode can cause losing ability to move my

src/actions.cpp

@@ -53,7 +53,10 @@ static lg::log_domain log_ai_testing("ai/testing");
 
 struct castle_cost_calculator : pathfind::cost_calculator
 {
-	castle_cost_calculator(const gamemap& map) : map_(map)
+	castle_cost_calculator(const gamemap& map, const team & view_team) :
+		map_(map),
+		viewer_(view_team),
+		use_shroud_(view_team.uses_shroud())
 	{}
 
 	virtual double cost(const map_location& loc, const double) const
@@ -61,11 +64,16 @@ struct castle_cost_calculator : pathfind::cost_calculator
 		if(!map_.is_castle(loc))
 			return 10000;
 
+		if ( use_shroud_ && viewer_.shrouded(loc) )
+			return 10000;
+
 		return 1;
 	}
 
 private:
 	const gamemap& map_;
+	const team& viewer_;
+	const bool use_shroud_; // Allows faster checks when shroud is disabled.
 };
 
 void move_unit_spectator::add_seen_friend(const unit_map::const_iterator &u)
@@ -326,19 +334,43 @@ void unit_creator::post_create(const map_location &loc, const unit &new_unit, bo
 }
 
 
-bool can_recruit_on(const gamemap& map, const map_location& leader, const map_location& loc)
+/**
+ * Checks to see if a leader at @a leader_loc could recruit on @a recruit_loc.
+ * This takes into account terrain, shroud (for side @a side), and whether or
+ * not there is already a visible unit at recruit_loc.
+ * The behavior for an invalid @a side is subject to change for future needs.
+ */
+bool can_recruit_on(const gamemap& map, const map_location& leader_loc, const map_location& recruit_loc, int side)
 {
-	if(!map.is_castle(loc))
+	if( !map.is_castle(recruit_loc) )
 		return false;
 
-	if(!map.is_keep(leader))
+	if( !map.is_keep(leader_loc) )
 		return false;
 
-	castle_cost_calculator calc(map);
+	if ( side < 1  ||  resources::teams == NULL  ||
+	     resources::teams->size() < static_cast<size_t>(side) ) {
+		// Invalid side specified.
+		// Currently this cannot happen, but it could conceivably be used in
+		// the future to request that shroud and visibility be ignored. Until
+		// that comes to pass, just return.
+ 		return false;
+	}
+	const team & view_team = (*resources::teams)[side-1];
+
+	if ( view_team.shrouded(recruit_loc) )
+		return false;
+
+	if ( get_visible_unit(recruit_loc, view_team) != NULL )
+		return false;
+
+	castle_cost_calculator calc(map, view_team);
 	// The limit computed in the third argument is more than enough for
 	// any convex castle on the map. Strictly speaking it could be
 	// reduced to sqrt(map.w()**2 + map.h()**2).
-	pathfind::plain_route rt = pathfind::a_star_search(leader, loc, map.w()+map.h(), &calc, map.w(), map.h());
+	pathfind::plain_route rt =
+		pathfind::a_star_search(leader_loc, recruit_loc, map.w()+map.h(), &calc,
+		                        map.w(), map.h());
 	return !rt.steps.empty();
 }
 
@@ -362,7 +394,7 @@ const std::set<std::string> get_recruits_for_location(int side, const map_locati
 			continue;
 
 		// Check if the leader is on a connected keep.
-		if ( can_recruit_on(*resources::game_map, u->get_location(), recruit_loc) ) {
+		if ( can_recruit_on(*resources::game_map, *u, recruit_loc) ) {
 			leader_in_place= true;
 			local_result.insert(u->recruits().begin(), u->recruits().end());
 		} else
@@ -407,7 +439,7 @@ const std::vector<const unit*> get_recalls_for_location(int side, const map_loca
 				continue;
 
 			// Check if the leader is on a connected keep.
-			if ( can_recruit_on(*resources::game_map, u->get_location(), recall_loc) )
+			if ( can_recruit_on(*resources::game_map, *u, recall_loc) )
 				leader_in_place= true;
 			else continue;
 
@@ -478,7 +510,7 @@ std::string find_recall_location(const int side, map_location& recall_loc, map_l
 			continue;
 		leader_fit = leader_keep;
 
-		if (can_recruit_on(*resources::game_map, leader_keep->get_location(), recall_loc)) {
+		if ( can_recruit_on(*resources::game_map, *leader_fit, recall_loc)) {
 			leader_opt = leader_fit;
 			if (resources::units->count(recall_loc) == 1)
 				recall_loc = tmp_location;
@@ -563,7 +595,7 @@ std::string find_recruit_location(const int side, map_location& recruit_location
 			continue;
 		leader_fit = leader_keep;
 
-		if (can_recruit_on(*resources::game_map, leader_fit->get_location(), recruit_location)) {
+		if ( can_recruit_on(*resources::game_map, *leader_fit, recruit_location)) {
 			leader_opt = leader_fit;
 			if (resources::units->count(recruit_location) == 1)
 				recruit_location = tmp_location;
@@ -644,7 +676,7 @@ void place_recruit(const unit &u, const map_location &recruit_location, const ma
 	for(; leader != resources::units->end(); ++leader)
 		if (leader->can_recruit() &&
 		    leader->side() == new_unit.side() &&
-		    can_recruit_on(*resources::game_map, leader->get_location(), recruit_location))
+		    can_recruit_on(*resources::game_map, *leader, recruit_location))
 			break;
 	if (show) {
 		if (leader.valid()) {

src/actions.hpp

@@ -68,7 +68,13 @@ private:
 };
 
 
-bool can_recruit_on(const gamemap& map, const map_location& leader, const map_location& loc);
+/// Checks to see if a leader at @a leader_loc could recruit on @a recruit_loc.
+bool can_recruit_on(const gamemap& map, const map_location& leader_loc, const map_location& recruit_loc, int side);
+/// Checks to see if @a leader (assumed a leader) can recruit on @a recruit_loc.
+/// This takes into account terrain, shroud, and whether or not there is already
+/// a visible unit at recruit_loc.
+inline bool can_recruit_on(const gamemap& map, const unit& leader, const map_location& recruit_loc)
+{ return can_recruit_on(map, leader.get_location(), recruit_loc, leader.side()); }
 
 /**
  * Finds a location to place a unit.

src/ai/actions.cpp

@@ -579,7 +579,7 @@ bool recall_result::test_suitable_recall_location(const unit &my_leader)
 		recall_location_ = pathfind::find_vacant_tile(*resources::game_map, *resources::units, my_leader.get_location(), pathfind::VACANT_CASTLE);
 	}
 
-	if (!can_recruit_on(*resources::game_map, my_leader.get_location(), recall_location_)) {
+	if ( !can_recruit_on(*resources::game_map, my_leader, recall_location_) ) {
 		set_error(E_BAD_RECALL_LOCATION);
 		return false;
 	}
@@ -779,7 +779,7 @@ bool recruit_result::test_suitable_recruit_location(const unit &my_leader)
 		recruit_location_ = pathfind::find_vacant_tile(*resources::game_map, *resources::units, my_leader.get_location(), pathfind::VACANT_CASTLE);
 	}
 
-	if (!can_recruit_on(*resources::game_map, my_leader.get_location(), recruit_location_)) {
+	if ( !can_recruit_on(*resources::game_map, my_leader, recruit_location_) ) {
 		set_error(E_BAD_RECRUIT_LOCATION);
 		return false;
 	}

src/play_controller.cpp

@@ -1240,7 +1240,7 @@ bool play_controller::in_context_menu(hotkey::HOTKEY_COMMAND command) const
 				leader != units_.end();++leader) {
 			if (leader->can_recruit() &&
 			    leader->side() == resources::screen->viewing_side() &&
-				can_recruit_on(map_, leader->get_location(), mouse_handler_.get_last_hex()))
+				can_recruit_on(map_, *leader, mouse_handler_.get_last_hex()))
 				return true;
 		}
 		return false;

src/whiteboard/manager.cpp

@@ -277,7 +277,7 @@ bool manager::allow_leader_to_move(unit const& leader) const
 		if(recruit || recall)
 		{
 			map_location const target_hex = recruit?recruit->get_recruit_hex():recall->get_recall_hex();
-			if (can_recruit_on(*resources::game_map, leader.get_location(), target_hex))
+			if ( can_recruit_on(*resources::game_map, leader, target_hex) )
 				return false;
 		}
 	}

src/whiteboard/side_actions.cpp

@@ -460,7 +460,7 @@ side_actions::iterator side_actions::bump_earlier(side_actions::iterator positio
 			if(recruit_recall_loc.valid()) {
 				unit const* leader = bump_earlier->get_unit();
 				if(leader->can_recruit()
-					&& can_recruit_on(*resources::game_map, leader->get_location(), recruit_recall_loc)) {
+					&& can_recruit_on(*resources::game_map, *leader, recruit_recall_loc)) {
 					if(unit const* backup_leader = find_backup_leader(*leader)) {
 						side_actions::iterator it = find_first_action_of(*backup_leader);
 						if(!(it == end() || position < it)) {

src/whiteboard/utility.cpp

@@ -70,7 +70,7 @@ unit const* find_backup_leader(unit const& leader)
 	{
 		if (unit.can_recruit() && unit.id() != leader.id())
 		{
-			if (can_recruit_on(*resources::game_map, unit.get_location(), leader.get_location()))
+			if ( can_recruit_on(*resources::game_map, unit, leader.get_location()) )
 				return &unit;
 		}
 	}
@@ -87,7 +87,7 @@ unit* find_recruiter(size_t team_index, map_location const& hex)
 	BOOST_FOREACH(unit& u, *resources::units)
 		if(u.can_recruit()
 				&& u.side() == static_cast<int>(team_index+1)
-				&& can_recruit_on(map,u.get_location(),hex))
+				&& can_recruit_on(map, u, hex))
 			return &u;
 	return NULL;
 }