widelands-dev team mailing list archive

[TiborB <tiborb95@xxxxxxxxx>]

See my comments below

BTW I am doing some training, I hope this will not interfere. Once it is merged, I will re-train too..

Diff comments:

> 
> === modified file 'src/ai/defaultai.cc'
> --- src/ai/defaultai.cc	2019-05-17 11:45:39 +0000
> +++ src/ai/defaultai.cc	2019-05-18 13:37:09 +0000
> @@ -2489,21 +2499,25 @@
>  
>  			// Now verifying that all 'buildable' buildings has positive max_needed_preciousness
>  			// if they have outputs, all other must have zero max_needed_preciousness
> -			if ((bo.new_building == BuildingNecessity::kNeeded ||
> -			     bo.new_building == BuildingNecessity::kForced ||
> -			     bo.new_building == BuildingNecessity::kAllowed ||
> -			     bo.new_building == BuildingNecessity::kNeededPending) &&
> -			    (!bo.outputs.empty() || bo.is(BuildingAttribute::kBarracks))) {
> -				if (bo.max_needed_preciousness <= 0) {
> -					throw wexception("AI: Max presciousness must not be <= 0 for building: %s",
> -					                 bo.desc->name().c_str());
> -				}
> -			} else if (bo.new_building == BuildingNecessity::kForbidden) {
> +
> +			if (bo.new_building == BuildingNecessity::kForbidden) {
>  				bo.max_needed_preciousness = 0;
>  			} else {
> -				// For other situations we make sure max_needed_preciousness is zero
> -
> -				assert(bo.max_needed_preciousness == 0);
> +				bo.max_needed_preciousness = std::max(bo.max_needed_preciousness, bo.initial_preciousness);
> +				bo.max_preciousness = std::max(bo.max_preciousness, bo.initial_preciousness);
> +				if ((bo.new_building == BuildingNecessity::kNeeded ||
> +					 bo.new_building == BuildingNecessity::kForced ||
> +					 bo.new_building == BuildingNecessity::kAllowed ||
> +					 bo.new_building == BuildingNecessity::kNeededPending) &&
> +					(!bo.outputs.empty() || bo.is(BuildingAttribute::kBarracks))) {
> +					if (bo.max_needed_preciousness <= 0) {
> +						throw wexception("AI: Max presciousness must not be <= 0 for building: %s",
> +										 bo.desc->name().c_str());
> +					}
> +				} else {
> +					// For other situations we make sure max_needed_preciousness is zero
> +					// NOCOM this fails for recruitment sites now assert(bo.max_needed_preciousness == 0);

This assert?
See comment few lines above:

			// Now verifying that all 'buildable' buildings has positive max_needed_preciousness
			// if they have outputs, all other must have zero max_needed_preciousness

Also note the line:

(!bo.outputs.empty() || bo.is(BuildingAttribute::kBarracks))

So you should add recruitements sites here too (the same exemptions as barracks), what do you think?

> +				}
>  			}
>  
>  			// Positive max_needed_preciousness says a building type is needed


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1829471-worker-preciousness/+merge/367608
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1829471-worker-preciousness into lp:widelands.