widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

Review: Approve

Looks good to me - just some code style nits. Feel free to merge when you're done - don't forget to set a commit message above, so that bunnybot can pick it up.

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-23 20:35:11 +0000
> @@ -335,6 +335,52 @@
>     : in_construction(0), finished(0), is_critical(false), unoccupied(0) {
>  }
>  
> +// Reset counter for all field types
> +void MineFieldsObserver::zero() {
> +	for (auto& material : stat) {
> +		material.second = 0;
> +	}
> +}
> +
> +// Increase counter by one for specific ore/minefield type
> +void MineFieldsObserver::add(const Widelands::DescriptionIndex idx) {
> +	stat[idx] += 1;
> +}
> +
> +// Add ore into critical_ores
> +void MineFieldsObserver::add_critical_ore(const Widelands::DescriptionIndex idx) {
> +	critical_ores.insert(idx);
> +}
> +
> +// Does the player has at least one mineable field with positive amount for each critical ore?
> +bool MineFieldsObserver::has_critical_ore_fields() {
> +	for (auto ore : critical_ores) {
> +		if (get(ore) == 0) {
> +			return false;
> +		}
> +	}
> +	return true;
> +}
> +
> +// Returns count of fields with desired ore
> +uint16_t MineFieldsObserver::get(const Widelands::DescriptionIndex idx) {
> +	if (stat.count(idx) == 0) {
> +		return 0;
> +	}
> +	return stat[idx];

std::map has this side effect that this line will insert a default value if the element doesn't exist. It is fine here, just making sure that you're aware of it. Using at() is safer.

> +}
> +
> +// Count of types of mineable fields, up to 4 currently
> +uint8_t MineFieldsObserver::count_types() {
> +	uint16_t count = 0;
> +	for (auto material : stat) {
> +		if (material.second > 0) {
> +			count += 1;
> +		}
> +	}
> +	return count;
> +}
> +
>  ExpansionType::ExpansionType() {
>  	type = ExpansionMode::kResources;
>  }
> 
> === modified file 'src/ai/defaultai.cc'
> --- src/ai/defaultai.cc	2018-03-02 07:50:34 +0000
> +++ src/ai/defaultai.cc	2018-03-23 20:35:11 +0000
> @@ -1212,6 +1211,21 @@
>  
>  		i += 1;
>  	}
> +	// Updating overall statistics, first we flush the data and then iterate over all mine fields
> +	mine_fields_stat.zero();
> +	for (uint32_t j = 0; j < mineable_fields.size(); j++) {

Use ranged-based for loop - we're not manipulating the index. E.g.

for (const auto& mineable_field : mineable_fields) { ...

> +	    if (mineable_fields[j]->coords.field->get_resources_amount() > 0) {
> +		    mine_fields_stat.add(mineable_fields[j]->coords.field->get_resources());
> +		   }
> +	}
> +
> +	// Following asserts presume that there is 1-3 critical mines

is -> are

> +	if (mine_fields_stat.count_types() == 4) {

This looks hard-coded to the amount of resource types in the world. Try using World::get_nr_resources() instead.

> +		assert(mine_fields_stat.has_critical_ore_fields());
> +	}
> +	if (mine_fields_stat.count_types() == 0) {
> +		assert(!mine_fields_stat.has_critical_ore_fields());
> +	}
>  }
>  
>  /**
> @@ -2218,6 +2242,12 @@
>  	inputs[50] = (bakeries_count_ <= 1);
>  	inputs[51] = (numof_psites_in_constr > 8);
>  	inputs[52] = (numof_psites_in_constr < 8);
> +	inputs[53] = (mine_fields_stat.has_critical_ore_fields());
> +	inputs[54] = (!mine_fields_stat.has_critical_ore_fields());
> +	inputs[55] = (mine_fields_stat.count_types() == 4);

These look hard-coded to the amount of resource types in the world. Try using World::get_nr_resources() instead.

> +	inputs[56] = (mine_fields_stat.count_types() != 4);
> +	inputs[57] = (mine_fields_stat.has_critical_ore_fields());
> +	inputs[58] = (!mine_fields_stat.has_critical_ore_fields());
>  
>  	static int16_t needs_boost_economy_score = management_data.get_military_number_at(61) / 5;
>  	needs_boost_economy_score = management_data.get_military_number_at(61) / 5;


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