widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

Review: Approve

Looks good to me - just some minor nits. Please feel free to merge once you have addressed those to your liking. Don't forget to set the commit message above ;)

Diff comments:

> === modified file 'src/ai/ai_help_structs.cc'
> --- src/ai/ai_help_structs.cc	2018-02-16 20:42:21 +0000
> +++ src/ai/ai_help_structs.cc	2018-03-16 07:47:06 +0000
> @@ -83,6 +83,55 @@
>  	return true;
>  }
>  
> +
> +// CheckStepOwnTerritory
> +CheckStepOwnTerritory::CheckStepOwnTerritory(Player* const pl, uint8_t const mc, bool const oe)
> +   : player(pl), movecaps(mc), open_end(oe) {
> +}
> +
> +// Defines when movement is allowed:
> +// 1. startfield is walkable (or it is the first step)
> +// And endfield either:
> +// 2a. is walkable
> +// 2b. has our PlayerImmovable (building or flag)
> +bool CheckStepOwnTerritory::allowed(
> +   const Map& map, FCoords start, FCoords end, int32_t, CheckStep::StepId const id) const {
> +	uint8_t endcaps = player->get_buildcaps(end);
> +	uint8_t startcaps = player->get_buildcaps(start);

Make these const - this will help the compiler to optimize.

> +
> +	// We should not cross fields with road or flags (or any other immovable)
> +	// Or rather we can step on it, but not go on from such field
> +	if ((map.get_immovable(start)) && !(id == CheckStep::stepFirst)) {
> +		return false;
> +	}
> +
> +	// Start field must be walkable
> +	if (!(startcaps & movecaps))

Please add the {}. Leaving them out can cause bugs that are hard to find.

> +		return false;
> +
> +	// Endfield can not be water
> +	if (endcaps & MOVECAPS_SWIM)

Please add the {}. Leaving them out can cause bugs that are hard to find.

> +		return false;
> +
> +	return true;
> +}
> +
> +// We accept either walkable teritory or field with own immovable

territory

> +bool CheckStepOwnTerritory::reachable_dest(const Map& map, const FCoords& dest) const {
> +	uint8_t endcaps = player->get_buildcaps(dest);

const

> +	if (BaseImmovable const* const imm = map.get_immovable(dest)) {
> +		if (upcast(PlayerImmovable const, player_immovable, imm)) {

use imm->descr().type() instead - much more efficient than doing a dynamic cast.

> +			return true;
> +		} else {
> +			return false;
> +		}
> +	}
> +	if (endcaps & MOVECAPS_WALK) {
> +		return true;
> +	}
> +	return false;
> +}
> +
>  // We are looking for fields we can walk on
>  // and owned by hostile player.
>  FindNodeEnemy::FindNodeEnemy(Player* p, Game& g) : player(p), game(g) {
> 
> === modified file 'src/ai/defaultai.cc'
> --- src/ai/defaultai.cc	2018-03-02 07:50:34 +0000
> +++ src/ai/defaultai.cc	2018-03-16 07:47:06 +0000
> @@ -3819,19 +3825,41 @@
>  		game().send_player_build_road(player_number(), path);
>  		return true;
>  	}
> -	// if all possible roads skipped
> -	if (last_attempt_) {
> -		Building* bld = flag.get_building();
> -		// first we block the field and vicinity for 15 minutes, probably it is not a good place to
> -		// build on
> -		MapRegion<Area<FCoords>> mr(map, Area<FCoords>(map.get_fcoords(bld->get_position()), 2));
> -		do {
> -			blocked_fields.add(mr.location(), game().get_gametime() + 15 * 60 * 1000);
> -		} while (mr.advance(map));
> -		remove_from_dqueue<Widelands::Flag>(eco->flags, &flag);
> -		game().send_player_bulldoze(*const_cast<Flag*>(&flag));
> -		dead_ends_check_ = true;
> -		return true;
> +	// We cant build a road so let block the vicinity as an indication this area is not connectible

can't, let's

> +	// Usually we block for 2 minutes, but if it is a last attempt we block for 10 minutes
> +	// Note: we block the vicinity only if this economy (usually a sole flag with a building) is not
> +	// connected to a warehouse
> +	if (flag.get_economy()->warehouses().empty()) {
> +
> +		// blocking only if latest block was less then 60 seconds ago or it is last attempt
> +		if (eco->fields_block_last_time + 60000 < gametime || last_attempt_) {
> +			eco->fields_block_last_time = gametime;
> +
> +			uint32_t block_time = 2 * 60 * 1000;

Suggestion:

const uint32_t block_time = last_attempt_ ? 10 * 60 * 1000 : 2 * 60 * 1000;

> +			if (last_attempt_) {
> +				block_time = 10 * 60 * 1000;
> +			}
> +
> +			FindNodeAcceptAll buildable_functor;
> +			CheckStepOwnTerritory check_own(player_, MOVECAPS_WALK, true);
> +
> +			// get all flags within radius
> +			std::vector<Coords> reachable_to_block;
> +			map.find_reachable_fields(Area<FCoords>(map.get_fcoords(flag.get_position()), checkradius),
> +			                          &reachable_to_block, check_own, buildable_functor);
> +
> +			for (auto coords : reachable_to_block) {
> +				blocked_fields.add(coords, game().get_gametime() + block_time);
> +			}
> +		}
> +
> +		// If it last attempt we also destroy the flag (with a building if any attached)
> +		if (last_attempt_) {
> +			remove_from_dqueue<Widelands::Flag>(eco->flags, &flag);
> +			game().send_player_bulldoze(*const_cast<Flag*>(&flag));
> +			dead_ends_check_ = true;
> +			return true;
> +		}
>  	}
>  	return false;
>  }


-- 
https://code.launchpad.net/~widelands-dev/widelands/ai_blocked_fields_tweaks/+merge/341483
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ai_blocked_fields_tweaks.