widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

2 comments for code readability

Diff comments:

> 
> === modified file 'src/logic/map_objects/tribes/tribes.cc'
> --- src/logic/map_objects/tribes/tribes.cc	2019-05-29 06:24:42 +0000
> +++ src/logic/map_objects/tribes/tribes.cc	2019-06-05 14:12:33 +0000
> @@ -335,6 +335,21 @@
>  			for (const auto& job : de->working_positions()) {
>  				workers_->get_mutable(job.first)->add_employer(i);
>  			}
> +
> +			for (const auto& pair : de->get_highlight_overlapping_workarea_for()) {

Please add a comment above this loop to describe what it's for.

> +				const DescriptionIndex di = safe_building_index(pair.first);
> +				if (upcast(const ProductionSiteDescr, p, get_building_descr(di))) {
> +					if (!p->workarea_info().empty()) {
> +						continue;
> +					}
> +					throw GameDataError(
> +							"Productionsite %s will inform about conflicting building %s which doesn’t have a workarea",
> +							de->name().c_str(), pair.first.c_str());
> +				}
> +				throw GameDataError(
> +						"Productionsite %s will inform about conflicting building %s which is not a productionsite",
> +						de->name().c_str(), pair.first.c_str());
> +			}
>  		}
>  
>  		// Register which buildings buildings can have been enhanced from
> 
> === modified file 'src/wui/fieldaction.cc'
> --- src/wui/fieldaction.cc	2019-05-31 19:31:45 +0000
> +++ src/wui/fieldaction.cc	2019-06-05 14:12:33 +0000
> @@ -768,11 +779,14 @@
>  						continue;
>  					}
>  					const Widelands::BuildingDescr* d = nullptr;
> +					bool positive;

Turn this into bool& again and then pass bool* to the functions. This will make the code easier to read.

>  					if (imm_type == Widelands::MapObjectType::CONSTRUCTIONSITE) {
>  						upcast(Widelands::ConstructionSite, cs, imm);
>  						d = cs->get_info().becomes;
>  						if ((descr.type() == Widelands::MapObjectType::PRODUCTIONSITE &&
> -						     d->type() != Widelands::MapObjectType::PRODUCTIONSITE) ||
> +						     (d->type() != Widelands::MapObjectType::PRODUCTIONSITE ||
> +						     !dynamic_cast<const Widelands::ProductionSiteDescr&>(descr).
> +						     		highlight_overlapping_workarea_for(d->name(), positive))) ||
>  						    ((descr.type() == Widelands::MapObjectType::MILITARYSITE ||
>  						      descr.type() == Widelands::MapObjectType::WAREHOUSE) &&
>  						     imm_type != Widelands::MapObjectType::MILITARYSITE &&


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