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.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
