Codechange: simplify some complicated logic

src/gfxinit.cpp

@@ -481,18 +481,27 @@ template <class Tbase_set>
 	if (BaseMedia<Tbase_set>::used_set != nullptr) return true;
 
 	const Tbase_set *best = nullptr;
+
+	auto IsBetter = [&best] (const auto *current) {
+		/* Nothing chosen yet. */
+		if (best == nullptr) return true;
+		/* Not being a fallback is better. */
+		if (best->fallback && !current->fallback) return true;
+		/* Having more valid files is better. */
+		if (best->valid_files < current->valid_files) return true;
+		/* Having (essentially) fewer valid files is worse. */
+		if (best->valid_files != current->valid_files) return false;
+		/* Having a later version of the same base set is better. */
+		if (best->shortname == current->shortname && best->version < current->version) return true;
+		/* The DOS palette is the better palette. */
+		return best->palette != PAL_DOS && current->palette == PAL_DOS;
+	};
+
 	for (const Tbase_set *c = BaseMedia<Tbase_set>::available_sets; c != nullptr; c = c->next) {
 		/* Skip unusable sets */
 		if (c->GetNumMissing() != 0) continue;
 
-		if (best == nullptr ||
-				(best->fallback && !c->fallback) ||
-				best->valid_files < c->valid_files ||
-				(best->valid_files == c->valid_files && (
-					(best->shortname == c->shortname && best->version < c->version) ||
-					(best->palette != PAL_DOS && c->palette == PAL_DOS)))) {
-			best = c;
-		}
+		if (IsBetter(c)) best = c;
 	}
 
 	BaseMedia<Tbase_set>::used_set = best;

src/roadveh_cmd.cpp

@@ -620,21 +620,24 @@ static Vehicle *EnumCheckRoadVehClose(Vehicle *v, void *data)
 	short x_diff = v->x_pos - rvf->x;
 	short y_diff = v->y_pos - rvf->y;
 
-	if (v->type == VEH_ROAD &&
-			!v->IsInDepot() &&
-			abs(v->z_pos - rvf->veh->z_pos) < 6 &&
-			v->direction == rvf->dir &&
-			rvf->veh->First() != v->First() &&
-			(dist_x[v->direction] >= 0 || (x_diff > dist_x[v->direction] && x_diff <= 0)) &&
-			(dist_x[v->direction] <= 0 || (x_diff < dist_x[v->direction] && x_diff >= 0)) &&
-			(dist_y[v->direction] >= 0 || (y_diff > dist_y[v->direction] && y_diff <= 0)) &&
-			(dist_y[v->direction] <= 0 || (y_diff < dist_y[v->direction] && y_diff >= 0))) {
-		uint diff = abs(x_diff) + abs(y_diff);
+	/* Not a close Road vehicle when it's not a road vehicle, in the depot, or ourself. */
+	if (v->type != VEH_ROAD || v->IsInDepot() || rvf->veh->First() == v->First()) return nullptr;
 
-		if (diff < rvf->best_diff || (diff == rvf->best_diff && v->index < rvf->best->index)) {
-			rvf->best = v;
-			rvf->best_diff = diff;
-		}
+	/* Not close when at a different height or when going in a different direction. */
+	if (abs(v->z_pos - rvf->veh->z_pos) >= 6 || v->direction != rvf->dir) return nullptr;
+
+	/* We 'return' the closest vehicle, in distance and then VehicleID as tie-breaker. */
+	uint diff = abs(x_diff) + abs(y_diff);
+	if (diff > rvf->best_diff || (diff == rvf->best_diff && v->index > rvf->best->index)) return nullptr;
+
+	auto IsCloseOnAxis = [](short dist, short diff) {
+		if (dist < 0) return diff > dist && diff <= 0;
+		return diff < dist && diff >= 0;
+	};
+
+	if (IsCloseOnAxis(dist_x[v->direction], x_diff) && IsCloseOnAxis(dist_y[v->direction], y_diff)) {
+		rvf->best = v;
+		rvf->best_diff = diff;
 	}
 
 	return nullptr;

src/script/api/script_goal.cpp

@@ -30,15 +30,24 @@
 
 /* static */ bool ScriptGoal::IsValidGoalDestination(ScriptCompany::CompanyID company, GoalType type, SQInteger destination)
 {
-	::CompanyID c = ScriptCompany::FromScriptCompanyID(company);
-	StoryPage *story_page = nullptr;
-	if (type == GT_STORY_PAGE && ScriptStoryPage::IsValidStoryPage(static_cast<StoryPageID>(destination))) story_page = ::StoryPage::Get(static_cast<StoryPageID>(destination));
-	return (type == GT_NONE && destination == 0) ||
-			(type == GT_TILE && ScriptMap::IsValidTile(::TileIndex(destination))) ||
-			(type == GT_INDUSTRY && ScriptIndustry::IsValidIndustry(static_cast<IndustryID>(destination))) ||
-			(type == GT_TOWN && ScriptTown::IsValidTown(static_cast<TownID>(destination))) ||
-			(type == GT_COMPANY && ScriptCompany::ResolveCompanyID(ScriptCompany::ToScriptCompanyID(static_cast<::CompanyID>(destination))) != ScriptCompany::COMPANY_INVALID) ||
-			(type == GT_STORY_PAGE && story_page != nullptr && (c == CompanyID::Invalid() ? story_page->company == CompanyID::Invalid() : story_page->company == CompanyID::Invalid() || story_page->company == c));
+	switch (type) {
+		case GT_NONE: return destination == 0;
+		case GT_TILE: return ScriptMap::IsValidTile(::TileIndex(destination));
+		case GT_INDUSTRY: return ScriptIndustry::IsValidIndustry(static_cast<IndustryID>(destination));
+		case GT_TOWN: return ScriptTown::IsValidTown(static_cast<TownID>(destination));
+		case GT_COMPANY: return ScriptCompany::ResolveCompanyID(ScriptCompany::ToScriptCompanyID(static_cast<::CompanyID>(destination))) != ScriptCompany::COMPANY_INVALID;
+		case GT_STORY_PAGE: {
+			if (!ScriptStoryPage::IsValidStoryPage(static_cast<StoryPageID>(destination))) return false;
+
+			StoryPage *story_page = ::StoryPage::Get(static_cast<StoryPageID>(destination));
+			if (story_page->company == CompanyID::Invalid()) return true;
+
+			::CompanyID c = ScriptCompany::FromScriptCompanyID(company);
+			return c != CompanyID::Invalid() && story_page->company == c;
+		}
+
+		default: return false;
+	}
 }
 
 /* static */ GoalID ScriptGoal::New(ScriptCompany::CompanyID company, Text *goal, GoalType type, SQInteger destination)