(svn r6776) -Codechange: Use IsValidPlayer() function to determine of a PlayerID is an

actual playable player (< MAX_PLAYERS) or not.

ai/ai.c

@@ -213,7 +213,7 @@ void AI_RunGameLoop(void)
  */
 void AI_StartNewAI(PlayerID player)
 {
-	assert(player < MAX_PLAYERS);
+	assert(IsValidPlayer(player));
 
 	/* Called if a new AI is booted */
 	_ai_player[player].active = true;

command.c

@@ -383,7 +383,7 @@ error:
 	if (--_docommand_recursive == 0) {
 		SubtractMoneyFromPlayer(res);
 		// XXX - Old AI hack which doesn't use DoCommandDP; update last build coord of player
-		if (tile != 0 && _current_player < MAX_PLAYERS) {
+		if (tile != 0 && IsValidPlayer(_current_player)) {
 			GetPlayer(_current_player)->last_build_coordinate = tile;
 		}
 	}
@@ -395,7 +395,7 @@ error:
 int32 GetAvailableMoneyForCommand(void)
 {
 	PlayerID pid = _current_player;
-	if (pid >= MAX_PLAYERS) return 0x7FFFFFFF; // max int
+	if (!IsValidPlayer(pid)) return 0x7FFFFFFF; // max int
 	return GetPlayer(pid)->player_money;
 }
 
@@ -508,7 +508,7 @@ bool DoCommandP(TileIndex tile, uint32 p1, uint32 p2, CommandCallback *callback,
 #endif /* ENABLE_NETWORK */
 
 	// update last build coordinate of player.
-	if (tile != 0 && _current_player < MAX_PLAYERS) {
+	if (tile != 0 && IsValidPlayer(_current_player)) {
 		GetPlayer(_current_player)->last_build_coordinate = tile;
 	}
 

console_cmds.c

@@ -1290,12 +1290,12 @@ DEF_CONSOLE_HOOK(ConHookRconPW)
 bool NetworkChangeCompanyPassword(byte argc, char *argv[])
 {
 	if (argc == 0) {
-		if (_local_player >= MAX_PLAYERS) return true; // dedicated server
+		if (!IsValidPlayer(_local_player)) return true; // dedicated server
 		IConsolePrintF(_icolour_warn, "Current value for 'company_pw': %s", _network_player_info[_local_player].password);
 		return true;
 	}
 
-	if (_local_player >= MAX_PLAYERS) {
+	if (!IsValidPlayer(_local_player)) {
 		IConsoleError("You have to own a company to make use of this command.");
 		return false;
 	}

economy.c

@@ -1543,7 +1543,7 @@ int32 CmdBuyShareInCompany(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 	int64 cost;
 
 	/* Check if buying shares is allowed (protection against modified clients */
-	if (p1 >= MAX_PLAYERS || !_patches.allow_shares) return CMD_ERROR;
+	if (!IsValidPlayer((PlayerID)p1) || !_patches.allow_shares) return CMD_ERROR;
 
 	SET_EXPENSES_TYPE(EXPENSES_OTHER);
 	p = GetPlayer(p1);
@@ -1588,7 +1588,7 @@ int32 CmdSellShareInCompany(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 	int64 cost;
 
 	/* Check if buying shares is allowed (protection against modified clients */
-	if (p1 >= MAX_PLAYERS || !_patches.allow_shares) return CMD_ERROR;
+	if (!IsValidPlayer((PlayerID)p1) || !_patches.allow_shares) return CMD_ERROR;
 
 	SET_EXPENSES_TYPE(EXPENSES_OTHER);
 	p = GetPlayer(p1);
@@ -1622,7 +1622,7 @@ int32 CmdBuyCompany(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 	Player *p;
 
 	/* Disable takeovers in multiplayer games */
-	if (p1 >= MAX_PLAYERS || _networking) return CMD_ERROR;
+	if (!IsValidPlayer((PlayerID)p1) || _networking) return CMD_ERROR;
 
 	SET_EXPENSES_TYPE(EXPENSES_OTHER);
 	p = GetPlayer(p1);

misc_cmd.c

@@ -291,7 +291,7 @@ int32 CmdGiveMoney(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 
 	/* You can only transfer funds that is in excess of your loan */
 	if (p->money64 - p->current_loan < amount || amount <= 0) return CMD_ERROR;
-	if (!_networking || p2 >= MAX_PLAYERS) return CMD_ERROR;
+	if (!_networking || !IsValidPlayer((PlayerID)p2)) return CMD_ERROR;
 
 	if (flags & DC_EXEC) {
 		/* Add money to player */

misc_gui.c

@@ -155,7 +155,7 @@ static void Place_LandInfo(TileIndex tile)
 	lid.tile = tile;
 	lid.town = ClosestTownFromTile(tile, _patches.dist_local_authority);
 
-	p = GetPlayer(_local_player < MAX_PLAYERS ? _local_player : 0);
+	p = GetPlayer(IsValidPlayer(_local_player) ? _local_player : 0);
 
 	old_money = p->money64;
 	p->money64 = p->player_money = 0x7fffffff;
@@ -1306,7 +1306,7 @@ static void GenerateFileName(void)
 {
 	/* Check if we are not a specatator who wants to generate a name..
 	    Let's use the name of player #0 for now. */
-	const Player *p = GetPlayer(_local_player < MAX_PLAYERS ? _local_player : 0);
+	const Player *p = GetPlayer(IsValidPlayer(_local_player) ? _local_player : 0);
 
 	SetDParam(0, p->name_1);
 	SetDParam(1, p->name_2);
@@ -1636,9 +1636,9 @@ static int32 ClickMoneyCheat(int32 p1, int32 p2)
 // p1 player to set to, p2 is -1 or +1 (down/up)
 static int32 ClickChangePlayerCheat(int32 p1, int32 p2)
 {
-	while (p1 >= 0 && p1 < MAX_PLAYERS) {
+	while (IsValidPlayer((PlayerID)p1)) {
 		if (_players[p1].is_active) {
-			_local_player = p1;
+			_local_player = (PlayerID)p1;
 			MarkWholeScreenDirty();
 			return _local_player;
 		}

network_server.c

@@ -1232,7 +1232,7 @@ void NetworkPopulateCompanyInfo(void)
 
 	// Go through all vehicles and count the type of vehicles
 	FOR_ALL_VEHICLES(v) {
-		if (v->owner >= MAX_PLAYERS) continue;
+		if (!IsValidPlayer(v->owner)) continue;
 		switch (v->type) {
 			case VEH_Train:
 				if (IsFrontEngine(v)) {
@@ -1266,7 +1266,7 @@ void NetworkPopulateCompanyInfo(void)
 
 	// Go through all stations and count the types of stations
 	FOR_ALL_STATIONS(s) {
-		if (s->owner < MAX_PLAYERS) {
+		if (IsValidPlayer(s->owner)) {
 			NetworkPlayerInfo* npi = &_network_player_info[s->owner];
 
 			if (s->facilities & FACIL_TRAIN)      npi->num_station[0]++;

openttd.c

@@ -1257,7 +1257,7 @@ bool AfterLoadGame(void)
 			p->engine_renew_months = -6;
 			p->engine_renew_money = 100000;
 		}
-		if (_local_player < MAX_PLAYERS) {
+		if (IsValidPlayer(_local_player)) {
 			// Set the human controlled player to the patch settings
 			// Scenario editor do not have any companies
 			p = GetPlayer(_local_player);

player.h

@@ -245,6 +245,11 @@ static inline bool IsLocalPlayer(void)
 	return _local_player == _current_player;
 }
 
+static inline bool IsValidPlayer(PlayerID pi)
+{
+	return pi < MAX_PLAYERS;
+}
+
 void DeletePlayerWindows(PlayerID pi);
 byte GetPlayerRailtypes(PlayerID p);
 

players.c

@@ -204,7 +204,7 @@ bool CheckPlayerHasMoney(int32 cost)
 {
 	if (cost > 0) {
 		PlayerID pid = _current_player;
-		if (pid < MAX_PLAYERS && cost > GetPlayer(pid)->player_money) {
+		if (IsValidPlayer(pid) && cost > GetPlayer(pid)->player_money) {
 			SetDParam(0, cost);
 			_error_message = STR_0003_NOT_ENOUGH_CASH_REQUIRES;
 			return false;
@@ -233,7 +233,7 @@ void SubtractMoneyFromPlayer(int32 cost)
 {
 	PlayerID pid = _current_player;
 
-	if (pid < MAX_PLAYERS) SubtractMoneyFromAnyPlayer(GetPlayer(pid), cost);
+	if (IsValidPlayer(pid)) SubtractMoneyFromAnyPlayer(GetPlayer(pid), cost);
 }
 
 void SubtractMoneyFromPlayerFract(PlayerID player, int32 cost)
@@ -264,7 +264,7 @@ void GetNameOfOwner(Owner owner, TileIndex tile)
 	SetDParam(2, owner);
 
 	if (owner != OWNER_TOWN) {
-		if (owner >= MAX_PLAYERS) {
+		if (!IsValidPlayer(owner)) {
 			SetDParam(0, STR_0150_SOMEONE);
 		} else {
 			const Player* p = GetPlayer(owner);
@@ -584,7 +584,7 @@ void OnTick_Players(void)
 // index is the next parameter in _decode_parameters to set up
 StringID GetPlayerNameString(PlayerID player, uint index)
 {
-	if (IsHumanPlayer(player) && player < MAX_PLAYERS) {
+	if (IsHumanPlayer(player) && IsValidPlayer(player)) {
 		SetDParam(index, player+1);
 		return STR_7002_PLAYER;
 	}
@@ -691,8 +691,7 @@ static void DeletePlayerStuff(PlayerID pi)
 int32 CmdSetAutoReplace(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 {
 	Player *p;
-	if (!(_current_player < MAX_PLAYERS))
+	if (!IsValidPlayer(_current_player)) return CMD_ERROR;
-		return CMD_ERROR;
 
 	p = GetPlayer(_current_player);
 	switch (GB(p1, 0, 3)) {
@@ -824,7 +823,7 @@ int32 CmdPlayerCtrl(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 		Player *p;
 		PlayerID pid = p2;
 
-		if (!(flags & DC_EXEC) || pid >= MAX_PLAYERS) return 0;
+		if (!(flags & DC_EXEC) || !IsValidPlayer(pid)) return 0;
 
 		p = DoStartupNewPlayer(false);
 
@@ -910,7 +909,7 @@ int32 CmdPlayerCtrl(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 	case 2: { /* Delete a player */
 		Player *p;
 
-		if (p2 >= MAX_PLAYERS) return CMD_ERROR;
+		if (!IsValidPlayer(p2)) return CMD_ERROR;
 
 		if (!(flags & DC_EXEC)) return 0;
 
@@ -938,7 +937,7 @@ int32 CmdPlayerCtrl(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 		PlayerID pid_old = GB(p2,  0, 16);
 		PlayerID pid_new = GB(p2, 16, 16);
 
-		if (pid_old >= MAX_PLAYERS || pid_new >= MAX_PLAYERS) return CMD_ERROR;
+		if (!IsValidPlayer(pid_old) || !IsValidPlayer(pid_new)) return CMD_ERROR;
 
 		if (!(flags & DC_EXEC)) return CMD_ERROR;
 

road_cmd.c

@@ -45,7 +45,7 @@ static bool CheckAllowRemoveRoad(TileIndex tile, RoadBits remove, bool* edge_roa
 	if (_game_mode == GM_EDITOR) return true;
 
 	// Only do the special processing for actual players.
-	if (_current_player >= MAX_PLAYERS) return true;
+	if (!IsValidPlayer(_current_player)) return true;
 
 	owner = IsLevelCrossingTile(tile) ? GetCrossingRoadOwner(tile) : GetTileOwner(tile);
 
@@ -289,7 +289,7 @@ int32 CmdBuildRoad(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 
 	/* Road pieces are max 4 bitset values (NE, NW, SE, SW) and town can only be non-zero
 	 * if a non-player is building the road */
-	if ((p1 >> 4) || (_current_player < MAX_PLAYERS && p2 != 0) || !IsValidTownID(p2)) return CMD_ERROR;
+	if ((p1 >> 4) || (IsValidPlayer(_current_player) && p2 != 0) || !IsValidTownID(p2)) return CMD_ERROR;
 	pieces = p1;
 
 	tileh = GetTileSlope(tile, NULL);

station_cmd.c

@@ -1024,7 +1024,7 @@ int32 CmdBuildRailroadStation(TileIndex tile_org, uint32 flags, uint32 p1, uint3
 		if (st == NULL) return CMD_ERROR;
 
 		st->town = ClosestTownFromTile(tile_org, (uint)-1);
-		if (_current_player < MAX_PLAYERS && flags & DC_EXEC)
+		if (IsValidPlayer(_current_player) && (flags & DC_EXEC))
 			SETBIT(st->town->have_ratings, _current_player);
 
 		if (!GenerateStationName(st, tile_org, 0)) return CMD_ERROR;
@@ -1452,7 +1452,7 @@ int32 CmdBuildRoadStop(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 
 		FindRoadStopSpot(type, st, &currstop, &prev);
 
-		if (_current_player < MAX_PLAYERS && flags & DC_EXEC) {
+		if (IsValidPlayer(_current_player) && (flags & DC_EXEC)) {
 			SETBIT(t->have_ratings, _current_player);
 		}
 
@@ -1698,7 +1698,7 @@ int32 CmdBuildAirport(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 
 		st->town = t;
 
-		if (_current_player < MAX_PLAYERS && flags & DC_EXEC)
+		if (IsValidPlayer(_current_player) && (flags & DC_EXEC))
 			SETBIT(t->have_ratings, _current_player);
 
 		st->sign.width_1 = 0;
@@ -1859,10 +1859,8 @@ static int32 RemoveBuoy(Station *st, uint32 flags)
 {
 	TileIndex tile;
 
-	if (_current_player >= MAX_PLAYERS) {
+	/* XXX: strange stuff */
-		/* XXX: strange stuff */
+	if (!IsValidPlayer(_current_player))  return_cmd_error(INVALID_STRING_ID);
-		return_cmd_error(INVALID_STRING_ID);
-	}
 
 	tile = st->dock_tile;
 
@@ -1967,7 +1965,7 @@ int32 CmdBuildDock(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 
 		st->town = t = ClosestTownFromTile(tile, (uint)-1);
 
-		if (_current_player < MAX_PLAYERS && flags&DC_EXEC)
+		if (IsValidPlayer(_current_player) && (flags & DC_EXEC))
 			SETBIT(t->have_ratings, _current_player);
 
 		st->sign.width_1 = 0;
@@ -2044,7 +2042,7 @@ static void DrawTile_Station(TileInfo *ti)
 	PlayerID owner = GetTileOwner(ti->tile);
 	uint32 palette;
 
-	if (owner < MAX_PLAYERS) {
+	if (IsValidPlayer(owner)) {
 		palette = PLAYER_SPRITE_COLOR(owner);
 	} else {
 		// Some stations are not owner by a player, namely oil rigs
@@ -2400,7 +2398,7 @@ void DeleteAllPlayerStations(void)
 	Station *st;
 
 	FOR_ALL_STATIONS(st) {
-		if (st->owner < MAX_PLAYERS) DeleteStation(st);
+		if (IsValidPlayer(st->owner)) DeleteStation(st);
 	}
 }
 
@@ -2448,8 +2446,7 @@ static void UpdateStationRating(Station *st)
 				(rating += 13, true);
 			}
 
-			if (st->owner < MAX_PLAYERS && HASBIT(st->town->statues, st->owner))
+			if (IsValidPlayer(st->owner) && HASBIT(st->town->statues, st->owner)) rating += 26;
-				rating += 26;
 
 			{
 				byte days = ge->days_since_pickup;

town_cmd.c

@@ -355,7 +355,7 @@ static int32 ClearTile_Town(TileIndex tile, byte flags)
 	_cleared_town_rating += rating;
 	_cleared_town = t = GetTownByTile(tile);
 
-	if (_current_player < MAX_PLAYERS) {
+	if (IsValidPlayer(_current_player)) {
 		if (rating > t->ratings[_current_player] && !(flags & DC_NO_TOWN_RATING) && !_cheats.magic_bulldozer.value) {
 			SetDParam(0, t->index);
 			return_cmd_error(STR_2009_LOCAL_AUTHORITY_REFUSES);
@@ -1630,10 +1630,10 @@ static void UpdateTownGrowRate(Town *t)
 		if (DistanceSquare(st->xy, t->xy) <= t->radius[0]) {
 			if (st->time_since_load <= 20 || st->time_since_unload <= 20) {
 				n++;
-				if (st->owner < MAX_PLAYERS && t->ratings[st->owner] <= 1000-12)
+				if (IsValidPlayer(st->owner) && t->ratings[st->owner] <= 1000-12)
 					t->ratings[st->owner] += 12;
 			} else {
-				if (st->owner < MAX_PLAYERS && t->ratings[st->owner] >= -1000+15)
+				if (IsValidPlayer(st->owner) && t->ratings[st->owner] >= -1000+15)
 					t->ratings[st->owner] -= 15;
 			}
 		}
@@ -1704,7 +1704,7 @@ bool CheckIfAuthorityAllows(TileIndex tile)
 {
 	Town *t;
 
-	if (_current_player >= MAX_PLAYERS) return true;
+	if (!IsValidPlayer(_current_player)) return true;
 
 	t = ClosestTownFromTile(tile, _patches.dist_local_authority);
 	if (t == NULL) return true;
@@ -1755,7 +1755,7 @@ void ChangeTownRating(Town *t, int add, int max)
 
 	// if magic_bulldozer cheat is active, town doesn't penaltize for removing stuff
 	if (t == NULL ||
-			_current_player >= MAX_PLAYERS ||
+			!IsValidPlayer(_current_player) ||
 			(_cheats.magic_bulldozer.value && add < 0)) {
 		return;
 	}
@@ -1791,7 +1791,7 @@ bool CheckforTownRating(uint32 flags, Town *t, byte type)
 	int modemod;
 
 	// if magic_bulldozer cheat is active, town doesn't restrict your destructive actions
-	if (t == NULL || _current_player >= MAX_PLAYERS || _cheats.magic_bulldozer.value)
+	if (t == NULL || !IsValidPlayer(_current_player) || _cheats.magic_bulldozer.value)
 		return true;
 
 	/* check if you're allowed to remove the street/bridge/tunnel/industry

tree_cmd.c

@@ -265,7 +265,7 @@ int32 CmdPlantTree(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 						TreeType treetype;
 						uint growth;
 
-						if (_game_mode != GM_EDITOR && _current_player < MAX_PLAYERS) {
+						if (_game_mode != GM_EDITOR && IsValidPlayer(_current_player)) {
 							Town *t = ClosestTownFromTile(tile, _patches.dist_local_authority);
 							if (t != NULL)
 								ChangeTownRating(t, RATING_TREE_UP_STEP, RATING_TREE_MAXIMUM);
@@ -415,7 +415,7 @@ static int32 ClearTile_Trees(TileIndex tile, byte flags)
 {
 	uint num;
 
-	if (flags & DC_EXEC && _current_player < MAX_PLAYERS) {
+	if ((flags & DC_EXEC) && IsValidPlayer(_current_player)) {
 		Town *t = ClosestTownFromTile(tile, _patches.dist_local_authority);
 		if (t != NULL)
 			ChangeTownRating(t, RATING_TREE_DOWN_STEP, RATING_TREE_MINIMUM);

tunnelbridge_cmd.c

@@ -423,7 +423,7 @@ not_valid_below:;
 
 		bridge_len += 2; // begin and end tiles/ramps
 
-		if (_current_player < MAX_PLAYERS && !_is_old_ai_player)
+		if (IsValidPlayer(_current_player) && !_is_old_ai_player)
 			bridge_len = CalcBridgeLenCostFactor(bridge_len);
 
 		cost += (int64)bridge_len * _price.build_bridge * b->price >> 8;