(svn r6047) -Codechange: FOR_ALL now _only_ loops valid items, and skips invalid ones

-Codechange: use IsValidXXX where ever possible
  Note: both changes to prepare for new pool system, which needs those changes.
  For every pool there are 2 ugly lines, which will be removed when done
  implementing new pool system.
  Based on FS#13 by blathijs, partly implemented.

ai/default/default.c

@@ -90,7 +90,7 @@ static void AiStateVehLoop(Player *p)
 	index = (p->ai.cur_veh == NULL) ? 0 : p->ai.cur_veh->index + 1;
 
 	FOR_ALL_VEHICLES_FROM(v, index) {
-		if (v->type == 0 || v->owner != _current_player) continue;
+		if (v->owner != _current_player) continue;
 
 		if ((v->type == VEH_Train && v->subtype == 0) ||
 				v->type == VEH_Road ||
@@ -411,7 +411,7 @@ static void AiStateCheckReplaceVehicle(Player *p)
 {
 	const Vehicle* v = p->ai.cur_veh;
 
-	if (v->type == 0 ||
+	if (!IsValidVehicle(v) ||
 			v->owner != _current_player ||
 			v->type > VEH_Ship ||
 			_veh_check_replace_proc[v->type - VEH_Train](p, v) == INVALID_ENGINE) {
@@ -428,7 +428,7 @@ static void AiStateDoReplaceVehicle(Player *p)
 
 	p->ai.state = AIS_VEH_LOOP;
 	// vehicle is not owned by the player anymore, something went very wrong.
-	if (v->type == 0 || v->owner != _current_player) return;
+	if (!IsValidVehicle(v) || v->owner != _current_player) return;
 	_veh_do_replace_proc[v->type - VEH_Train](p);
 }
 
@@ -442,13 +442,13 @@ typedef struct FoundRoute {
 static Town *AiFindRandomTown(void)
 {
 	Town *t = GetTown(RandomRange(_total_towns));
-	return (t->xy != 0) ? t : NULL;
+	return IsValidTown(t) ? t : NULL;
 }
 
 static Industry *AiFindRandomIndustry(void)
 {
 	Industry *i = GetIndustry(RandomRange(_total_industries));
-	return (i->xy != 0) ? i : NULL;
+	return IsValidIndustry(i) ? i : NULL;
 }
 
 static void AiFindSubsidyIndustryRoute(FoundRoute *fr)
@@ -608,7 +608,7 @@ static bool AiCheckIfRouteIsGood(Player *p, FoundRoute *fr, byte bitmask)
 	FOR_ALL_STATIONS(st) {
 		int cur;
 
-		if (st->xy == 0 || st->owner != _current_player) continue;
+		if (st->owner != _current_player) continue;
 		cur = DistanceMax(from_tile, st->xy);
 		if (cur < dist) dist = cur;
 		cur = DistanceMax(to_tile, st->xy);
@@ -3243,9 +3243,6 @@ static void AiStateAirportStuff(Player *p)
 		aib = &p->ai.src + i;
 
 		FOR_ALL_STATIONS(st) {
-			// Dismiss ghost stations.
-			if (st->xy == 0) continue;
-
 			// Is this an airport?
 			if (!(st->facilities & FACIL_AIRPORT)) continue;
 
@@ -3578,7 +3575,7 @@ static void AiStateRemoveStation(Player *p)
 	// Go through all stations and delete those that aren't in use
 	used = in_use;
 	FOR_ALL_STATIONS(st) {
-		if (st->xy != 0 && st->owner == _current_player && !*used &&
+		if (st->owner == _current_player && !*used &&
 				( (st->bus_stops != NULL && (tile = st->bus_stops->xy) != 0) ||
 					(st->truck_stops != NULL && (tile = st->truck_stops->xy)) != 0 ||
 					(tile = st->train_tile) != 0 ||

ai/trolly/trolly.c

@@ -231,8 +231,6 @@ static bool AiNew_Check_City_or_Industry(Player *p, int ic, byte type)
 		//  and sometimes it takes up to 4 months before the stats are corectly.
 		//  This way we don't get 12 busstations in one city of 100 population ;)
 		FOR_ALL_STATIONS(st) {
-			// Is it an active station
-			if (st->xy == 0) continue;
 			// Do we own it?
 			if (st->owner == _current_player) {
 				// Are we talking busses?
@@ -291,9 +289,6 @@ static bool AiNew_Check_City_or_Industry(Player *p, int ic, byte type)
 		//  else we don't do it. This is done, because stat updates can be slow
 		//  and sometimes it takes up to 4 months before the stats are corectly.
 		FOR_ALL_STATIONS(st) {
-			// Is it an active station
-			if (st->xy == 0) continue;
-
 			// Do we own it?
 			if (st->owner == _current_player) {
 				// Are we talking trucks?
@@ -620,21 +615,19 @@ static void AiNew_State_FindStation(Player *p)
 	}
 
 	FOR_ALL_STATIONS(st) {
-		if (st->xy != 0) {
-			if (st->owner == _current_player) {
-				if (p->ainew.tbt == AI_BUS && (FACIL_BUS_STOP & st->facilities) == FACIL_BUS_STOP) {
-					if (st->town == town) {
-						// Check how much cargo there is left in the station
-						if ((st->goods[p->ainew.cargo].waiting_acceptance & 0xFFF) > RoadVehInfo(i)->capacity * AI_STATION_REUSE_MULTIPLER) {
-							if (AiNew_CheckVehicleStation(p, st)) {
-								// We did found a station that was good enough!
-								new_tile = st->xy;
-								direction = GetRoadStopDir(st->xy);
-								break;
-							}
+		if (st->owner == _current_player) {
+			if (p->ainew.tbt == AI_BUS && (FACIL_BUS_STOP & st->facilities) == FACIL_BUS_STOP) {
+				if (st->town == town) {
+					// Check how much cargo there is left in the station
+					if ((st->goods[p->ainew.cargo].waiting_acceptance & 0xFFF) > RoadVehInfo(i)->capacity * AI_STATION_REUSE_MULTIPLER) {
+						if (AiNew_CheckVehicleStation(p, st)) {
+							// We did found a station that was good enough!
+							new_tile = st->xy;
+							direction = GetRoadStopDir(st->xy);
+							break;
 						}
-						count++;
 					}
+					count++;
 				}
 			}
 		}
@@ -1302,7 +1295,6 @@ static void AiNew_State_CheckAllVehicles(Player *p)
 	Vehicle *v;
 
 	FOR_ALL_VEHICLES(v) {
-		if (v->type == 0) continue;
 		if (v->owner != p->index) continue;
 		// Currently, we only know how to handle road-vehicles
 		if (v->type != VEH_Road) continue;

aircraft_cmd.c

@@ -652,7 +652,7 @@ static void CheckIfAircraftNeedsService(Vehicle *v)
 
 	st = GetStation(v->current_order.station);
 	// only goto depot if the target airport has terminals (eg. it is airport)
-	if (st->xy != 0 && st->airport_tile != 0 && GetAirport(st->airport_type)->terminals != NULL) {
+	if (IsValidStation(st) && st->airport_tile != 0 && GetAirport(st->airport_type)->terminals != NULL) {
 //		printf("targetairport = %d, st->index = %d\n", v->u.air.targetairport, st->index);
 //		v->u.air.targetairport = st->index;
 		v->current_order.type = OT_GOTO_DEPOT;

console_cmds.c

@@ -141,15 +141,13 @@ DEF_CONSOLE_CMD(ConStopAllVehicles)
 	}
 
 	FOR_ALL_VEHICLES(v) {
-		if (IsValidVehicle(v)) {
-			/* Code ripped from CmdStartStopTrain. Can't call it, because of
-			 * ownership problems, so we'll duplicate some code, for now */
-			if (v->type == VEH_Train)
-				v->u.rail.days_since_order_progr = 0;
-			v->vehstatus |= VS_STOPPED;
-			InvalidateWindowWidget(WC_VEHICLE_VIEW, v->index, STATUS_BAR);
-			InvalidateWindow(WC_VEHICLE_DEPOT, v->tile);
-		}
+		/* Code ripped from CmdStartStopTrain. Can't call it, because of
+		 * ownership problems, so we'll duplicate some code, for now */
+		if (v->type == VEH_Train)
+			v->u.rail.days_since_order_progr = 0;
+		v->vehstatus |= VS_STOPPED;
+		InvalidateWindowWidget(WC_VEHICLE_VIEW, v->index, STATUS_BAR);
+		InvalidateWindow(WC_VEHICLE_DEPOT, v->tile);
 	}
 	return true;
 }

date.c

@@ -202,7 +202,7 @@ static void RunVehicleDayProc(uint daytick)
 	for (i = daytick; i < total; i += DAY_TICKS) {
 		Vehicle *v = GetVehicle(i);
 
-		if (v->type != 0) _on_new_vehicle_day_proc[v->type - 0x10](v);
+		if (IsValidVehicle(v)) _on_new_vehicle_day_proc[v->type - 0x10](v);
 	}
 }
 

depot.c

@@ -21,10 +21,11 @@ enum {
  */
 static void DepotPoolNewBlock(uint start_item)
 {
-	Depot *depot;
+	Depot *d;
 
-	FOR_ALL_DEPOTS_FROM(depot, start_item)
-		depot->index = start_item++;
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (d = GetDepot(start_item); d != NULL; d = (d->index + 1 < GetDepotPoolSize()) ? GetDepot(d->index + 1) : NULL) d->index = start_item++;
 }
 
 /* Initialize the town-pool */
@@ -52,16 +53,18 @@ Depot *GetDepotByTile(TileIndex tile)
  */
 Depot *AllocateDepot(void)
 {
-	Depot *depot;
+	Depot *d;
 
-	FOR_ALL_DEPOTS(depot) {
-		if (!IsValidDepot(depot)) {
-			uint index = depot->index;
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (d = GetDepot(0); d != NULL; d = (d->index + 1 < GetDepotPoolSize()) ? GetDepot(d->index + 1) : NULL) {
+		if (!IsValidDepot(d)) {
+			uint index = d->index;
 
-			memset(depot, 0, sizeof(Depot));
-			depot->index = index;
+			memset(d, 0, sizeof(Depot));
+			d->index = index;
 
-			return depot;
+			return d;
 		}
 	}
 
@@ -116,10 +119,8 @@ static void Save_DEPT(void)
 	Depot *depot;
 
 	FOR_ALL_DEPOTS(depot) {
-		if (IsValidDepot(depot)) {
-			SlSetArrayIndex(depot->index);
-			SlObject(depot, _depot_desc);
-		}
+		SlSetArrayIndex(depot->index);
+		SlObject(depot, _depot_desc);
 	}
 }
 

depot.h

@@ -40,7 +40,15 @@ static inline bool IsDepotIndex(uint index)
 	return index < GetDepotPoolSize();
 }
 
-#define FOR_ALL_DEPOTS_FROM(d, start) for (d = GetDepot(start); d != NULL; d = (d->index + 1 < GetDepotPoolSize()) ? GetDepot(d->index + 1) : NULL)
+/**
+ * Check if a depot really exists.
+ */
+static inline bool IsValidDepot(const Depot* depot)
+{
+	return depot->xy != 0;
+}
+
+#define FOR_ALL_DEPOTS_FROM(d, start) for (d = GetDepot(start); d != NULL; d = (d->index + 1 < GetDepotPoolSize()) ? GetDepot(d->index + 1) : NULL) if (IsValidDepot(d))
 #define FOR_ALL_DEPOTS(d) FOR_ALL_DEPOTS_FROM(d, 0)
 
 #define MIN_SERVINT_PERCENT  5
@@ -58,15 +66,6 @@ static inline Date GetServiceIntervalClamped(uint index)
 	return (_patches.servint_ispercent) ? clamp(index, MIN_SERVINT_PERCENT, MAX_SERVINT_PERCENT) : clamp(index, MIN_SERVINT_DAYS, MAX_SERVINT_DAYS);
 }
 
-
-/**
- * Check if a depot really exists.
- */
-static inline bool IsValidDepot(const Depot* depot)
-{
-	return depot->xy != 0; /* XXX: Replace by INVALID_TILE someday */
-}
-
 /**
  * Check if a tile is a depot of the given type.
  */

disaster_cmd.c

@@ -724,7 +724,7 @@ static void Disaster0_Init(void)
 	x = TileX(Random()) * TILE_SIZE + TILE_SIZE / 2;
 
 	FOR_ALL_STATIONS(st) {
-		if (st->xy && st->airport_tile != 0 &&
+		if (st->airport_tile != 0 &&
 				st->airport_type <= 1 &&
 				IS_HUMAN_PLAYER(st->owner)) {
 			x = (TileX(st->xy) + 2) * TILE_SIZE;
@@ -774,8 +774,7 @@ static void Disaster2_Init(void)
 	found = NULL;
 
 	FOR_ALL_INDUSTRIES(i) {
-		if (i->xy != 0 &&
-				i->type == IT_OIL_REFINERY &&
+		if (i->type == IT_OIL_REFINERY &&
 				(found == NULL || CHANCE16(1, 2))) {
 			found = i;
 		}
@@ -808,8 +807,7 @@ static void Disaster3_Init(void)
 	found = NULL;
 
 	FOR_ALL_INDUSTRIES(i) {
-		if (i->xy != 0 &&
-				i->type == IT_FACTORY &&
+		if (i->type == IT_FACTORY &&
 				(found==NULL || CHANCE16(1,2))) {
 			found = i;
 		}
@@ -919,7 +917,7 @@ static void Disaster7_Init(void)
 		const Industry* i;
 
 		FOR_ALL_INDUSTRIES(i) {
-			if (i->xy != 0 && i->type == IT_COAL_MINE && --index < 0) {
+			if (i->type == IT_COAL_MINE && --index < 0) {
 				SetDParam(0, i->town->index);
 				AddNewsItem(STR_B005_COAL_MINE_SUBSIDENCE_LEAVES,
 					NEWS_FLAGS(NM_THIN,NF_VIEWPORT|NF_TILE,NT_ACCIDENT,0), i->xy + TileDiffXY(1, 1), 0);

economy.c

@@ -57,7 +57,7 @@ int64 CalculateCompanyValue(const Player* p)
 		uint num = 0;
 
 		FOR_ALL_STATIONS(st) {
-			if (st->xy != 0 && st->owner == owner) {
+			if (st->owner == owner) {
 				uint facil = st->facilities;
 				do num += (facil&1); while (facil >>= 1);
 			}
@@ -70,8 +70,8 @@ int64 CalculateCompanyValue(const Player* p)
 		Vehicle *v;
 
 		FOR_ALL_VEHICLES(v) {
-			if (v->owner != owner)
-				continue;
+			if (v->owner != owner) continue;
+
 			if (v->type == VEH_Train ||
 					v->type == VEH_Road ||
 					(v->type == VEH_Aircraft && v->subtype<=2) ||
@@ -133,7 +133,7 @@ int UpdateCompanyRatingAndValue(Player *p, bool update)
 		const Station* st;
 
 		FOR_ALL_STATIONS(st) {
-			if (st->xy != 0 && st->owner == owner) {
+			if (st->owner == owner) {
 				int facil = st->facilities;
 				do num += facil&1; while (facil>>=1);
 			}
@@ -266,7 +266,7 @@ void ChangeOwnershipOfPlayerItems(PlayerID old_player, PlayerID new_player)
 		Town *t;
 		FOR_ALL_TOWNS(t) {
 			/* If a player takes over, give the ratings to that player. */
-			if (IsValidTown(t) && HASBIT(t->have_ratings, old_player)) {
+			if (HASBIT(t->have_ratings, old_player)) {
 				if (HASBIT(t->have_ratings, new_player)) {
 					// use max of the two ratings.
 					t->ratings[new_player] = max(t->ratings[new_player], t->ratings[old_player]);
@@ -276,11 +276,8 @@ void ChangeOwnershipOfPlayerItems(PlayerID old_player, PlayerID new_player)
 				}
 			}
 
-			/* Reset ratings for the town */
-			if (IsValidTown(t)) {
-				t->ratings[old_player] = 500;
-				CLRBIT(t->have_ratings, old_player);
-			}
+			t->ratings[old_player] = 500;
+			CLRBIT(t->have_ratings, old_player);
 		}
 	}
 
@@ -573,11 +570,9 @@ static void PlayersGenStatistics(void)
 	Player *p;
 
 	FOR_ALL_STATIONS(st) {
-		if (st->xy != 0) {
-			_current_player = st->owner;
-			SET_EXPENSES_TYPE(EXPENSES_PROPERTY);
-			SubtractMoneyFromPlayer(_price.station_value >> 1);
-		}
+		_current_player = st->owner;
+		SET_EXPENSES_TYPE(EXPENSES_PROPERTY);
+		SubtractMoneyFromPlayer(_price.station_value >> 1);
 	}
 
 	if (!HASBIT(1<<0|1<<3|1<<6|1<<9, _cur_month))
@@ -888,11 +883,11 @@ static void FindSubsidyPassengerRoute(FoundRoute *fr)
 	fr->distance = (uint)-1;
 
 	fr->from = from = GetTown(RandomRange(_total_towns));
-	if (from->xy == 0 || from->population < 400)
+	if (!IsValidTown(from) || from->population < 400)
 		return;
 
 	fr->to = to = GetTown(RandomRange(_total_towns));
-	if (from==to || to->xy == 0 || to->population < 400 || to->pct_pass_transported > 42)
+	if (from == to || !IsValidTown(to) || to->population < 400 || to->pct_pass_transported > 42)
 		return;
 
 	fr->distance = DistanceManhattan(from->xy, to->xy);
@@ -907,8 +902,7 @@ static void FindSubsidyCargoRoute(FoundRoute *fr)
 	fr->distance = (uint)-1;
 
 	fr->from = i = GetIndustry(RandomRange(_total_industries));
-	if (i->xy == 0)
-		return;
+	if (!IsValidIndustry(i)) return;
 
 	// Randomize cargo type
 	if (Random()&1 && i->produced_cargo[1] != CT_INVALID) {
@@ -934,8 +928,8 @@ static void FindSubsidyCargoRoute(FoundRoute *fr)
 		Town *t = GetTown(RandomRange(_total_towns));
 
 		// Only want big towns
-		if (t->xy == 0 || t->population < 900)
-			return;
+		if (!IsValidTown(t) || t->population < 900) return;
+
 		fr->distance = DistanceManhattan(i->xy, t->xy);
 		fr->to = t;
 	} else {
@@ -943,7 +937,7 @@ static void FindSubsidyCargoRoute(FoundRoute *fr)
 		Industry *i2 = GetIndustry(RandomRange(_total_industries));
 
 		// The industry must accept the cargo
-		if (i == i2 || i2->xy == 0 ||
+		if (i == i2 || !IsValidIndustry(i2) ||
 				(cargo != i2->accepts_cargo[0] &&
 				cargo != i2->accepts_cargo[1] &&
 				cargo != i2->accepts_cargo[2]))
@@ -1113,8 +1107,7 @@ static void DeliverGoodsToIndustry(TileIndex xy, CargoID cargo_type, int num_pie
 	FOR_ALL_INDUSTRIES(ind) {
 		uint t;
 
-		if (ind->xy != 0 && (
-					cargo_type == ind->accepts_cargo[0] ||
+		if (( cargo_type == ind->accepts_cargo[0] ||
 					cargo_type == ind->accepts_cargo[1] ||
 					cargo_type == ind->accepts_cargo[2]
 				) &&

engine.c

@@ -458,7 +458,7 @@ static inline uint16 GetEngineRenewPoolSize(void)
 	return _engine_renew_pool.total_items;
 }
 
-#define FOR_ALL_ENGINE_RENEWS_FROM(er, start) for (er = GetEngineRenew(start); er != NULL; er = (er->index + 1 < GetEngineRenewPoolSize()) ? GetEngineRenew(er->index + 1) : NULL)
+#define FOR_ALL_ENGINE_RENEWS_FROM(er, start) for (er = GetEngineRenew(start); er != NULL; er = (er->index + 1 < GetEngineRenewPoolSize()) ? GetEngineRenew(er->index + 1) : NULL) if (er->from != INVALID_ENGINE)
 #define FOR_ALL_ENGINE_RENEWS(er) FOR_ALL_ENGINE_RENEWS_FROM(er, 0)
 
 static void EngineRenewPoolNewBlock(uint start_item)

graph_gui.c

@@ -1132,10 +1132,8 @@ static void GlobalSortSignList(void)
 		error("Could not allocate memory for the sign-sorting-list");
 
 	FOR_ALL_SIGNS(ss) {
-		if (ss->str != STR_NULL) {
-			_sign_sort[n++] = ss->index;
-			_num_sign_sort++;
-		}
+		_sign_sort[n++] = ss->index;
+		_num_sign_sort++;
 	}
 
 	qsort(_sign_sort, n, sizeof(_sign_sort[0]), SignNameSorter);

industry.h

@@ -74,9 +74,9 @@ extern MemoryPool _industry_pool;
 /**
  * Check if an Industry really exists.
  */
-static inline bool IsValidIndustry(Industry* industry)
+static inline bool IsValidIndustry(const Industry *industry)
 {
-	return industry->xy != 0; /* XXX: Replace by INVALID_TILE someday */
+	return industry->xy != 0;
 }
 
 /**
@@ -95,7 +95,7 @@ static inline uint16 GetIndustryPoolSize(void)
 	return _industry_pool.total_items;
 }
 
-#define FOR_ALL_INDUSTRIES_FROM(i, start) for (i = GetIndustry(start); i != NULL; i = (i->index + 1 < GetIndustryPoolSize()) ? GetIndustry(i->index + 1) : NULL)
+#define FOR_ALL_INDUSTRIES_FROM(i, start) for (i = GetIndustry(start); i != NULL; i = (i->index + 1 < GetIndustryPoolSize()) ? GetIndustry(i->index + 1) : NULL) if (IsValidIndustry(i))
 #define FOR_ALL_INDUSTRIES(i) FOR_ALL_INDUSTRIES_FROM(i, 0)
 
 VARDEF int _total_industries; // For the AI: the amount of industries active

industry_cmd.c

@@ -38,7 +38,9 @@ static void IndustryPoolNewBlock(uint start_item)
 {
 	Industry *i;
 
-	FOR_ALL_INDUSTRIES_FROM(i, start_item) i->index = start_item++;
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (i = GetIndustry(start_item); i != NULL; i = (i->index + 1 < GetIndustryPoolSize()) ? GetIndustry(i->index + 1) : NULL) i->index = start_item++;
 }
 
 /* Initialize the industry-pool */
@@ -1018,7 +1020,7 @@ void OnTick_Industry(void)
 	if (_game_mode == GM_EDITOR) return;
 
 	FOR_ALL_INDUSTRIES(i) {
-		if (i->xy != 0) ProduceIndustryGoods(i);
+		ProduceIndustryGoods(i);
 	}
 }
 
@@ -1141,8 +1143,7 @@ static const Town *CheckMultipleIndustryInTown(TileIndex tile, int type)
 	if (_patches.multiple_industry_per_town) return t;
 
 	FOR_ALL_INDUSTRIES(i) {
-		if (i->xy != 0 &&
-				i->type == (byte)type &&
+		if (i->type == (byte)type &&
 				i->town == t) {
 			_error_message = STR_0287_ONLY_ONE_ALLOWED_PER_TOWN;
 			return NULL;
@@ -1375,8 +1376,7 @@ static bool CheckIfTooCloseToIndustry(TileIndex tile, int type)
 
 	FOR_ALL_INDUSTRIES(i) {
 		// check if an industry that accepts the same goods is nearby
-		if (i->xy != 0 &&
-				DistanceMax(tile, i->xy) <= 14 &&
+		if (DistanceMax(tile, i->xy) <= 14 &&
 				indspec->accepts_cargo[0] != CT_INVALID &&
 				indspec->accepts_cargo[0] == i->accepts_cargo[0] && (
 					_game_mode != GM_EDITOR ||
@@ -1388,8 +1388,7 @@ static bool CheckIfTooCloseToIndustry(TileIndex tile, int type)
 		}
 
 		// check "not close to" field.
-		if (i->xy != 0 &&
-				(i->type == indspec->conflicting[0] || i->type == indspec->conflicting[1] || i->type == indspec->conflicting[2]) &&
+		if ((i->type == indspec->conflicting[0] || i->type == indspec->conflicting[1] || i->type == indspec->conflicting[2]) &&
 				DistanceMax(tile, i->xy) <= 14) {
 			_error_message = STR_INDUSTRY_TOO_CLOSE;
 			return false;
@@ -1402,17 +1401,19 @@ static Industry *AllocateIndustry(void)
 {
 	Industry *i;
 
-	FOR_ALL_INDUSTRIES(i) {
-		if (i->xy == 0) {
-			IndustryID index = i->index;
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (i = GetIndustry(0); i != NULL; i = (i->index + 1 < GetIndustryPoolSize()) ? GetIndustry(i->index + 1) : NULL) {
+		IndustryID index = i->index;
 
-			if (i->index > _total_industries) _total_industries = i->index;
+		if (IsValidIndustry(i)) continue;
 
-			memset(i, 0, sizeof(*i));
-			i->index = index;
+		if (i->index > _total_industries) _total_industries = i->index;
 
-			return i;
-		}
+		memset(i, 0, sizeof(*i));
+		i->index = index;
+
+		return i;
 	}
 
 	/* Check if we can add a block to the pool */
@@ -1871,7 +1872,7 @@ void IndustryMonthlyLoop(void)
 	_current_player = OWNER_NONE;
 
 	FOR_ALL_INDUSTRIES(i) {
-		if (i->xy != 0) UpdateIndustryStatistics(i);
+		UpdateIndustryStatistics(i);
 	}
 
 	/* 3% chance that we start a new industry */
@@ -1879,7 +1880,7 @@ void IndustryMonthlyLoop(void)
 		MaybeNewIndustry(Random());
 	} else if (!_patches.smooth_economy && _total_industries > 0) {
 		i = GetIndustry(RandomRange(_total_industries));
-		if (i->xy != 0) ChangeIndustryProduction(i);
+		if (IsValidIndustry(i)) ChangeIndustryProduction(i);
 	}
 
 	_current_player = old_player;
@@ -1953,10 +1954,8 @@ static void Save_INDY(void)
 
 	// Write the vehicles
 	FOR_ALL_INDUSTRIES(ind) {
-		if (ind->xy != 0) {
-			SlSetArrayIndex(ind->index);
-			SlObject(ind, _industry_desc);
-		}
+		SlSetArrayIndex(ind->index);
+		SlObject(ind, _industry_desc);
 	}
 }
 

industry_gui.c

@@ -561,9 +561,8 @@ static void MakeSortedIndustryList(void)
 	if (_industry_sort == NULL)
 		error("Could not allocate memory for the industry-sorting-list");
 
-	FOR_ALL_INDUSTRIES(i) {
-		if (i->xy != 0) _industry_sort[n++] = i;
-	}
+	FOR_ALL_INDUSTRIES(i) _industry_sort[n++] = i;
+
 	_num_industry_sort = n;
 	_last_industry = NULL; // used for "cache"
 

main_gui.c

@@ -1569,9 +1569,8 @@ static bool AnyTownExists(void)
 {
 	const Town *t;
 
-	FOR_ALL_TOWNS(t) {
-		if (t->xy != 0) return true;
-	}
+	FOR_ALL_TOWNS(t) return true;
+
 	return false;
 }
 

network_gui.c

@@ -1514,12 +1514,6 @@ static const char *ChatTabCompletionNextItem(uint *item)
 		FOR_ALL_TOWNS_FROM(t, *item - MAX_CLIENT_INFO) {
 			int32 temp[1];
 
-			/* Skip empty towns */
-			if (t->xy == 0) {
-				(*item)++;
-				continue;
-			}
-
 			/* Get the town-name via the string-system */
 			temp[0] = t->townnameparts;
 			GetStringWithArgs(chat_tab_temp_buffer, t->townnametype, temp);

npf.c

@@ -792,7 +792,7 @@ NPFFoundTargetData NPFRouteToDepotTrialError(TileIndex tile, Trackdir trackdir,
 	FOR_ALL_DEPOTS(depot) {
 		/* Check if this is really a valid depot, it is of the needed type and
 		 * owner */
-		if (IsValidDepot(depot) && IsTileDepotType(depot->xy, type) && IsTileOwner(depot->xy, owner))
+		if (IsTileDepotType(depot->xy, type) && IsTileOwner(depot->xy, owner))
 			/* If so, let's add it to the queue, sorted by distance */
 			depots.push(&depots, depot, DistanceManhattan(tile, depot->xy));
 	}

oldloader.c

@@ -294,8 +294,6 @@ static void FixOldTowns(void)
 
 	/* Convert town-names if needed */
 	FOR_ALL_TOWNS(town) {
-		if (town->xy == 0) continue;
-
 		if (IS_INT_INSIDE(town->townnametype, 0x20C1, 0x20C3)) {
 			town->townnametype = SPECSTR_TOWNNAME_ENGLISH + _opt.town_name;
 			town->townnameparts = GetOldTownName(town->townnameparts, _opt.town_name);
@@ -346,11 +344,7 @@ static void FixOldVehicles(void)
 	FOR_ALL_VEHICLES(v) {
 		Vehicle *u;
 
-		if (v->type == 0) continue;
-
 		FOR_ALL_VEHICLES_FROM(u, v->index + 1) {
-			if (u->type == 0) continue;
-
 			/* If a vehicle has the same orders, add the link to eachother
 			 * in both vehicles */
 			if (v->orders == u->orders) {
@@ -532,7 +526,7 @@ static bool LoadOldDepot(LoadgameState *ls, int num)
 
 	if (!LoadChunk(ls, GetDepot(num), depot_chunk)) return false;
 
-	if (GetDepot(num)->xy != 0) {
+	if (IsValidDepot(GetDepot(num))) {
 		GetDepot(num)->town_index = REMAP_TOWN_IDX(_old_town_index);
 	}
 
@@ -650,7 +644,7 @@ static bool LoadOldStation(LoadgameState *ls, int num)
 	if (!LoadChunk(ls, st, station_chunk))
 		return false;
 
-	if (st->xy != 0) {
+	if (IsValidStation(st)) {
 		if (st->train_tile) {
 			/* Calculate the trainst_w and trainst_h */
 			uint w = GB(_old_platforms, 3, 3);
@@ -721,7 +715,7 @@ static bool LoadOldIndustry(LoadgameState *ls, int num)
 	i = GetIndustry(num);
 	if (!LoadChunk(ls, i, industry_chunk)) return false;
 
-	if (i->xy != 0) {
+	if (IsValidIndustry(i)) {
 		i->town = GetTown(REMAP_TOWN_IDX(_old_town_index));
 	}
 

openttd.c

@@ -1051,7 +1051,7 @@ static void UpdateExclusiveRights(void)
 	Town *t;
 
 	FOR_ALL_TOWNS(t) {
-		if (t->xy != 0) t->exclusivity = (byte)-1;
+		t->exclusivity = (byte)-1;
 	}
 
 	/* FIXME old exclusive rights status is not being imported (stored in s->blocked_months_obsolete)
@@ -1356,7 +1356,7 @@ bool AfterLoadGame(void)
 		Waypoint *wp;
 
 		FOR_ALL_WAYPOINTS(wp) {
-			if (wp->xy != 0 && wp->deleted == 0) {
+			if (wp->deleted == 0) {
 				const StationSpec *statspec = NULL;
 
 				if (HASBIT(_m[wp->xy].m3, 4))
@@ -1482,7 +1482,6 @@ bool AfterLoadGame(void)
 		FOR_ALL_INDUSTRIES(i) {
 			uint j;
 
-			if (i->xy == 0) continue;
 			if (i->type == IT_FARM || i->type == IT_FARM_2) {
 				for (j = 0; j != 50; j++) PlantRandomFarmField(i);
 			}

order.h

@@ -117,7 +117,15 @@ static inline uint16 GetOrderPoolSize(void)
 	return _order_pool.total_items;
 }
 
-#define FOR_ALL_ORDERS_FROM(order, start) for (order = GetOrder(start); order != NULL; order = (order->index + 1 < GetOrderPoolSize()) ? GetOrder(order->index + 1) : NULL)
+/**
+ * Check if a Order really exists.
+ */
+static inline bool IsValidOrder(const Order *o)
+{
+	return o->type != OT_NOTHING;
+}
+
+#define FOR_ALL_ORDERS_FROM(order, start) for (order = GetOrder(start); order != NULL; order = (order->index + 1 < GetOrderPoolSize()) ? GetOrder(order->index + 1) : NULL) if (IsValidOrder(order))
 #define FOR_ALL_ORDERS(order) FOR_ALL_ORDERS_FROM(order, 0)
 
 

order_cmd.c

@@ -29,7 +29,9 @@ static void OrderPoolNewBlock(uint start_item)
 {
 	Order *order;
 
-	FOR_ALL_ORDERS_FROM(order, start_item) order->index = start_item++;
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (order = GetOrder(start_item); order != NULL; order = (order->index + 1 < GetOrderPoolSize()) ? GetOrder(order->index + 1) : NULL) order->index = start_item++;
 }
 
 /* Initialize the order-pool */
@@ -112,8 +114,10 @@ static Order *AllocateOrder(void)
 {
 	Order *order;
 
-	FOR_ALL_ORDERS(order) {
-		if (order->type == OT_NOTHING) {
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (order = GetOrder(0); order != NULL; order = (order->index + 1 < GetOrderPoolSize()) ? GetOrder(order->index + 1) : NULL) {
+		if (!IsValidOrder(order)) {
 			uint index = order->index;
 
 			memset(order, 0, sizeof(*order));
@@ -177,7 +181,7 @@ int32 CmdInsertOrder(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 
 	if (!IsVehicleIndex(veh)) return CMD_ERROR;
 	v = GetVehicle(veh);
-	if (v->type == 0 || !CheckOwnership(v->owner)) return CMD_ERROR;
+	if (IsValidVehicle(v) || !CheckOwnership(v->owner)) return CMD_ERROR;
 
 	/* Check if the inserted order is to the correct destination (owner, type),
 	 * and has the correct flags if any */
@@ -440,7 +444,7 @@ int32 CmdDeleteOrder(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 
 	if (!IsVehicleIndex(veh_id)) return CMD_ERROR;
 	v = GetVehicle(veh_id);
-	if (v->type == 0 || !CheckOwnership(v->owner)) return CMD_ERROR;
+	if (IsValidVehicle(v) || !CheckOwnership(v->owner)) return CMD_ERROR;
 
 	/* If we did not select an order, we maybe want to de-clone the orders */
 	if (sel_ord >= v->num_orders)
@@ -512,7 +516,7 @@ int32 CmdSkipOrder(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 
 	if (!IsVehicleIndex(veh_id)) return CMD_ERROR;
 	v = GetVehicle(veh_id);
-	if (v->type == 0 || !CheckOwnership(v->owner)) return CMD_ERROR;
+	if (IsValidVehicle(v) || !CheckOwnership(v->owner)) return CMD_ERROR;
 
 	if (flags & DC_EXEC) {
 		/* Goto next order */
@@ -561,7 +565,7 @@ int32 CmdModifyOrder(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 	if (p2 != OFB_FULL_LOAD && p2 != OFB_UNLOAD && p2 != OFB_NON_STOP && p2 != OFB_TRANSFER) return CMD_ERROR;
 
 	v = GetVehicle(veh);
-	if (v->type == 0 || !CheckOwnership(v->owner)) return CMD_ERROR;
+	if (IsValidVehicle(v) || !CheckOwnership(v->owner)) return CMD_ERROR;
 
 	/* Is it a valid order? */
 	if (sel_ord >= v->num_orders) return CMD_ERROR;
@@ -628,7 +632,7 @@ int32 CmdCloneOrder(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 
 	dst = GetVehicle(veh_dst);
 
-	if (dst->type == 0 || !CheckOwnership(dst->owner)) return CMD_ERROR;
+	if (IsValidVehicle(dst) || !CheckOwnership(dst->owner)) return CMD_ERROR;
 
 	switch (p2) {
 		case CO_SHARE: {
@@ -639,7 +643,7 @@ int32 CmdCloneOrder(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 			src = GetVehicle(veh_src);
 
 			/* Sanity checks */
-			if (src->type == 0 || !CheckOwnership(src->owner) || dst->type != src->type || dst == src)
+			if (IsValidVehicle(src) || !CheckOwnership(src->owner) || dst->type != src->type || dst == src)
 				return CMD_ERROR;
 
 			/* Trucks can't share orders with busses (and visa versa) */
@@ -686,7 +690,7 @@ int32 CmdCloneOrder(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 			src = GetVehicle(veh_src);
 
 			/* Sanity checks */
-			if (src->type == 0 || !CheckOwnership(src->owner) || dst->type != src->type || dst == src)
+			if (IsValidVehicle(src) || !CheckOwnership(src->owner) || dst->type != src->type || dst == src)
 				return CMD_ERROR;
 
 			/* Trucks can't copy all the orders from busses (and visa versa) */
@@ -844,7 +848,7 @@ int32 CmdRestoreOrderIndex(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 
 	v = GetVehicle(p1);
 	/* Check the vehicle type and ownership, and if the service interval and order are in range */
-	if (v->type == 0 || !CheckOwnership(v->owner)) return CMD_ERROR;
+	if (IsValidVehicle(v) || !CheckOwnership(v->owner)) return CMD_ERROR;
 	if (serv_int != GetServiceIntervalClamped(serv_int) || cur_ord >= v->num_orders) return CMD_ERROR;
 
 	if (flags & DC_EXEC) {
@@ -963,8 +967,7 @@ void DeleteDestinationFromVehicleOrder(Order dest)
 
 	/* Go through all vehicles */
 	FOR_ALL_VEHICLES(v) {
-		if (v->type == 0 || v->orders == NULL)
-			continue;
+		if (v->orders == NULL) continue;
 
 		/* Forget about this station if this station is removed */
 		if (v->last_station_visited == dest.station && dest.type == OT_GOTO_STATION)
@@ -1130,10 +1133,8 @@ static void Save_ORDR(void)
 	Order *order;
 
 	FOR_ALL_ORDERS(order) {
-		if (order->type != OT_NOTHING) {
-			SlSetArrayIndex(order->index);
-			SlObject(order, _order_desc);
-		}
+		SlSetArrayIndex(order->index);
+		SlObject(order, _order_desc);
 	}
 }
 

settings.c

@@ -1074,9 +1074,8 @@ static int32 PopulationInLabelActive(int32 p1)
 {
 	Town* t;
 
-	FOR_ALL_TOWNS(t) {
-		if (t->xy != 0) UpdateTownVirtCoord(t);
-	}
+	FOR_ALL_TOWNS(t) UpdateTownVirtCoord(t);
+
 	return 0;
 }
 

ship_cmd.c

@@ -86,7 +86,7 @@ static const Depot* FindClosestShipDepot(const Vehicle* v)
 	} else {
 		FOR_ALL_DEPOTS(depot) {
 			tile = depot->xy;
-			if (IsValidDepot(depot) && IsTileDepotType(tile, TRANSPORT_WATER) && IsTileOwner(tile, v->owner)) {
+			if (IsTileDepotType(tile, TRANSPORT_WATER) && IsTileOwner(tile, v->owner)) {
 				dist = DistanceManhattan(tile, tile2);
 				if (dist < best_dist) {
 					best_dist = dist;

signs.c

@@ -25,8 +25,9 @@ static void SignPoolNewBlock(uint start_item)
 {
 	SignStruct *ss;
 
-	FOR_ALL_SIGNS_FROM(ss, start_item)
-		ss->index = start_item++;
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (ss = GetSign(start_item); ss != NULL; ss = (ss->index + 1 < GetSignPoolSize()) ? GetSign(ss->index + 1) : NULL) ss->index = start_item++;
 }
 
 /* Initialize the sign-pool */
@@ -53,9 +54,7 @@ void UpdateAllSignVirtCoords(void)
 {
 	SignStruct *ss;
 
-	FOR_ALL_SIGNS(ss)
-		if (ss->str != 0)
-			UpdateSignVirtCoords(ss);
+	FOR_ALL_SIGNS(ss) UpdateSignVirtCoords(ss);
 
 }
 
@@ -83,8 +82,11 @@ static void MarkSignDirty(SignStruct *ss)
 static SignStruct *AllocateSign(void)
 {
 	SignStruct *ss;
-	FOR_ALL_SIGNS(ss) {
-		if (ss->str == 0) {
+
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (ss = GetSign(0); ss != NULL; ss = (ss->index + 1 < GetSignPoolSize()) ? GetSign(ss->index + 1) : NULL) {
+		if (!IsValidSign(ss)) {
 			uint index = ss->index;
 
 			memset(ss, 0, sizeof(SignStruct));
@@ -246,11 +248,8 @@ static void Save_SIGN(void)
 	SignStruct *ss;
 
 	FOR_ALL_SIGNS(ss) {
-		/* Don't save empty signs */
-		if (ss->str != 0) {
-			SlSetArrayIndex(ss->index);
-			SlObject(ss, _sign_desc);
-		}
+		SlSetArrayIndex(ss->index);
+		SlObject(ss, _sign_desc);
 	}
 }
 

signs.h

@@ -19,14 +19,6 @@ typedef struct SignStruct {
 
 extern MemoryPool _sign_pool;
 
-/**
- * Check if a Sign really exists.
- */
-static inline bool IsValidSign(const SignStruct* ss)
-{
-	return ss->str != 0;
-}
-
 /**
  * Get the pointer to the sign with index 'index'
  */
@@ -48,7 +40,15 @@ static inline bool IsSignIndex(uint index)
 	return index < GetSignPoolSize();
 }
 
-#define FOR_ALL_SIGNS_FROM(ss, start) for (ss = GetSign(start); ss != NULL; ss = (ss->index + 1 < GetSignPoolSize()) ? GetSign(ss->index + 1) : NULL)
+/**
+ * Check if a Sign really exists.
+ */
+static inline bool IsValidSign(const SignStruct* ss)
+{
+	return ss->str != STR_NULL;
+}
+
+#define FOR_ALL_SIGNS_FROM(ss, start) for (ss = GetSign(start); ss != NULL; ss = (ss->index + 1 < GetSignPoolSize()) ? GetSign(ss->index + 1) : NULL) if (IsValidSign(ss))
 #define FOR_ALL_SIGNS(ss) FOR_ALL_SIGNS_FROM(ss, 0)
 
 VARDEF bool _sign_sort_dirty;

smallmap_gui.c

@@ -696,7 +696,7 @@ skip_column:
 		byte color;
 
 		FOR_ALL_VEHICLES(v) {
-			if (v->type != 0 && v->type != VEH_Special &&
+			if (v->type != VEH_Special &&
 					(v->vehstatus & (VS_HIDDEN | VS_UNCLICKABLE)) == 0) {
 				// Remap into flat coordinates.
 				Point pt = RemapCoords(
@@ -742,24 +742,22 @@ skip_column:
 		const Town *t;
 
 		FOR_ALL_TOWNS(t) {
-			if (t->xy != 0) {
-				// Remap the town coordinate
-				Point pt = RemapCoords(
-					(int)(TileX(t->xy) * TILE_SIZE - WP(w, smallmap_d).scroll_x) / TILE_SIZE,
-					(int)(TileY(t->xy) * TILE_SIZE - WP(w, smallmap_d).scroll_y) / TILE_SIZE,
-					0);
-				x = pt.x - WP(w,smallmap_d).subscroll + 3 - (t->sign.width_2 >> 1);
-				y = pt.y;
-
-				// Check if the town sign is within bounds
-				if (x + t->sign.width_2 > dpi->left &&
-						x < dpi->left + dpi->width &&
-						y + 6 > dpi->top &&
-						y < dpi->top + dpi->height) {
-					// And draw it.
-					SetDParam(0, t->index);
-					DrawString(x, y, STR_2056, 12);
-				}
+			// Remap the town coordinate
+			Point pt = RemapCoords(
+				(int)(TileX(t->xy) * TILE_SIZE - WP(w, smallmap_d).scroll_x) / TILE_SIZE,
+				(int)(TileY(t->xy) * TILE_SIZE - WP(w, smallmap_d).scroll_y) / TILE_SIZE,
+				0);
+			x = pt.x - WP(w,smallmap_d).subscroll + 3 - (t->sign.width_2 >> 1);
+			y = pt.y;
+
+			// Check if the town sign is within bounds
+			if (x + t->sign.width_2 > dpi->left &&
+					x < dpi->left + dpi->width &&
+					y + 6 > dpi->top &&
+					y < dpi->top + dpi->height) {
+				// And draw it.
+				SetDParam(0, t->index);
+				DrawString(x, y, STR_2056, 12);
 			}
 		}
 	}

station.h

@@ -166,7 +166,15 @@ static inline bool IsStationIndex(StationID index)
 	return index < GetStationPoolSize();
 }
 
-#define FOR_ALL_STATIONS_FROM(st, start) for (st = GetStation(start); st != NULL; st = (st->index + 1 < GetStationPoolSize()) ? GetStation(st->index + 1) : NULL)
+/**
+ * Check if a station really exists.
+ */
+static inline bool IsValidStation(const Station *st)
+{
+	return st->xy != 0;
+}
+
+#define FOR_ALL_STATIONS_FROM(st, start) for (st = GetStation(start); st != NULL; st = (st->index + 1 < GetStationPoolSize()) ? GetStation(st->index + 1) : NULL) if (IsValidStation(st))
 #define FOR_ALL_STATIONS(st) FOR_ALL_STATIONS_FROM(st, 0)
 
 
@@ -190,7 +198,15 @@ static inline uint16 GetRoadStopPoolSize(void)
 	return _roadstop_pool.total_items;
 }
 
-#define FOR_ALL_ROADSTOPS_FROM(rs, start) for (rs = GetRoadStop(start); rs != NULL; rs = (rs->index + 1 < GetRoadStopPoolSize()) ? GetRoadStop(rs->index + 1) : NULL)
+/**
+ * Check if a RaodStop really exists.
+ */
+static inline bool IsValidRoadStop(const RoadStop *rs)
+{
+	return rs->used;
+}
+
+#define FOR_ALL_ROADSTOPS_FROM(rs, start) for (rs = GetRoadStop(start); rs != NULL; rs = (rs->index + 1 < GetRoadStopPoolSize()) ? GetRoadStop(rs->index + 1) : NULL) if (IsValidRoadStop(rs))
 #define FOR_ALL_ROADSTOPS(rs) FOR_ALL_ROADSTOPS_FROM(rs, 0)
 
 /* End of stuff for ROADSTOPS */
@@ -212,14 +228,6 @@ uint GetNumRoadStops(const Station* st, RoadStopType type);
 RoadStop * AllocateRoadStop( void );
 void ClearSlot(Vehicle *v);
 
-/**
- * Check if a station really exists.
- */
-static inline bool IsValidStation(const Station *st)
-{
-	return st->xy != 0; /* XXX: Replace by INVALID_TILE someday */
-}
-
 static inline bool IsBuoy(const Station* st)
 {
 	return (st->had_vehicle_of_type & HVOT_BUOY) != 0; /* XXX: We should really ditch this ugly coding and switch to something sane... */

station_cmd.c

@@ -51,7 +51,9 @@ static void StationPoolNewBlock(uint start_item)
 {
 	Station *st;
 
-	FOR_ALL_STATIONS_FROM(st, start_item) st->index = start_item++;
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 *  This is just a temporary stage, this will be removed. */
+	for (st = GetStation(start_item); st != NULL; st = (st->index + 1 < GetStationPoolSize()) ? GetStation(st->index + 1) : NULL) st->index = start_item++;
 }
 
 static void StationPoolCleanBlock(uint start_item, uint end_item)
@@ -72,7 +74,9 @@ static void RoadStopPoolNewBlock(uint start_item)
 {
 	RoadStop *rs;
 
-	FOR_ALL_ROADSTOPS_FROM(rs, start_item) rs->index = start_item++;
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (rs = GetRoadStop(start_item); rs != NULL; rs = (rs->index + 1 < GetRoadStopPoolSize()) ? GetRoadStop(rs->index + 1) : NULL) rs->index = start_item++;
 }
 
 /* Initialize the station-pool and roadstop-pool */
@@ -145,8 +149,10 @@ RoadStop *AllocateRoadStop(void)
 {
 	RoadStop *rs;
 
-	FOR_ALL_ROADSTOPS(rs) {
-		if (!rs->used) {
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (rs = GetRoadStop(0); rs != NULL; rs = (rs->index + 1 < GetRoadStopPoolSize()) ? GetRoadStop(rs->index + 1) : NULL) {
+		if (!IsValidRoadStop(rs)) {
 			uint index = rs->index;
 
 			memset(rs, 0, sizeof(*rs));
@@ -252,8 +258,10 @@ static Station *AllocateStation(void)
 {
 	Station *st = NULL;
 
-	FOR_ALL_STATIONS(st) {
-		if (st->xy == 0) {
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (st = GetStation(0); st != NULL; st = (st->index + 1 < GetStationPoolSize()) ? GetStation(st->index + 1) : NULL) {
+		if (!IsValidStation(st)) {
 			StationID index = st->index;
 
 			memset(st, 0, sizeof(Station));
@@ -337,7 +345,7 @@ static bool GenerateStationName(Station *st, TileIndex tile, int flag)
 		Station *s;
 
 		FOR_ALL_STATIONS(s) {
-			if (s != st && s->xy != 0 && s->town==t) {
+			if (s != st && s->town==t) {
 				uint str = M(s->string_id);
 				if (str <= 0x20) {
 					if (str == M(STR_SV_STNAME_FOREST))
@@ -438,7 +446,7 @@ static Station* GetClosestStationFromTile(TileIndex tile, uint threshold, Player
 	Station* st;
 
 	FOR_ALL_STATIONS(st) {
-		if (st->xy != 0 && (owner == OWNER_SPECTATOR || st->owner == owner)) {
+		if ((owner == OWNER_SPECTATOR || st->owner == owner)) {
 			uint cur_dist = DistanceManhattan(tile, st->xy);
 
 			if (cur_dist < threshold) {
@@ -501,7 +509,7 @@ void UpdateAllStationVirtCoord(void)
 	Station* st;
 
 	FOR_ALL_STATIONS(st) {
-		if (st->xy != 0) UpdateStationVirtCoord(st);
+		UpdateStationVirtCoord(st);
 	}
 }
 
@@ -1664,7 +1672,7 @@ int32 CmdBuildAirport(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 	{
 		uint num = 0;
 		FOR_ALL_STATIONS(st) {
-			if (st->xy != 0 && st->town == t && st->facilities&FACIL_AIRPORT && st->airport_type != AT_OILRIG)
+			if (st->town == t && st->facilities&FACIL_AIRPORT && st->airport_type != AT_OILRIG)
 				num++;
 		}
 		if (num >= 2) {
@@ -2435,7 +2443,7 @@ void DeleteAllPlayerStations(void)
 	Station *st;
 
 	FOR_ALL_STATIONS(st) {
-		if (st->xy != 0 && st->owner < MAX_PLAYERS) DeleteStation(st);
+		if (st->owner < MAX_PLAYERS) DeleteStation(st);
 	}
 }
 
@@ -2569,10 +2577,10 @@ void OnTick_Station(void)
 	if (++_station_tick_ctr == GetStationPoolSize()) _station_tick_ctr = 0;
 
 	st = GetStation(i);
-	if (st->xy != 0) StationHandleBigTick(st);
+	if (IsValidStation(st)) StationHandleBigTick(st);
 
 	FOR_ALL_STATIONS(st) {
-		if (st->xy != 0) StationHandleSmallTick(st);
+		StationHandleSmallTick(st);
 	}
 }
 
@@ -2586,7 +2594,7 @@ void ModifyStationRatingAround(TileIndex tile, PlayerID owner, int amount, uint
 	Station *st;
 
 	FOR_ALL_STATIONS(st) {
-		if (st->xy != 0 && st->owner == owner &&
+		if (st->owner == owner &&
 				DistanceManhattan(tile, st->xy) <= radius) {
 			uint i;
 
@@ -3068,10 +3076,8 @@ static void Save_STNS(void)
 	Station *st;
 	// Write the stations
 	FOR_ALL_STATIONS(st) {
-		if (st->xy != 0) {
-			SlSetArrayIndex(st->index);
-			SlAutolength((AutolengthProc*)SaveLoad_STNS, st);
-		}
+		SlSetArrayIndex(st->index);
+		SlAutolength((AutolengthProc*)SaveLoad_STNS, st);
 	}
 }
 
@@ -3126,10 +3132,8 @@ static void Save_ROADSTOP(void)
 	RoadStop *rs;
 
 	FOR_ALL_ROADSTOPS(rs) {
-		if (rs->used) {
-			SlSetArrayIndex(rs->index);
-			SlObject(rs, _roadstop_desc);
-		}
+		SlSetArrayIndex(rs->index);
+		SlObject(rs, _roadstop_desc);
 	}
 }
 

station_gui.c

@@ -187,7 +187,7 @@ static void BuildStationsList(plstations_d* sl, PlayerID owner, byte facilities,
 	DEBUG(misc, 1) ("Building station list for player %d...", owner);
 
 	FOR_ALL_STATIONS(st) {
-		if (st->xy && st->owner == owner) {
+		if (st->owner == owner) {
 			if (facilities & st->facilities) { //only stations with selected facilities
 				int num_waiting_cargo = 0;
 				for (j = 0; j < NUM_CARGO; j++) {

strings.c

@@ -673,7 +673,7 @@ static char *FormatString(char *buff, const char *str, const int32 *argv, uint c
 				int32 args[2];
 
 				// industry not valid anymore?
-				if (i->xy == 0) break;
+				if (!IsValidIndustry(i)) break;
 
 				// First print the town name and the industry type name
 				// The string STR_INDUSTRY_PATTERN controls the formatting
@@ -829,7 +829,7 @@ static char *FormatString(char *buff, const char *str, const int32 *argv, uint c
 			const Station* st = GetStation(GetInt32(&argv));
 			int32 temp[2];
 
-			if (st->xy == 0) { // station doesn't exist anymore
+			if (!IsValidStation(st)) { // station doesn't exist anymore
 				buff = GetStringWithArgs(buff, STR_UNKNOWN_DESTINATION, NULL);
 				break;
 			}
@@ -842,7 +842,7 @@ static char *FormatString(char *buff, const char *str, const int32 *argv, uint c
 			const Town* t = GetTown(GetInt32(&argv));
 			int32 temp[1];
 
-			assert(t->xy != 0);
+			assert(IsValidTown(t));
 
 			temp[0] = t->townnameparts;
 			buff = GetStringWithArgs(buff, t->townnametype, temp);

town.h

@@ -160,7 +160,7 @@ extern MemoryPool _town_pool;
  */
 static inline bool IsValidTown(const Town* town)
 {
-	return town->xy != 0; /* XXX: Replace by INVALID_TILE someday */
+	return town->xy != 0;
 }
 
 /**
@@ -184,7 +184,7 @@ static inline bool IsTownIndex(uint index)
 	return index < GetTownPoolSize();
 }
 
-#define FOR_ALL_TOWNS_FROM(t, start) for (t = GetTown(start); t != NULL; t = (t->index + 1 < GetTownPoolSize()) ? GetTown(t->index + 1) : NULL)
+#define FOR_ALL_TOWNS_FROM(t, start) for (t = GetTown(start); t != NULL; t = (t->index + 1 < GetTownPoolSize()) ? GetTown(t->index + 1) : NULL) if (IsValidTown(t))
 #define FOR_ALL_TOWNS(t) FOR_ALL_TOWNS_FROM(t, 0)
 
 VARDEF uint _total_towns; // For the AI: the amount of towns active

town_cmd.c

@@ -43,8 +43,9 @@ static void TownPoolNewBlock(uint start_item)
 {
 	Town *t;
 
-	FOR_ALL_TOWNS_FROM(t, start_item)
-		t->index = start_item++;
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (t = GetTown(start_item); t != NULL; t = (t->index + 1 < GetTownPoolSize()) ? GetTown(t->index + 1) : NULL) t->index = start_item++;
 }
 
 /* Initialize the town-pool */
@@ -168,7 +169,7 @@ static bool IsCloseToTown(TileIndex tile, uint dist)
 	const Town* t;
 
 	FOR_ALL_TOWNS(t) {
-		if (t->xy != 0 && DistanceManhattan(tile, t->xy) < dist) return true;
+		if (DistanceManhattan(tile, t->xy) < dist) return true;
 	}
 	return false;
 }
@@ -415,7 +416,7 @@ void OnTick_Town(void)
 
 		t = GetTown(i);
 
-		if (t->xy != 0) TownTickHandler(t);
+		if (IsValidTown(t)) TownTickHandler(t);
 	}
 }
 
@@ -857,15 +858,13 @@ restart:
 		if (strlen(buf1) >= 31 || GetStringWidth(buf1) > 130) continue;
 
 		FOR_ALL_TOWNS(t2) {
-			if (t2->xy != 0) {
-				// We can't just compare the numbers since
-				// several numbers may map to a single name.
-				SetDParam(0, t2->index);
-				GetString(buf2, STR_TOWN);
-				if (strcmp(buf1, buf2) == 0) {
-					if (tries-- < 0) return false;
-					goto restart;
-				}
+			// We can't just compare the numbers since
+			// several numbers may map to a single name.
+			SetDParam(0, t2->index);
+			GetString(buf2, STR_TOWN);
+			if (strcmp(buf1, buf2) == 0) {
+				if (tries-- < 0) return false;
+				goto restart;
 			}
 		}
 		*townnameparts = r;
@@ -949,8 +948,11 @@ static void DoCreateTown(Town *t, TileIndex tile, uint32 townnameparts, uint siz
 static Town *AllocateTown(void)
 {
 	Town *t;
-	FOR_ALL_TOWNS(t) {
-		if (t->xy == 0) {
+
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (t = GetTown(0); t != NULL; t = (t->index + 1 < GetTownPoolSize()) ? GetTown(t->index + 1) : NULL) {
+		if (!IsValidTown(t)) {
 			TownID index = t->index;
 
 			if (t->index > _total_towns)
@@ -1066,7 +1068,7 @@ bool GenerateTowns(void)
 	if (num == 0 && CreateRandomTown(10000, 0) == NULL) {
 		const Town* t;
 
-		FOR_ALL_TOWNS(t) if (IsValidTown(t)) return true;
+		FOR_ALL_TOWNS(t) return true;
 
 		//XXX can we handle that more gracefully?
 		if (num == 0 && _game_mode != GM_EDITOR) {
@@ -1380,8 +1382,7 @@ void DeleteTown(Town *t)
 
 	// Delete all industries belonging to the town
 	FOR_ALL_INDUSTRIES(i) {
-		if (i->xy && i->town == t)
-			DeleteIndustry(i);
+		if (i->town == t) DeleteIndustry(i);
 	}
 
 	// Go through all tiles and delete those belonging to the town
@@ -1736,12 +1737,10 @@ Town* CalcClosestTownFromTile(TileIndex tile, uint threshold)
 	Town *best_town = NULL;
 
 	FOR_ALL_TOWNS(t) {
-		if (t->xy != 0) {
-			dist = DistanceManhattan(tile, t->xy);
-			if (dist < best) {
-				best = dist;
-				best_town = t;
-			}
+		dist = DistanceManhattan(tile, t->xy);
+		if (dist < best) {
+			best = dist;
+			best_town = t;
 		}
 	}
 
@@ -1826,7 +1825,7 @@ void TownsMonthlyLoop(void)
 {
 	Town *t;
 
-	FOR_ALL_TOWNS(t) if (t->xy != 0) {
+	FOR_ALL_TOWNS(t) {
 		if (t->road_build_months != 0) t->road_build_months--;
 
 		if (t->exclusive_counter != 0)
@@ -1942,10 +1941,8 @@ static void Save_TOWN(void)
 	Town *t;
 
 	FOR_ALL_TOWNS(t) {
-		if (t->xy != 0) {
-			SlSetArrayIndex(t->index);
-			SlObject(t, _town_desc);
-		}
+		SlSetArrayIndex(t->index);
+		SlObject(t, _town_desc);
 	}
 }
 
@@ -1978,10 +1975,8 @@ void AfterLoadTown(void)
 {
 	Town *t;
 	FOR_ALL_TOWNS(t) {
-		if (t->xy != 0) {
-			UpdateTownRadius(t);
-			UpdateTownVirtCoord(t);
-		}
+		UpdateTownRadius(t);
+		UpdateTownVirtCoord(t);
 	}
 	_town_sort_dirty = true;
 }

town_gui.c

@@ -414,9 +414,7 @@ static void MakeSortedTownList(void)
 	if (_town_sort == NULL)
 		error("Could not allocate memory for the town-sorting-list");
 
-	FOR_ALL_TOWNS(t) {
-		if (t->xy != 0) _town_sort[n++] = t;
-	}
+	FOR_ALL_TOWNS(t) _town_sort[n++] = t;
 
 	_num_town_sort = n;
 

vehicle.c

@@ -83,7 +83,9 @@ static void VehiclePoolNewBlock(uint start_item)
 {
 	Vehicle *v;
 
-	FOR_ALL_VEHICLES_FROM(v, start_item) v->index = start_item++;
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (v = GetVehicle(start_item); v != NULL; v = (v->index + 1 < GetVehiclePoolSize()) ? GetVehicle(v->index + 1) : NULL) v->index = start_item++;
 }
 
 /* Initialize the vehicle-pool */
@@ -225,23 +227,21 @@ void AfterLoadVehicles(void)
 	}
 
 	FOR_ALL_VEHICLES(v) {
-		if (v->type != 0) {
-			switch (v->type) {
-				case VEH_Train: v->cur_image = GetTrainImage(v, v->direction); break;
-				case VEH_Road: v->cur_image = GetRoadVehImage(v, v->direction); break;
-				case VEH_Ship: v->cur_image = GetShipImage(v, v->direction); break;
-				case VEH_Aircraft:
-					if (v->subtype == 0 || v->subtype == 2) {
-						v->cur_image = GetAircraftImage(v, v->direction);
-						if (v->next != NULL) v->next->cur_image = v->cur_image;
-					}
-					break;
-				default: break;
-			}
-
-			v->left_coord = INVALID_COORD;
-			VehiclePositionChanged(v);
+		switch (v->type) {
+			case VEH_Train: v->cur_image = GetTrainImage(v, v->direction); break;
+			case VEH_Road: v->cur_image = GetRoadVehImage(v, v->direction); break;
+			case VEH_Ship: v->cur_image = GetShipImage(v, v->direction); break;
+			case VEH_Aircraft:
+				if (v->subtype == 0 || v->subtype == 2) {
+					v->cur_image = GetAircraftImage(v, v->direction);
+					if (v->next != NULL) v->next->cur_image = v->cur_image;
+				}
+				break;
+			default: break;
 		}
+
+		v->left_coord = INVALID_COORD;
+		VehiclePositionChanged(v);
 	}
 }
 
@@ -284,13 +284,14 @@ Vehicle *ForceAllocateSpecialVehicle(void)
 
 	Vehicle *v;
 
-	FOR_ALL_VEHICLES(v) {
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (v = GetVehicle(0); v != NULL; v = (v->index + 1 < GetVehiclePoolSize()) ? GetVehicle(v->index + 1) : NULL) {
 		/* No more room for the special vehicles, return NULL */
 		if (v->index >= (1 << _vehicle_pool.block_size_bits) * BLOCKS_FOR_SPECIAL_VEHICLES)
 			return NULL;
 
-		if (v->type == 0)
-			return InitializeVehicle(v);
+		if (!IsValidVehicle(v)) return InitializeVehicle(v);
 	}
 
 	return NULL;
@@ -311,11 +312,12 @@ static Vehicle *AllocateSingleVehicle(VehicleID *skip_vehicles)
 	Vehicle *v;
 	const int offset = (1 << VEHICLES_POOL_BLOCK_SIZE_BITS) * BLOCKS_FOR_SPECIAL_VEHICLES;
 
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
 	if (*skip_vehicles < (_vehicle_pool.total_items - offset)) {	// make sure the offset in the array is not larger than the array itself
-		FOR_ALL_VEHICLES_FROM(v, offset + *skip_vehicles) {
+		for (v = GetVehicle(offset + *skip_vehicles); v != NULL; v = (v->index + 1 < GetVehiclePoolSize()) ? GetVehicle(v->index + 1) : NULL) {
 			(*skip_vehicles)++;
-			if (v->type == 0)
-				return InitializeVehicle(v);
+			if (!IsValidVehicle(v)) return InitializeVehicle(v);
 		}
 	}
 
@@ -620,9 +622,7 @@ void CallVehicleTicks(void)
 	_first_veh_in_depot_list = NULL;	// now we are sure it's initialized at the start of each tick
 
 	FOR_ALL_VEHICLES(v) {
-		if (v->type != 0) {
-			_vehicle_tick_procs[v->type - 0x10](v);
-		}
+		_vehicle_tick_procs[v->type - 0x10](v);
 	}
 
 	// now we handle all the vehicles that entered a depot this tick
@@ -1395,7 +1395,7 @@ Vehicle *CheckClickOnVehicle(const ViewPort *vp, int x, int y)
 	y = (y << vp->zoom) + vp->virtual_top;
 
 	FOR_ALL_VEHICLES(v) {
-		if (v->type != 0 && (v->vehstatus & (VS_HIDDEN|VS_UNCLICKABLE)) == 0 &&
+		if ((v->vehstatus & (VS_HIDDEN|VS_UNCLICKABLE)) == 0 &&
 				x >= v->left_coord && x <= v->right_coord &&
 				y >= v->top_coord && y <= v->bottom_coord) {
 
@@ -1944,7 +1944,7 @@ int32 CmdChangeServiceInt(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 
 	v = GetVehicle(p1);
 
-	if (v->type == 0 || !CheckOwnership(v->owner)) return CMD_ERROR;
+	if (!IsValidVehicle(v) || !CheckOwnership(v->owner)) return CMD_ERROR;
 
 	if (flags & DC_EXEC) {
 		v->service_interval = serv_int;
@@ -2396,10 +2396,8 @@ static void Save_VEHS(void)
 	Vehicle *v;
 	// Write the vehicles
 	FOR_ALL_VEHICLES(v) {
-		if (v->type != 0) {
-			SlSetArrayIndex(v->index);
-			SlObject(v, _veh_descs[v->type - 0x10]);
-		}
+		SlSetArrayIndex(v->index);
+		SlObject(v, _veh_descs[v->type - 0x10]);
 	}
 }
 
@@ -2435,13 +2433,7 @@ static void Load_VEHS(void)
 		FOR_ALL_VEHICLES(v) {
 			Vehicle *u;
 
-			if (v->type == 0)
-				continue;
-
 			FOR_ALL_VEHICLES_FROM(u, v->index + 1) {
-				if (u->type == 0)
-					continue;
-
 				/* If a vehicle has the same orders, add the link to eachother
 				    in both vehicles */
 				if (v->orders == u->orders) {

vehicle.h

@@ -359,9 +359,6 @@ static inline uint16 GetVehiclePoolSize(void)
 	return _vehicle_pool.total_items;
 }
 
-#define FOR_ALL_VEHICLES_FROM(v, start) for (v = GetVehicle(start); v != NULL; v = (v->index + 1 < GetVehiclePoolSize()) ? GetVehicle(v->index + 1) : NULL)
-#define FOR_ALL_VEHICLES(v) FOR_ALL_VEHICLES_FROM(v, 0)
-
 /**
  * Check if a Vehicle really exists.
  */
@@ -370,6 +367,9 @@ static inline bool IsValidVehicle(const Vehicle *v)
 	return v->type != 0;
 }
 
+#define FOR_ALL_VEHICLES_FROM(v, start) for (v = GetVehicle(start); v != NULL; v = (v->index + 1 < GetVehiclePoolSize()) ? GetVehicle(v->index + 1) : NULL) if (IsValidVehicle(v))
+#define FOR_ALL_VEHICLES(v) FOR_ALL_VEHICLES_FROM(v, 0)
+
 /**
  * Check if an index is a vehicle-index (so between 0 and max-vehicles)
  *

viewport.c

@@ -770,8 +770,7 @@ static void ViewportAddTownNames(DrawPixelInfo *dpi)
 
 	if (dpi->zoom < 1) {
 		FOR_ALL_TOWNS(t) {
-			if (t->xy &&
-			    bottom > t->sign.top &&
+			if (bottom > t->sign.top &&
 					top < t->sign.top + 12 &&
 					right > t->sign.left &&
 					left < t->sign.left + t->sign.width_1) {
@@ -786,8 +785,7 @@ static void ViewportAddTownNames(DrawPixelInfo *dpi)
 		bottom += 2;
 
 		FOR_ALL_TOWNS(t) {
-			if (t->xy &&
-			    bottom > t->sign.top &&
+			if (bottom > t->sign.top &&
 					top < t->sign.top + 24 &&
 					right > t->sign.left &&
 					left < t->sign.left + t->sign.width_1*2) {
@@ -803,8 +801,7 @@ static void ViewportAddTownNames(DrawPixelInfo *dpi)
 
 		assert(dpi->zoom == 2);
 		FOR_ALL_TOWNS(t) {
-			if (t->xy &&
-			    bottom > t->sign.top &&
+			if (bottom > t->sign.top &&
 					top < t->sign.top + 24 &&
 					right > t->sign.left &&
 					left < t->sign.left + t->sign.width_2*4) {
@@ -832,8 +829,7 @@ static void ViewportAddStationNames(DrawPixelInfo *dpi)
 
 	if (dpi->zoom < 1) {
 		FOR_ALL_STATIONS(st) {
-			if (st->xy &&
-			    bottom > st->sign.top &&
+			if (bottom > st->sign.top &&
 					top < st->sign.top + 12 &&
 					right > st->sign.left &&
 					left < st->sign.left + st->sign.width_1) {
@@ -850,8 +846,7 @@ static void ViewportAddStationNames(DrawPixelInfo *dpi)
 		bottom += 2;
 
 		FOR_ALL_STATIONS(st) {
-			if (st->xy &&
-			    bottom > st->sign.top &&
+			if (bottom > st->sign.top &&
 					top < st->sign.top + 24 &&
 					right > st->sign.left &&
 					left < st->sign.left + st->sign.width_1*2) {
@@ -871,8 +866,7 @@ static void ViewportAddStationNames(DrawPixelInfo *dpi)
 		bottom += 5;
 
 		FOR_ALL_STATIONS(st) {
-			if (st->xy &&
-			    bottom > st->sign.top &&
+			if (bottom > st->sign.top &&
 					top < st->sign.top + 24 &&
 					right > st->sign.left &&
 					left < st->sign.left + st->sign.width_2*4) {
@@ -903,8 +897,7 @@ static void ViewportAddSigns(DrawPixelInfo *dpi)
 
 	if (dpi->zoom < 1) {
 		FOR_ALL_SIGNS(ss) {
-			if (ss->str &&
-					bottom > ss->sign.top &&
+			if (bottom > ss->sign.top &&
 					top < ss->sign.top + 12 &&
 					right > ss->sign.left &&
 					left < ss->sign.left + ss->sign.width_1) {
@@ -920,8 +913,7 @@ static void ViewportAddSigns(DrawPixelInfo *dpi)
 		right += 2;
 		bottom += 2;
 		FOR_ALL_SIGNS(ss) {
-			if (ss->str &&
-					bottom > ss->sign.top &&
+			if (bottom > ss->sign.top &&
 					top < ss->sign.top + 24 &&
 					right > ss->sign.left &&
 					left < ss->sign.left + ss->sign.width_1*2) {
@@ -938,8 +930,7 @@ static void ViewportAddSigns(DrawPixelInfo *dpi)
 		bottom += 5;
 
 		FOR_ALL_SIGNS(ss) {
-			if (ss->str &&
-					bottom > ss->sign.top &&
+			if (bottom > ss->sign.top &&
 					top < ss->sign.top + 24 &&
 					right > ss->sign.left &&
 					left < ss->sign.left + ss->sign.width_2*4) {
@@ -971,8 +962,7 @@ static void ViewportAddWaypoints(DrawPixelInfo *dpi)
 
 	if (dpi->zoom < 1) {
 		FOR_ALL_WAYPOINTS(wp) {
-			if (wp->xy &&
-					bottom > wp->sign.top &&
+			if (bottom > wp->sign.top &&
 					top < wp->sign.top + 12 &&
 					right > wp->sign.left &&
 					left < wp->sign.left + wp->sign.width_1) {
@@ -988,8 +978,7 @@ static void ViewportAddWaypoints(DrawPixelInfo *dpi)
 		right += 2;
 		bottom += 2;
 		FOR_ALL_WAYPOINTS(wp) {
-			if (wp->xy &&
-					bottom > wp->sign.top &&
+			if (bottom > wp->sign.top &&
 					top < wp->sign.top + 24 &&
 					right > wp->sign.left &&
 					left < wp->sign.left + wp->sign.width_1*2) {
@@ -1006,8 +995,7 @@ static void ViewportAddWaypoints(DrawPixelInfo *dpi)
 		bottom += 5;
 
 		FOR_ALL_WAYPOINTS(wp) {
-			if (wp->xy &&
-					bottom > wp->sign.top &&
+			if (bottom > wp->sign.top &&
 					top < wp->sign.top + 24 &&
 					right > wp->sign.left &&
 					left < wp->sign.left + wp->sign.width_2*4) {
@@ -1488,8 +1476,7 @@ static bool CheckClickOnTown(const ViewPort *vp, int x, int y)
 		y = y - vp->top + vp->virtual_top;
 
 		FOR_ALL_TOWNS(t) {
-			if (t->xy &&
-			    y >= t->sign.top &&
+			if (y >= t->sign.top &&
 					y < t->sign.top + 12 &&
 					x >= t->sign.left &&
 					x < t->sign.left + t->sign.width_1) {
@@ -1501,8 +1488,7 @@ static bool CheckClickOnTown(const ViewPort *vp, int x, int y)
 		x = (x - vp->left + 1) * 2 + vp->virtual_left;
 		y = (y - vp->top + 1) * 2 + vp->virtual_top;
 		FOR_ALL_TOWNS(t) {
-			if (t->xy &&
-			    y >= t->sign.top &&
+			if (y >= t->sign.top &&
 					y < t->sign.top + 24 &&
 					x >= t->sign.left &&
 					x < t->sign.left + t->sign.width_1 * 2) {
@@ -1514,8 +1500,7 @@ static bool CheckClickOnTown(const ViewPort *vp, int x, int y)
 		x = (x - vp->left + 3) * 4 + vp->virtual_left;
 		y = (y - vp->top + 3) * 4 + vp->virtual_top;
 		FOR_ALL_TOWNS(t) {
-			if (t->xy &&
-			    y >= t->sign.top &&
+			if (y >= t->sign.top &&
 					y < t->sign.top + 24 &&
 					x >= t->sign.left &&
 					x < t->sign.left + t->sign.width_2 * 4) {
@@ -1539,8 +1524,7 @@ static bool CheckClickOnStation(const ViewPort *vp, int x, int y)
 		y = y - vp->top + vp->virtual_top;
 
 		FOR_ALL_STATIONS(st) {
-			if (st->xy &&
-			    y >= st->sign.top &&
+			if (y >= st->sign.top &&
 					y < st->sign.top + 12 &&
 					x >= st->sign.left &&
 					x < st->sign.left + st->sign.width_1) {
@@ -1552,8 +1536,7 @@ static bool CheckClickOnStation(const ViewPort *vp, int x, int y)
 		x = (x - vp->left + 1) * 2 + vp->virtual_left;
 		y = (y - vp->top + 1) * 2 + vp->virtual_top;
 		FOR_ALL_STATIONS(st) {
-			if (st->xy &&
-			    y >= st->sign.top &&
+			if (y >= st->sign.top &&
 					y < st->sign.top + 24 &&
 					x >= st->sign.left &&
 					x < st->sign.left + st->sign.width_1 * 2) {
@@ -1565,8 +1548,7 @@ static bool CheckClickOnStation(const ViewPort *vp, int x, int y)
 		x = (x - vp->left + 3) * 4 + vp->virtual_left;
 		y = (y - vp->top + 3) * 4 + vp->virtual_top;
 		FOR_ALL_STATIONS(st) {
-			if (st->xy &&
-			    y >= st->sign.top &&
+			if (y >= st->sign.top &&
 					y < st->sign.top + 24 &&
 					x >= st->sign.left &&
 					x < st->sign.left + st->sign.width_2 * 4) {
@@ -1590,8 +1572,7 @@ static bool CheckClickOnSign(const ViewPort *vp, int x, int y)
 		y = y - vp->top + vp->virtual_top;
 
 		FOR_ALL_SIGNS(ss) {
-			if (ss->str &&
-			    y >= ss->sign.top &&
+			if (y >= ss->sign.top &&
 					y < ss->sign.top + 12 &&
 					x >= ss->sign.left &&
 					x < ss->sign.left + ss->sign.width_1) {
@@ -1603,8 +1584,7 @@ static bool CheckClickOnSign(const ViewPort *vp, int x, int y)
 		x = (x - vp->left + 1) * 2 + vp->virtual_left;
 		y = (y - vp->top + 1) * 2 + vp->virtual_top;
 		FOR_ALL_SIGNS(ss) {
-			if (ss->str &&
-			    y >= ss->sign.top &&
+			if (y >= ss->sign.top &&
 					y < ss->sign.top + 24 &&
 					x >= ss->sign.left &&
 					x < ss->sign.left + ss->sign.width_1 * 2) {
@@ -1616,8 +1596,7 @@ static bool CheckClickOnSign(const ViewPort *vp, int x, int y)
 		x = (x - vp->left + 3) * 4 + vp->virtual_left;
 		y = (y - vp->top + 3) * 4 + vp->virtual_top;
 		FOR_ALL_SIGNS(ss) {
-			if (ss->str &&
-			    y >= ss->sign.top &&
+			if (y >= ss->sign.top &&
 					y < ss->sign.top + 24 &&
 					x >= ss->sign.left &&
 					x < ss->sign.left + ss->sign.width_2 * 4) {
@@ -1641,8 +1620,7 @@ static bool CheckClickOnWaypoint(const ViewPort *vp, int x, int y)
 		y = y - vp->top + vp->virtual_top;
 
 		FOR_ALL_WAYPOINTS(wp) {
-			if (wp->xy &&
-			    y >= wp->sign.top &&
+			if (y >= wp->sign.top &&
 					y < wp->sign.top + 12 &&
 					x >= wp->sign.left &&
 					x < wp->sign.left + wp->sign.width_1) {
@@ -1654,8 +1632,7 @@ static bool CheckClickOnWaypoint(const ViewPort *vp, int x, int y)
 		x = (x - vp->left + 1) * 2 + vp->virtual_left;
 		y = (y - vp->top + 1) * 2 + vp->virtual_top;
 		FOR_ALL_WAYPOINTS(wp) {
-			if (wp->xy &&
-			    y >= wp->sign.top &&
+			if (y >= wp->sign.top &&
 					y < wp->sign.top + 24 &&
 					x >= wp->sign.left &&
 					x < wp->sign.left + wp->sign.width_1 * 2) {
@@ -1667,8 +1644,7 @@ static bool CheckClickOnWaypoint(const ViewPort *vp, int x, int y)
 		x = (x - vp->left + 3) * 4 + vp->virtual_left;
 		y = (y - vp->top + 3) * 4 + vp->virtual_top;
 		FOR_ALL_WAYPOINTS(wp) {
-			if (wp->xy &&
-			    y >= wp->sign.top &&
+			if (y >= wp->sign.top &&
 					y < wp->sign.top + 24 &&
 					x >= wp->sign.left &&
 					x < wp->sign.left + wp->sign.width_2 * 4) {

waypoint.c

@@ -35,7 +35,9 @@ static void WaypointPoolNewBlock(uint start_item)
 {
 	Waypoint *wp;
 
-	FOR_ALL_WAYPOINTS_FROM(wp, start_item) wp->index = start_item++;
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (wp = GetWaypoint(start_item); wp != NULL; wp = (wp->index + 1 < GetWaypointPoolSize()) ? GetWaypoint(wp->index + 1) : NULL) wp->index = start_item++;
 }
 
 /* Initialize the town-pool */
@@ -46,8 +48,10 @@ static Waypoint* AllocateWaypoint(void)
 {
 	Waypoint *wp;
 
-	FOR_ALL_WAYPOINTS(wp) {
-		if (wp->xy == 0) {
+	/* We don't use FOR_ALL here, because FOR_ALL skips invalid items.
+	 * TODO - This is just a temporary stage, this will be removed. */
+	for (wp = GetWaypoint(0); wp != NULL; wp = (wp->index + 1 < GetWaypointPoolSize()) ? GetWaypoint(wp->index + 1) : NULL) {
+		if (!IsValidWaypoint(wp)) {
 			uint index = wp->index;
 
 			memset(wp, 0, sizeof(*wp));
@@ -87,7 +91,7 @@ void UpdateAllWaypointSigns(void)
 	Waypoint *wp;
 
 	FOR_ALL_WAYPOINTS(wp) {
-		if (wp->xy != 0) UpdateWaypointSign(wp);
+		UpdateWaypointSign(wp);
 	}
 }
 
@@ -124,7 +128,7 @@ static Waypoint *FindDeletedWaypointCloseTo(TileIndex tile)
 	uint thres = 8;
 
 	FOR_ALL_WAYPOINTS(wp) {
-		if (wp->deleted && wp->xy != 0) {
+		if (wp->deleted) {
 			uint cur_dist = DistanceManhattan(tile, wp->xy);
 
 			if (cur_dist < thres) {
@@ -383,8 +387,6 @@ void FixOldWaypoints(void)
 
 	/* Convert the old 'town_or_string', to 'string' / 'town' / 'town_cn' */
 	FOR_ALL_WAYPOINTS(wp) {
-		if (wp->xy == 0) continue;
-
 		wp->town_index = ClosestTownFromTile(wp->xy, (uint)-1)->index;
 		wp->town_cn = 0;
 		if (wp->string & 0xC000) {
@@ -421,10 +423,8 @@ static void Save_WAYP(void)
 	Waypoint *wp;
 
 	FOR_ALL_WAYPOINTS(wp) {
-		if (wp->xy != 0) {
-			SlSetArrayIndex(wp->index);
-			SlObject(wp, _waypoint_desc);
-		}
+		SlSetArrayIndex(wp->index);
+		SlObject(wp, _waypoint_desc);
 	}
 }
 

waypoint.h

@@ -47,7 +47,15 @@ static inline bool IsWaypointIndex(uint index)
 	return index < GetWaypointPoolSize();
 }
 
-#define FOR_ALL_WAYPOINTS_FROM(wp, start) for (wp = GetWaypoint(start); wp != NULL; wp = (wp->index + 1 < GetWaypointPoolSize()) ? GetWaypoint(wp->index + 1) : NULL)
+/**
+ * Check if a Waypoint really exists.
+ */
+static inline bool IsValidWaypoint(const Waypoint *wp)
+{
+	return wp->xy != 0;
+}
+
+#define FOR_ALL_WAYPOINTS_FROM(wp, start) for (wp = GetWaypoint(start); wp != NULL; wp = (wp->index + 1 < GetWaypointPoolSize()) ? GetWaypoint(wp->index + 1) : NULL) if (IsValidWaypoint(wp))
 #define FOR_ALL_WAYPOINTS(wp) FOR_ALL_WAYPOINTS_FROM(wp, 0)