widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

Some replies

Diff comments:

> 
> === modified file 'src/wui/fieldaction.cc'
> --- src/wui/fieldaction.cc	2019-02-23 11:00:49 +0000
> +++ src/wui/fieldaction.cc	2019-04-28 16:58:57 +0000
> @@ -683,17 +695,99 @@
>  
>  void FieldActionWindow::building_icon_mouse_out(Widelands::DescriptionIndex) {
>  	if (showing_workarea_preview_) {
> -		ibase().hide_workarea(node_);
> +		ibase().hide_workarea(node_, false);
> +		clear_overlapping_workareas();
>  		showing_workarea_preview_ = false;
>  	}
>  }
>  
>  void FieldActionWindow::building_icon_mouse_in(const Widelands::DescriptionIndex idx) {
>  	if (!showing_workarea_preview_) {
> -		const WorkareaInfo& workarea_info =
> -		   player_->tribe().get_building_descr(Widelands::DescriptionIndex(idx))->workarea_info();
> +		assert(overlapping_workareas_.empty());
> +		const Widelands::BuildingDescr& descr = *player_->tribe().get_building_descr(idx);
> +		const WorkareaInfo& workarea_info = descr.workarea_info();
>  		ibase().show_workarea(workarea_info, node_);
>  		showing_workarea_preview_ = true;
> +
> +		const Widelands::Map& map = ibase().egbase().map();
> +		uint32_t workarea_radius = 0;
> +		for (const auto& pair : workarea_info) {
> +			workarea_radius = std::max(workarea_radius, pair.first);
> +		}
> +		if (workarea_radius == 0) {
> +			return;
> +		}
> +		std::set<Widelands::TCoords<>> main_region = map.triangles_in_region(
> +				map.to_set(Widelands::Area<>(node_, workarea_radius)));
> +
> +		Widelands::MapRegion<Widelands::Area<Widelands::FCoords>> mr(map, Widelands::Area<Widelands::FCoords>(
> +				node_, workarea_radius + ibase().egbase().tribes().get_largest_workarea()));
> +		do {
> +			if (player_->vision(map.get_index(mr.location())) > 1) {
> +				if (Widelands::BaseImmovable* imm = mr.location().field->get_immovable()) {
> +					const Widelands::MapObjectType imm_type = imm->descr().type();
> +					if (imm_type < Widelands::MapObjectType::BUILDING) {
> +						// We are not interested in trees and pebbles
> +						continue;
> +					}
> +					const Widelands::BuildingDescr* d = nullptr;
> +					if (descr.type() == Widelands::MapObjectType::PRODUCTIONSITE) {
> +						if (imm->get_owner() != player_) {
> +							continue;
> +						}
> +						if (imm_type != Widelands::MapObjectType::PRODUCTIONSITE) {

You're doing it for the remaining building types again below though - If I see it from the diff here correctly, the only difference is the owner check. So, only do the owner check here and do the common checks below?

> +							if (imm_type != Widelands::MapObjectType::CONSTRUCTIONSITE) {
> +								continue;
> +							}
> +							upcast(Widelands::ConstructionSite, cs, imm);
> +							d = cs->get_info().becomes;
> +							if (d->type() != Widelands::MapObjectType::PRODUCTIONSITE) {
> +								continue;
> +							}
> +						}
> +					} else if (descr.type() == Widelands::MapObjectType::WAREHOUSE ||
> +							descr.type() == Widelands::MapObjectType::MILITARYSITE) {
> +						if (imm_type != Widelands::MapObjectType::MILITARYSITE &&
> +								imm_type != Widelands::MapObjectType::WAREHOUSE) {
> +							if (imm_type != Widelands::MapObjectType::CONSTRUCTIONSITE) {
> +								continue;
> +							}
> +							upcast(Widelands::ConstructionSite, cs, imm);
> +							d = cs->get_info().becomes;
> +							if (d->type() != Widelands::MapObjectType::WAREHOUSE &&
> +									d->type() != Widelands::MapObjectType::MILITARYSITE) {
> +								continue;
> +							}
> +						}
> +					}
> +					upcast(Widelands::Building, bld, imm);
> +					if (bld->get_position() != mr.location()) {
> +						// Don't count big buildings more than once
> +						continue;
> +					}
> +					if (!d) {
> +						d = &bld->descr();
> +					}
> +					const WorkareaInfo& wa = d->workarea_info();
> +					uint32_t wa_radius = 0;
> +					for (const auto& pair : wa) {
> +						wa_radius = std::max(wa_radius, pair.first);
> +					}
> +					if (wa_radius == 0) {
> +						continue;
> +					}
> +					if (map.calc_distance(node_, mr.location()) <= workarea_radius + wa_radius) {
> +						std::map<Widelands::TCoords<>, uint32_t> colors;
> +						for (const Widelands::TCoords<>& t : map.triangles_in_region(
> +								map.to_set(Widelands::Area<>(mr.location(), wa_radius)))) {
> +							colors[t] = main_region.count(t) ? 0xbf7f0000 : 0x3fffffff;
> +						}
> +						ibase().show_workarea(wa, mr.location(), colors);
> +						overlapping_workareas_.insert(mr.location());
> +					}
> +				}
> +			}
> +		} while (mr.advance(map));
>  	}
>  }
>  
> 
> === modified file 'src/wui/interactive_base.cc'
> --- src/wui/interactive_base.cc	2019-04-25 06:31:33 +0000
> +++ src/wui/interactive_base.cc	2019-04-28 16:58:57 +0000
> @@ -196,14 +197,19 @@
>  bool InteractiveBase::has_workarea_preview(const Widelands::Coords& coords,
>                                             const Widelands::Map* map) const {
>  	if (!map) {
> -		return workarea_previews_.count(coords) == 1;
> +		for (const auto& it : workarea_previews_) {
> +			if (it->coords == coords) {
> +				return true;
> +			}
> +		}
> +		return false;
>  	}
> -	for (const auto& pair : workarea_previews_) {
> +	for (const auto& it : workarea_previews_) {

It's still not an iterator, but an entry though. The difference in variable name is important for readability.

for (const auto& preview : workarea_previews_) {

Or some such would work better.

>  		uint32_t radius = 0;
> -		for (const auto& p : *pair.second) {
> -			radius = std::max(radius, p.first);
> +		for (const auto& wa : *it->info) {
> +			radius = std::max(radius, wa.first);
>  		}
> -		if (map->calc_distance(coords, pair.first) <= radius) {
> +		if (map->calc_distance(coords, it->coords) <= radius) {
>  			return true;
>  		}
>  	}
> @@ -350,73 +364,100 @@
>  	}
>  }
>  
> -Workareas InteractiveBase::get_workarea_overlays(const Widelands::Map& map) const {
> -	Workareas result_set;
> -	for (const auto& wa_pair : workarea_previews_) {
> -		std::map<Coords, uint8_t> intermediate_result;
> -		const Coords& coords = wa_pair.first;
> -		const WorkareaInfo* workarea_info = wa_pair.second;
> -		intermediate_result[coords] = 0;
> -		WorkareaInfo::size_type wa_index;
> -		switch (workarea_info->size()) {
> -		case 0:
> -			continue;  // no workarea
> -		case 1:
> -			wa_index = 5;
> -			break;
> -		case 2:
> -			wa_index = 3;
> -			break;
> -		case 3:
> -			wa_index = 0;
> -			break;
> -		default:
> -			throw wexception(
> -			   "Encountered unexpected WorkareaInfo size %i", static_cast<int>(workarea_info->size()));
> -		}
> -
> -		Widelands::HollowArea<> hollow_area(Widelands::Area<>(coords, 0), 0);
> -
> -		// Iterate through the work areas, from building to its enhancement
> -		WorkareaInfo::const_iterator it = workarea_info->begin();
> -		for (; it != workarea_info->end(); ++it) {
> -			hollow_area.radius = it->first;
> -			Widelands::MapHollowRegion<> mr(map, hollow_area);
> -			do {
> -				intermediate_result[mr.location()] = wa_index;
> -			} while (mr.advance(map));
> -			wa_index++;
> -			hollow_area.hole_radius = hollow_area.radius;
> -		}
> -
> -		std::map<TCoords<>, uint8_t> result;
> -		for (const auto& pair : intermediate_result) {
> -			Coords c;
> -			map.get_brn(pair.first, &c);
> -			const auto brn = intermediate_result.find(c);
> -			if (brn == intermediate_result.end()) {
> -				continue;
> -			}
> -			map.get_bln(pair.first, &c);
> -			const auto bln = intermediate_result.find(c);
> -			map.get_rn(pair.first, &c);
> -			const auto rn = intermediate_result.find(c);
> -			if (bln != intermediate_result.end()) {
> -				result[TCoords<>(pair.first, Widelands::TriangleIndex::D)] =
> -				   workarea_max(pair.second, brn->second, bln->second);
> -			}
> -			if (rn != intermediate_result.end()) {
> -				result[TCoords<>(pair.first, Widelands::TriangleIndex::R)] =
> -				   workarea_max(pair.second, brn->second, rn->second);
> -			}
> -		}
> -		result_set.emplace(result);
> -	}
> -	return result_set;
> -}
> -
> -void InteractiveBase::hide_workarea(const Widelands::Coords& coords) {
> -	workarea_previews_.erase(coords);
> +Workareas InteractiveBase::get_workarea_overlays(const Widelands::Map& map) {
> +	if (!workareas_cache_) {
> +		workareas_cache_.reset(new Workareas());
> +		for (const auto& it : workarea_previews_) {
> +			workareas_cache_->push_back(get_workarea_overlay(map, *it));
> +		}
> +	}
> +	return Workareas(*workareas_cache_);
> +}
> +
> +// static

Ah, I didn't compare to the header file, duh! Never mind then.

> +WorkareasEntry InteractiveBase::get_workarea_overlay(const Widelands::Map& map, const WorkareaPreview& workarea) {
> +	std::map<Coords, uint8_t> intermediate_result;
> +	const Coords& coords = workarea.coords;
> +	const WorkareaInfo* workarea_info = workarea.info;
> +	intermediate_result[coords] = 0;
> +	WorkareaInfo::size_type wa_index;
> +	switch (workarea_info->size()) {
> +	case 0:
> +		return WorkareasEntry();  // no workarea
> +	case 1:
> +		wa_index = 5;
> +		break;
> +	case 2:
> +		wa_index = 3;
> +		break;
> +	case 3:
> +		wa_index = 0;
> +		break;
> +	default:
> +		throw wexception(
> +		   "Encountered unexpected WorkareaInfo size %i", static_cast<int>(workarea_info->size()));
> +	}
> +
> +	Widelands::HollowArea<> hollow_area(Widelands::Area<>(coords, 0), 0);
> +
> +	// Iterate through the work areas, from building to its enhancement
> +	WorkareaInfo::const_iterator it = workarea_info->begin();
> +	for (; it != workarea_info->end(); ++it) {
> +		hollow_area.radius = it->first;
> +		Widelands::MapHollowRegion<> mr(map, hollow_area);
> +		do {
> +			intermediate_result[mr.location()] = wa_index;
> +		} while (mr.advance(map));
> +		wa_index++;
> +		hollow_area.hole_radius = hollow_area.radius;
> +	}
> +
> +	WorkareasEntry result;
> +	for (const auto& pair : intermediate_result) {
> +		Coords c;
> +		map.get_brn(pair.first, &c);
> +		const auto brn = intermediate_result.find(c);
> +		if (brn == intermediate_result.end()) {
> +			continue;
> +		}
> +		map.get_bln(pair.first, &c);
> +		const auto bln = intermediate_result.find(c);
> +		map.get_rn(pair.first, &c);
> +		const auto rn = intermediate_result.find(c);
> +		if (bln != intermediate_result.end()) {
> +			TCoords<> tc(pair.first, Widelands::TriangleIndex::D);
> +			WorkareaPreviewData wd(tc, workarea_max(pair.second, brn->second, bln->second));
> +			for (const auto& p : workarea.data) {
> +				if (p.first == tc) {
> +					wd = WorkareaPreviewData(tc, wd.index, p.second);
> +					break;
> +				}
> +			}
> +			result.push_back(wd);
> +		}
> +		if (rn != intermediate_result.end()) {
> +			TCoords<> tc(pair.first, Widelands::TriangleIndex::R);
> +			WorkareaPreviewData wd(tc, workarea_max(pair.second, brn->second, rn->second));
> +			for (const auto& p : workarea.data) {
> +				if (p.first == tc) {
> +					wd = WorkareaPreviewData(tc, wd.index, p.second);
> +					break;
> +				}
> +			}
> +			result.push_back(wd);
> +		}
> +	}
> +	return result;
> +}
> +
> +void InteractiveBase::hide_workarea(const Widelands::Coords& coords, bool is_additional) {
> +	for (auto it = workarea_previews_.begin(); it != workarea_previews_.end(); ++it) {
> +		if (it->get()->coords == coords && (is_additional ^ it->get()->data.empty())) {
> +			workarea_previews_.erase(it);
> +			workareas_cache_.reset(nullptr);
> +			return;
> +		}
> +	}
>  }
>  
>  /**


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