widelands-dev team mailing list archive

[TiborB <tiborb95@xxxxxxxxx>]

As for grouping the same buildings together - I was proposing new vector of nearby productionsites based on id - this could be used here

Also see some comments in code. 

Generally we can do small tweaks in this branch even if out of scope :)

Diff comments:

> 
> === modified file 'src/ai/defaultai.cc'
> --- src/ai/defaultai.cc	2018-04-29 09:20:29 +0000
> +++ src/ai/defaultai.cc	2018-06-03 20:52:04 +0000
> @@ -2867,16 +2883,21 @@
>  
>  						if (bo.is(BuildingAttribute::kSpaceConsumer)) {
>  							// we dont like trees nearby
> +							// NOCOM this means we must have 30 trees in a radius of 6 to get a negative value
> +							// NOCOM note: the radius to calculate all nearby values is hardcoded to 6 (kProductionArea)
>  							prio += 1 - bf->trees_nearby / 15;

I dont mind changing 15 to lower number (or remove "/ 15" entirely), even in this branch

>  							// we attempt to cluster space consumers together
> +							// NOCOM not sure whther rangers are considered here as well cause they have also the
> +							// NOCOM space consumer = true AI hint.
>  							prio += bf->space_consumers_nearby * 2;

see https://bazaar.launchpad.net/~widelands-dev/widelands/frisian_balancing_with_ai_hints/view/head:/src/ai/defaultai.cc#L5797
rangergs are excluded from space_consumers_nearby

>  							// and be far from rangers
> +							// NOCOM as rangers have a very big working area we might be still to close to them

This probably need two ranges when looking for neighbors - doeable, but more CPU will be used

>  							prio += 1 -
>  							        bf->rangers_nearby *
>  							           std::abs(management_data.get_military_number_at(102)) / 5;
>  						} else {
>  							// leave some free space between them
> -							prio -= bf->producers_nearby.at(bo.outputs.at(0)) *
> +							prio -= bf->collecting_producers_nearby.at(bo.collected_map_resource) *
>  							        std::abs(management_data.get_military_number_at(108)) / 5;
>  						}
>  
> @@ -5714,6 +5736,9 @@
>  			}
>  			assert(bo.stocklevel_count < std::numeric_limits<uint32_t>::max());
>  		} else if (!bo.outputs.empty()) {
> +			// NOCOM from what I understand from the definition of calculate_stocklevel
> +			// it is expecting a ware index or a worker index but I believe we are delivering a building index here
> +			// might lead to nonsense results. 

The function is overloaded:
https://bazaar.launchpad.net/~widelands-dev/widelands/frisian_balancing_with_ai_hints/view/head:/src/ai/defaultai.cc#L5690

>  			bo.stocklevel_count = calculate_stocklevel(bo, what);
>  		} else {
>  			bo.stocklevel_count = 0;


-- 
https://code.launchpad.net/~widelands-dev/widelands/frisian_balancing_with_ai_hints/+merge/347166
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/frisian_balancing_with_ai_hints into lp:widelands.