widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Review: Needs Fixing

I suggest to simplify the code.

Diff comments:

> 
> === modified file 'src/logic/map.cc'
> --- src/logic/map.cc	2016-02-07 09:30:20 +0000
> +++ src/logic/map.cc	2016-02-10 08:20:28 +0000
> @@ -2121,11 +2121,15 @@
>  	return false;
>  }
>  
> -bool Map::has_artifacts(const World& world) {
> -	for (int32_t i = 0; i < world.get_nr_immovables(); ++i) {
> -		const ImmovableDescr& descr = *world.get_immovable_descr(i);
> -		if (descr.has_attribute(descr.get_attribute_id("artifact"))) {
> -			return true;
> +bool Map::has_artifacts() {

It is more efficient to just iterate all fields. It is also more readable since you do not care about the coordinates anyways

> +	for (FCoords c(Coords(0, 0), m_fields.get()); c.y < m_height; ++c.y) {
> +		for (c.x = 0; c.x < m_width; ++c.x, ++c.field) {
> +			assert(c.field == &operator[] (c));
> +			if (upcast(Immovable, immovable, c.field->get_immovable())) {
> +				if (immovable->descr().has_attribute(immovable->descr().get_attribute_id("artifact"))) {
> +					return true;
> +				}
> +			}
>  		}
>  	}
>  	return false;


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1525706-artifacts/+merge/285566
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1525706-artifacts.