widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

Review: Approve

Code LGTM, just a few tiny nits. Not tested.

There were a lot of entries in the diff that don't contain any code changes at all - please use a merge tool like Meld to compare to trunk the next time this happens. This will save the reviewer a lot of time.

Diff comments:

> === modified file 'src/ai/ai_help_structs.cc'
> --- src/ai/ai_help_structs.cc	2017-11-24 09:19:52 +0000
> +++ src/ai/ai_help_structs.cc	2018-01-06 19:40:55 +0000
> @@ -236,6 +236,7 @@
>       unowned_land_nearby(0),
>       enemy_owned_land_nearby(0U),
>       unowned_buildable_spots_nearby(0U),
> +     unowned_portspace_vicinity_nearby(0U),

"vicinity" means the same as "nearby", so you can just drop it from the variable name.

>       nearest_buildable_spot_nearby(0U),
>       near_border(false),
>       unowned_mines_spots_nearby(0),
> 
> === modified file 'src/ai/defaultai.cc'
> --- src/ai/defaultai.cc	2017-12-10 21:19:02 +0000
> +++ src/ai/defaultai.cc	2018-01-06 19:40:55 +0000
> @@ -997,6 +979,19 @@
>  		}
>  	}
>  
> +	// getting list of all fields nearby port space
> +	// TODO it seems port spaces can change over time so ports_vicinity needs to be refreshed from

TODO(tiborb):

> +	// time to time
> +	for (const Coords& c : map.get_port_spaces()) {
> +		MapRegion<Area<FCoords>> mr(map, Area<FCoords>(map.get_fcoords(c), 3));
> +		do {
> +			const uint32_t hash = mr.location().hash();
> +			if (!ports_vicinity.count(hash)) {
> +				ports_vicinity.insert(hash);
> +			}
> +		} while (mr.advance(map));
> +	}
> +
>  	// printing identified basic buildings if we are in the basic economy mode
>  	basic_economy_established = persistent_data->remaining_basic_buildings.empty();
>  	if (!basic_economy_established) {
> @@ -2454,10 +2436,21 @@
>  
>  			int32_t prio = 0;  // score of a bulding on a field
>  
> +			// testing for reserved ports
> +			if (!bo.is(BuildingAttribute::kPort)) {
> +				if (bf->portspace_nearby == ExtendedBool::kTrue) {
> +					if (num_ports == 0) {
> +						continue;
> +					}
> +					// If we have at least on port, we can perhaps build here something

on -> one .... build something here

> +					// but decreasing the score to discourage it
> +					prio -= 5 * std::abs(management_data.get_military_number_at(52));
> +				}
> +			}
> +
>  			if (bo.type == BuildingObserver::Type::kProductionsite) {
>  
> -				prio = management_data.neuron_pool[44].get_result_safe(bf->military_score_ / 20) / 5;
> -				assert(prio >= -20 && prio <= 20);
> +				prio += management_data.neuron_pool[44].get_result_safe(bf->military_score_ / 20) / 5;
>  
>  				// this can be only a well (as by now)
>  				if (bo.is(BuildingAttribute::kWell)) {


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