(svn r2297) - CodeChange: server-check the next batch of commands.

- CodeChange: since only the server will be able to modify difficulty settings, leave the checking of correct values besides, and trust users will join legit servers.
- CodeChange: for renaming signs, only check if GetDParam(); eg _decode_parameters is empty ('\0') or not, instead of the extra check of players, etc. That basically does the same thing. Also dirty sign two times when renaming, once before, once after the action. Because if the name becomes shorter and you update only after, garbage remains on the screen.
- CodeChange: made GetMaskOfTownActions() available to the town-cmd to double-check if the action was available to the player. For this purpose the hardcoded _local_player has been removed from the function and is now passed as a parameter.

13 files changed, 341 insertions, 233 deletions

diff --git a/aircraft_cmd.c b/aircraft_cmd.c
index 423e97c45..b351fca6f 100644
--- a/aircraft_cmd.c
+++ b/aircraft_cmd.c
@@ -41,11 +41,11 @@ static const SpriteID _aircraft_sprite[] = {
  * INVALID_STATION is returned, if the player does not have any suitable
  * airports (like helipads only)
  */
-static uint16 FindNearestHangar(const Vehicle *v)
+static StationID FindNearestHangar(const Vehicle *v)
 {
 	const Station *st;
 	uint best = 0;
-	uint16 index = INVALID_STATION;
+	StationID index = INVALID_STATION;
 
 	FOR_ALL_STATIONS(st) {
 		if (st->owner == v->owner && st->facilities & FACIL_AIRPORT &&
@@ -153,13 +153,17 @@ static bool AllocateVehicles(Vehicle **vl, int num)
 	return success;
 }
 
-int32 EstimateAircraftCost(uint16 engine_type)
+int32 EstimateAircraftCost(EngineID engine_type)
 {
 	return AircraftVehInfo(engine_type)->base_cost * (_price.aircraft_base>>3)>>5;
 }
 
 
-/* p1 = engine */
+/** Build an aircraft.
+ * @param x,y tile coordinates of depot where aircraft is built
+ * @param p1 aircraft type being built (engine)
+ * @param p2 unused
+ */
 int32 CmdBuildAircraft(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	int32 value;
@@ -180,8 +184,7 @@ int32 CmdBuildAircraft(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 
 	value = EstimateAircraftCost(p1);
 
-	if (flags & DC_QUERY_COST)
-		return value;
+	if (flags & DC_QUERY_COST) return value;
 
 	// allocate 2 or 3 vehicle structs, depending on type
 	if (!AllocateVehicles(vl, (avi->subtype & 1) == 0 ? 3 : 2) ||
@@ -299,7 +302,7 @@ int32 CmdBuildAircraft(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 		VehiclePositionChanged(v);
 		VehiclePositionChanged(u);
 
-		// Aircraft with 3 vehicles?
+		// Aircraft with 3 vehicles (chopper)?
 		if (v->subtype == 0) {
 			w = vl[2];
 
@@ -323,10 +326,9 @@ int32 CmdBuildAircraft(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 		InvalidateWindow(WC_VEHICLE_DEPOT, v->tile);
 		RebuildVehicleLists();
 		InvalidateWindow(WC_COMPANY, v->owner);
+		InvalidateWindow(WC_REPLACE_VEHICLE, VEH_Aircraft); //updates the replace Aircraft window
 	}
 
-	InvalidateWindow(WC_REPLACE_VEHICLE, VEH_Aircraft); //updates the replace Aircraft window
-
 	return value;
 }
 
@@ -381,16 +383,15 @@ int32 CmdSellAircraft(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 		// Invalidate depot
 		InvalidateWindow(WC_VEHICLE_DEPOT, v->tile);
 		DoDeleteAircraft(v);
+		InvalidateWindow(WC_REPLACE_VEHICLE, VEH_Aircraft); // updates the replace Aircraft window
 	}
 
-	InvalidateWindow(WC_REPLACE_VEHICLE, VEH_Aircraft); // updates the replace Aircraft window
-
 	return -(int32)v->value;
 }
 
 /** Start/Stop an aircraft.
  * @param x,y unused
- * @param p1 aircraft to start/stop
+ * @param p1 aircraft ID to start/stop
  * @param p2 unused
  */
 int32 CmdStartStopAircraft(int x, int y, uint32 flags, uint32 p1, uint32 p2)
@@ -417,20 +418,23 @@ int32 CmdStartStopAircraft(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	return 0;
 }
 
-// p1 = vehicle
-// p2 = if set, the aircraft will try to goto a depot, but not stop
+/** Send an aircraft to the hangar.
+ * @param x,y unused
+ * @param p1 vehicle ID to send to the hangar
+ * @param p2 various bitmasked elements
+ * - p2 = 0      - aircraft goes to the depot and stays there (user command)
+ * - p2 non-zero - aircraft will try to goto a depot, but not stop there (eg forced servicing)
+ * - p2 (bit 17) - aircraft will try to goto a depot at the next airport
+ */
 int32 CmdSendAircraftToHangar(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	Vehicle *v;
-	Station *st;
-	uint16 next_airport_index;
 
 	if (!IsVehicleIndex(p1)) return CMD_ERROR;
 
 	v = GetVehicle(p1);
 
-	if (v->type != VEH_Aircraft || !CheckOwnership(v->owner))
-		return CMD_ERROR;
+	if (v->type != VEH_Aircraft || !CheckOwnership(v->owner)) return CMD_ERROR;
 
 	if (v->current_order.type == OT_GOTO_DEPOT && p2 == 0) {
 		if (flags & DC_EXEC) {
@@ -441,23 +445,23 @@ int32 CmdSendAircraftToHangar(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 		}
 	} else {
 		bool next_airport_has_hangar = true;
-		next_airport_index = (HASBIT(p2, 17)) ? (int16)p2 : v->u.air.targetairport;
-		st = GetStation(next_airport_index);
+		/* XXX - I don't think p2 is any valid station cause all calls use either 0, 1, or 1<<16!!!!!!!!! */
+		StationID next_airport_index = (HASBIT(p2, 17)) ? (StationID)p2 : v->u.air.targetairport;
+		const Station *st = GetStation(next_airport_index);
 		// If an airport doesn't have terminals (so no landing space for airports),
 		// it surely doesn't have any hangars
-		if (st->xy == 0 || st->airport_tile == 0 ||
-				GetAirport(st->airport_type)->nof_depots == 0) {
-			if (p2 == 0) {
-				// the aircraft has to search for a hangar on its own
-				uint16 station = FindNearestHangar(v);
-
-				next_airport_has_hangar = false;
-				if (station == INVALID_STATION) return CMD_ERROR;
-				st = GetStation(station);
-				next_airport_index = station;
-			} else {
-				return CMD_ERROR;
-			}
+		if (!IsValidStation(st) || st->airport_tile == 0 || GetAirport(st->airport_type)->nof_depots == 0) {
+			StationID station;
+
+			if (p2 != 0) return CMD_ERROR;
+			// the aircraft has to search for a hangar on its own
+			station = FindNearestHangar(v);
+
+			next_airport_has_hangar = false;
+			if (station == INVALID_STATION) return CMD_ERROR;
+			st = GetStation(station);
+			next_airport_index = station;
+
 		}
 
 		if (flags & DC_EXEC) {
@@ -1594,7 +1598,7 @@ static void AircraftEventHandler_Landing(Vehicle *v, const AirportFTAClass *Airp
 		// only the vehicle owner needs to calculate the rest (locally)
 		if ((_autoreplace_array[v->engine_type] != v->engine_type) ||
 			(_patches.autorenew && v->age - v->max_age > (_patches.autorenew_months * 30))) {
-			// send the aircraft to the hangar at next airport
+			// send the aircraft to the hangar at next airport (bit 17 set)
 			_current_player = _local_player;
 			DoCommandP(v->tile, v->index, 1 << 16, NULL, CMD_SEND_AIRCRAFT_TO_HANGAR | CMD_SHOW_NO_ERROR);
 			_current_player = OWNER_NONE;
diff --git a/economy.c b/economy.c
index d77611f71..8e2c27f02 100644
--- a/economy.c
+++ b/economy.c
@@ -1527,41 +1527,40 @@ static void DoAcquireCompany(Player *p)
 
 extern int GetAmountOwnedBy(Player *p, byte owner);
 
+/** Acquire shares in an opposing company.
+ * @param x,y unused
+ * @param p1 player to buy the shares from
+ * @param p2 unused
+ */
 int32 CmdBuyShareInCompany(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	Player *p;
 	int64 cost;
-	byte *b;
-	int i;
+
+	/* Check if buying shares is allowed (protection against modified clients */
+	if (p1 >= MAX_PLAYERS || !_patches.allow_shares) return CMD_ERROR;
 
 	SET_EXPENSES_TYPE(EXPENSES_OTHER);
 	p = DEREF_PLAYER(p1);
 
-
-	if (_cur_year - p->inaugurated_year < 6) {
-		_error_message = STR_7080_PROTECTED;
-		return CMD_ERROR;
-	}
-
-	/* Check if buying shares is allowed (protection against modified clients */
-	if (!_patches.allow_shares)
-		return CMD_ERROR;
+	/* Protect new companies from hostile takeovers */
+	if (_cur_year - p->inaugurated_year < 6) return_cmd_error(STR_7080_PROTECTED);
 
 	/* Those lines are here for network-protection (clients can be slow) */
-	if (GetAmountOwnedBy(p, OWNER_SPECTATOR) == 0)
-		return 0;
+	if (GetAmountOwnedBy(p, OWNER_SPECTATOR) == 0) return 0;
 
-	/* We can not buy out a real player in networking */
-	if (GetAmountOwnedBy(p, OWNER_SPECTATOR) == 1 && !p->is_ai)
-		return 0;
+	/* We can not buy out a real player (temporarily). TODO: well, enable it obviously */
+	if (GetAmountOwnedBy(p, OWNER_SPECTATOR) == 1 && !p->is_ai) return 0;
 
 	cost = CalculateCompanyValue(p) >> 2;
 	if (flags & DC_EXEC) {
-		b = p->share_owners;
+		int i;
+		byte *b = p->share_owners;
+
 		while (*b != 0xFF) b++; /* share owners is guaranteed to contain at least one 0xFF */
 		*b = _current_player;
 
-		for(i=0;p->share_owners[i] == _current_player;) {
+		for (i = 0; p->share_owners[i] == _current_player;) {
 			if (++i == 4) {
 				p->bankrupt_value = 0;
 				DoAcquireCompany(p);
@@ -1573,29 +1572,31 @@ int32 CmdBuyShareInCompany(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	return cost;
 }
 
+/** Sell shares in an opposing company.
+ * @param x,y unused
+ * @param p1 player to sell the shares from
+ * @param p2 unused
+ */
 int32 CmdSellShareInCompany(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	Player *p;
 	int64 cost;
-	byte *b;
+
+	/* Check if buying shares is allowed (protection against modified clients */
+	if (p1 >= MAX_PLAYERS || !_patches.allow_shares) return CMD_ERROR;
 
 	SET_EXPENSES_TYPE(EXPENSES_OTHER);
 	p = DEREF_PLAYER(p1);
 
-	/* Check if selling shares is allowed (protection against modified clients */
-	if (!_patches.allow_shares)
-		return CMD_ERROR;
-
 	/* Those lines are here for network-protection (clients can be slow) */
-	if (GetAmountOwnedBy(p, _current_player) == 0)
-		return 0;
+	if (GetAmountOwnedBy(p, _current_player) == 0) return 0;
 
 	/* adjust it a little to make it less profitable to sell and buy */
 	cost = CalculateCompanyValue(p) >> 2;
 	cost = -(cost - (cost >> 7));
 
 	if (flags & DC_EXEC) {
-		b = p->share_owners;
+		byte *b = p->share_owners;
 		while (*b != _current_player) b++; /* share owners is guaranteed to contain player */
 		*b = 0xFF;
 		InvalidateWindow(WC_COMPANY, (int)p1);
@@ -1603,12 +1604,27 @@ int32 CmdSellShareInCompany(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	return cost;
 }
 
+/** Buy up another company.
+ * When a competing company is gone bankrupt you get the chance to purchase
+ * that company.
+ * @todo currently this only works for AI players
+ * @param x,y unused
+ * @param p1 player/company to buy up
+ * @param p2 unused
+ */
 int32 CmdBuyCompany(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	Player *p;
+
+	/* Disable takeovers in multiplayer games */
+	if (p1 >= MAX_PLAYERS || _networking) return CMD_ERROR;
+
 	SET_EXPENSES_TYPE(EXPENSES_OTHER);
 	p = DEREF_PLAYER(p1);
-	if (flags & 1) {
+
+	if (!p->is_ai) return CMD_ERROR;
+
+	if (flags & DC_EXEC) {
 		DoAcquireCompany(p);
 	}
 	return p->bankrupt_value;
diff --git a/landscape.c b/landscape.c
index 46c1e13c1..755e3a48f 100644
--- a/landscape.c
+++ b/landscape.c
@@ -287,14 +287,20 @@ int32 CmdLandscapeClear(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	return _tile_type_procs[GetTileType(tile)]->clear_tile_proc(tile, flags);
 }
 
-// p1 = end tile
+/** Clear a big piece of landscape
+ * @param x,y end coordinates of area dragging
+ * @param p1 start tile of area dragging
+ * @param p2 unused
+ */
 int32 CmdClearArea(int ex, int ey, uint32 flags, uint32 p1, uint32 p2)
 {
-	int32 cost,ret, money;
+	int32 cost, ret, money;
 	int sx,sy;
 	int x,y;
 	bool success = false;
 
+	if (p1 > MapSize()) return CMD_ERROR;
+
 	SET_EXPENSES_TYPE(EXPENSES_CONSTRUCTION);
 
 	// make sure sx,sy are smaller than ex,ey
@@ -306,22 +312,22 @@ int32 CmdClearArea(int ex, int ey, uint32 flags, uint32 p1, uint32 p2)
 	money = GetAvailableMoneyForCommand();
 	cost = 0;
 
-	for(x=sx; x<=ex; x+=16) {
-		for(y=sy; y<=ey; y+=16) {
+	for (x = sx; x <= ex; x += 16) {
+		for (y = sy; y <= ey; y += 16) {
 			ret = DoCommandByTile(TILE_FROM_XY(x,y), 0, 0, flags &~DC_EXEC, CMD_LANDSCAPE_CLEAR);
-			if (ret == CMD_ERROR) continue;
+			if (CmdFailed(ret)) continue;
 			cost += ret;
 			success = true;
 
 			if (flags & DC_EXEC) {
-				if ( ret>0 && (money -= ret) < 0) {
+				if (ret > 0 && (money -= ret) < 0) {
 					_additional_cash_required = ret;
 					return cost - ret;
 				}
 				DoCommandByTile(TILE_FROM_XY(x,y), 0, 0, flags, CMD_LANDSCAPE_CLEAR);
 
 				// draw explosion animation...
-				if ((x==sx || x==ex) && (y==sy || y==ey)) {
+				if ((x == sx || x == ex) && (y == sy || y == ey)) {
 					// big explosion in each corner, or small explosion for single tiles
 					CreateEffectVehicleAbove(x + 8, y + 8, 2,
 						sy == ey && sx == ex ? EV_EXPLOSION_SMALL : EV_EXPLOSION_LARGE
@@ -331,9 +337,7 @@ int32 CmdClearArea(int ex, int ey, uint32 flags, uint32 p1, uint32 p2)
 		}
 	}
 
-	if (!success)
-		cost = CMD_ERROR;
-	return cost;
+	return (success) ? cost : CMD_ERROR;
 }
 
 
diff --git a/main_gui.c b/main_gui.c
index c7b30a5e3..39efb8f35 100644
--- a/main_gui.c
+++ b/main_gui.c
@@ -69,7 +69,7 @@ void HandleOnEditText(WindowEvent *e) {
 	switch(_rename_what) {
 	case 0:
 		// for empty string send "remove sign" parameter
-		DoCommandP(0, id, (*b==0)?OWNER_NONE:_current_player, NULL, CMD_RENAME_SIGN | CMD_MSG(STR_280C_CAN_T_CHANGE_SIGN_NAME));
+		DoCommandP(0, id, 0, NULL, CMD_RENAME_SIGN | CMD_MSG(STR_280C_CAN_T_CHANGE_SIGN_NAME));
 		break;
 	case 1:
 		if(*b == 0)
diff --git a/misc_cmd.c b/misc_cmd.c
index 52f237ba2..ea7a6ecb6 100644
--- a/misc_cmd.c
+++ b/misc_cmd.c
@@ -172,8 +172,14 @@ int32 CmdChangePresidentName(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	return 0;
 }
 
-// p1 = 0   decrease pause counter
-// p1 = 1   increase pause counter
+/** Pause/Unpause the game (server-only).
+ * Increase or decrease the pause counter. If the counter is zero,
+ * the game is unpaused. A counter is used instead of a boolean value
+ * to have more control over the game when saving/loading, etc.
+ * @param x,y unused
+ * @param p1 0 = decrease pause counter; 1 = increase pause counter
+ * @param p2 unused
+ */
 int32 CmdPause(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	if (flags & DC_EXEC) {
@@ -213,16 +219,29 @@ int32 CmdGiveMoney(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	return (int32)p1;
 }
 
+/** Change difficulty level/settings (server-only).
+ * We cannot really check for valid values of p2 (too much work mostly); stored
+ * in file 'settings_gui.c' _game_setting_info[]; we'll just trust the server it knows
+ * what to do and does this correctly
+ * @param x,y unused
+ * @param p1 the difficulty setting being changed. If it is -1, the difficulty level
+ *           itself is changed. The new value is inside p2
+ * @param p2 new value for a difficulty setting or difficulty level
+ */
 int32 CmdChangeDifficultyLevel(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
+	if (p1 >= GAME_DIFFICULTY_NUM) return CMD_ERROR;
+
 	if (flags & DC_EXEC) {
 		if (p1 != (uint32)-1L) {
 			((int*)&_opt_ptr->diff)[p1] = p2;
-			_opt_ptr->diff_level = 3;
+			_opt_ptr->diff_level = 3; // custom difficulty level
 		} else
 			_opt_ptr->diff_level = p2;
 
-		// If we are a network-client, update the difficult setting (if it is open)
+		/* If we are a network-client, update the difficult setting (if it is open).
+		 * Use this instead of just dirtying the window because we need to load in
+		* the new difficulty settings */
 		if (_networking && !_network_server && FindWindowById(WC_GAME_OPTIONS, 0) != NULL)
 			ShowGameDifficulty();
 	}
diff --git a/order_cmd.c b/order_cmd.c
index 02e3f4267..fb4178492 100644
--- a/order_cmd.c
+++ b/order_cmd.c
@@ -570,27 +570,34 @@ int32 CmdModifyOrder(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	return 0;
 }
 
-/**
- *
- * Clone/share/copy an order-list of an other vehicle
- *
- * @param veh1_veh2 First 16 bits are of destination vehicle, last 16 of source vehicle
- * @param mode      Mode of cloning (CO_SHARE, CO_COPY, CO_UNSHARE)
- *
+/** Clone/share/copy an order-list of an other vehicle.
+ * @param p1 various bitstuffed elements
+ * - p1 = (bit  0-15) - destination vehicle to clone orders to (p1 & 0xFFFF)
+ * - p1 = (bit 16-31) - source vehicle to clone orders from, if any (none for CO_UNSHARE)
+ * @param p2 mode of cloning: CO_SHARE, CO_COPY, or CO_UNSHARE
  */
-int32 CmdCloneOrder(int x, int y, uint32 flags, uint32 veh1_veh2, uint32 mode)
+int32 CmdCloneOrder(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
-	Vehicle *dst = GetVehicle(veh1_veh2 & 0xFFFF);
+	Vehicle *dst;
+	VehicleID veh_src = (p1 >> 16) & 0xFFFF;
+	VehicleID veh_dst = p1 & 0xFFFF;
 
-	if (dst->type == 0 || dst->owner != _current_player)
-		return CMD_ERROR;
+	if (!IsVehicleIndex(veh_dst)) return CMD_ERROR;
 
-	switch(mode) {
+	dst = GetVehicle(veh_dst);
+
+	if (dst->type == 0 || !CheckOwnership(dst->owner)) return CMD_ERROR;
+
+	switch (p2) {
 		case CO_SHARE: {
-			Vehicle *src = GetVehicle(veh1_veh2 >> 16);
+			Vehicle *src;
+
+			if (!IsVehicleIndex(veh_src)) return CMD_ERROR;
+
+			src = GetVehicle(veh_src);
 
 			/* Sanity checks */
-			if (src->type == 0 || src->owner != _current_player || dst->type != src->type || dst == src)
+			if (src->type == 0 || !CheckOwnership(src->owner) || dst->type != src->type || dst == src)
 				return CMD_ERROR;
 
 			/* Trucks can't share orders with busses (and visa versa) */
@@ -631,11 +638,15 @@ int32 CmdCloneOrder(int x, int y, uint32 flags, uint32 veh1_veh2, uint32 mode)
 		} break;
 
 		case CO_COPY: {
-			Vehicle *src = GetVehicle(veh1_veh2 >> 16);
+			Vehicle *src;
 			int delta;
 
+			if (!IsVehicleIndex(veh_src)) return CMD_ERROR;
+
+			src = GetVehicle(veh_src);
+
 			/* Sanity checks */
-			if (src->type == 0 || src->owner != _current_player || dst->type != src->type || dst == src)
+			if (src->type == 0 || !CheckOwnership(src->owner) || dst->type != src->type || dst == src)
 				return CMD_ERROR;
 
 			/* Trucks can't copy all the orders from busses (and visa versa) */
@@ -685,8 +696,7 @@ int32 CmdCloneOrder(int x, int y, uint32 flags, uint32 veh1_veh2, uint32 mode)
 			}
 		} break;
 
-		case CO_UNSHARE:
-			return DecloneOrder(dst, flags);
+		case CO_UNSHARE: return DecloneOrder(dst, flags);
 	}
 
 	return 0;
diff --git a/player_gui.c b/player_gui.c
index e527af1ec..4bf57f553 100644
--- a/player_gui.c
+++ b/player_gui.c
@@ -510,13 +510,16 @@ static void PlayerCompanyWndProc(Window *w, WindowEvent *e)
 			if (!_networking) SETBIT(w->hidden_state, 11); // hide company-password widget
 		} else {
 			if (_patches.allow_shares) { /* shares are allowed */
+				/* If all shares are owned by someone (none by nobody), disable buy button */
 				if (GetAmountOwnedBy(p, OWNER_SPECTATOR) == 0) SETBIT(dis, 9);
 
-				/* We cannot buy out real players in a network game */
+				/* Only 25% left to buy. If the player is human, disable buying it up.. TODO issues! */
 				if (GetAmountOwnedBy(p, OWNER_SPECTATOR) == 1 && !p->is_ai) SETBIT(dis, 9);
 
+				/* If the player doesn't own any shares, disable sell button */
 				if (GetAmountOwnedBy(p, _local_player) == 0) SETBIT(dis, 10);
 
+				/* Spectators cannot do anything of course */
 				if (_local_player == OWNER_SPECTATOR) dis |= (1 << 9) | (1 << 10);
 			} else /* shares are not allowed, disable buy/sell buttons */
 				dis |= (1 << 9) | (1 << 10);
diff --git a/roadveh_cmd.c b/roadveh_cmd.c
index 1630d832d..60c59f152 100644
--- a/roadveh_cmd.c
+++ b/roadveh_cmd.c
@@ -105,13 +105,16 @@ void DrawRoadVehEngineInfo(int engine, int x, int y, int maxw)
 	DrawStringMultiCenter(x, y, STR_902A_COST_SPEED_RUNNING_COST, maxw);
 }
 
-int32 EstimateRoadVehCost(byte engine_type)
+int32 EstimateRoadVehCost(EngineID engine_type)
 {
 	return ((_price.roadveh_base >> 3) * RoadVehInfo(engine_type)->base_cost) >> 5;
 }
 
-// p1 = engine_type
-// p2 not used
+/** Build a road vehicle.
+ * @param x,y tile coordinates of depot where road vehicle is built
+ * @param p1 bus/truck type being built (engine)
+ * @param p2 unused
+ */
 int32 CmdBuildRoadVeh(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	int32 cost;
@@ -125,8 +128,7 @@ int32 CmdBuildRoadVeh(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES);
 
 	cost = EstimateRoadVehCost(p1);
-	if (flags & DC_QUERY_COST)
-		return cost;
+	if (flags & DC_QUERY_COST) return cost;
 
 	/* The ai_new queries the vehicle cost before building the route,
 	 * so we must check against cheaters no sooner than now. --pasky */
@@ -181,7 +183,7 @@ int32 CmdBuildRoadVeh(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 		v->max_speed = rvi->max_speed;
 		v->engine_type = (byte)p1;
 
-		e = &_engines[p1];
+		e = DEREF_ENGINE(p1);
 		v->reliability = e->reliability;
 		v->reliability_spd_dec = e->reliability_spd_dec;
 		v->max_age = e->lifelength * 366;
@@ -202,14 +204,17 @@ int32 CmdBuildRoadVeh(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 		InvalidateWindow(WC_VEHICLE_DEPOT, v->tile);
 		RebuildVehicleLists();
 		InvalidateWindow(WC_COMPANY, v->owner);
+		InvalidateWindow(WC_REPLACE_VEHICLE, VEH_Road); // updates the replace Road window
 	}
 
-	InvalidateWindow(WC_REPLACE_VEHICLE, VEH_Road); // updates the replace Road window
-
 	return cost;
 }
 
-// p1 = vehicle
+/** Start/Stop a road vehicle.
+ * @param x,y unused
+ * @param p1 road vehicle ID to start/stop
+ * @param p2 unused
+ */
 int32 CmdStartStopRoadVeh(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	Vehicle *v;
@@ -218,8 +223,7 @@ int32 CmdStartStopRoadVeh(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 
 	v = GetVehicle(p1);
 
-	if (v->type != VEH_Road || !CheckOwnership(v->owner))
-		return CMD_ERROR;
+	if (v->type != VEH_Road || !CheckOwnership(v->owner)) return CMD_ERROR;
 
 	if (flags & DC_EXEC) {
 		v->vehstatus ^= VS_STOPPED;
@@ -242,8 +246,11 @@ void ClearSlot(Vehicle *v, RoadStop *rs)
  }
 }
 
-//  p1 = vehicle index in GetVehicle()
-//  p2 not used
+/** Sell a road vehicle.
+ * @param x,y unused
+ * @param p1 vehicle ID to be sold
+ * @param p2 unused
+ */
 int32 CmdSellRoadVeh(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	Vehicle *v;
@@ -252,8 +259,7 @@ int32 CmdSellRoadVeh(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 
 	v = GetVehicle(p1);
 
-	if (v->type != VEH_Road || !CheckOwnership(v->owner))
-		return CMD_ERROR;
+	if (v->type != VEH_Road || !CheckOwnership(v->owner)) return CMD_ERROR;
 
 	SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES);
 
@@ -335,24 +341,29 @@ static Depot *FindClosestRoadDepot(Vehicle *v)
 	}
 }
 
+/** Send a road vehicle to the depot.
+ * @param x,y unused
+ * @param p1 vehicle ID to send to the depot
+ * @param p2 unused
+ */
 int32 CmdSendRoadVehToDepot(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	Vehicle *v;
-	Depot *depot;
+	const Depot *dep;
 
 	if (!IsVehicleIndex(p1)) return CMD_ERROR;
 
 	v = GetVehicle(p1);
 
-	if (v->type != VEH_Road || !CheckOwnership(v->owner))
-		return CMD_ERROR;
+	if (v->type != VEH_Road || !CheckOwnership(v->owner)) return CMD_ERROR;
 
-	if (v->vehstatus & VS_CRASHED)
-		return CMD_ERROR;
+	if (v->vehstatus & VS_CRASHED) return CMD_ERROR;
 
+	/* If the current orders are already goto-depot */
 	if (v->current_order.type == OT_GOTO_DEPOT) {
 		if (flags & DC_EXEC) {
-
+			/* If the orders to 'goto depot' are in the orders list (forced servicing),
+			 * then skip to the next order; effectively cancelling this forced service */
 			if (HASBIT(v->current_order.flags, OFB_PART_OF_ORDERS))
 				v->cur_order_index++;
 
@@ -363,21 +374,25 @@ int32 CmdSendRoadVehToDepot(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 		return 0;
 	}
 
-	depot = FindClosestRoadDepot(v);
-	if (depot == NULL)
-		return_cmd_error(STR_9019_UNABLE_TO_FIND_LOCAL_DEPOT);
+	dep = FindClosestRoadDepot(v);
+	if (dep == NULL) return_cmd_error(STR_9019_UNABLE_TO_FIND_LOCAL_DEPOT);
 
 	if (flags & DC_EXEC) {
 		v->current_order.type = OT_GOTO_DEPOT;
 		v->current_order.flags = OF_NON_STOP | OF_HALT_IN_DEPOT;
-		v->current_order.station = depot->index;
-		v->dest_tile = depot->xy;
+		v->current_order.station = dep->index;
+		v->dest_tile = dep->xy;
 		InvalidateWindowWidget(WC_VEHICLE_VIEW, v->index, STATUS_BAR);
 	}
 
 	return 0;
 }
 
+/** Turn a roadvehicle around.
+ * @param x,y unused
+ * @param p1 vehicle ID to turn
+ * @param p2 unused
+ */
 int32 CmdTurnRoadVeh(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	Vehicle *v;
@@ -386,15 +401,13 @@ int32 CmdTurnRoadVeh(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 
 	v = GetVehicle(p1);
 
-	if (v->type != VEH_Road || !CheckOwnership(v->owner))
-		return CMD_ERROR;
+	if (v->type != VEH_Road || !CheckOwnership(v->owner)) return CMD_ERROR;
 
 	if (v->vehstatus & (VS_HIDDEN|VS_STOPPED) ||
 			v->u.road.crashed_ctr != 0 ||
 			v->breakdown_ctr != 0 ||
 			v->u.road.overtaking != 0 ||
 			v->cur_speed < 5) {
-		_error_message = STR_EMPTY;
 		return CMD_ERROR;
 	}
 
diff --git a/ship_cmd.c b/ship_cmd.c
index 675b1ed05..24d21fc47 100644
--- a/ship_cmd.c
+++ b/ship_cmd.c
@@ -403,8 +403,10 @@ static bool ShipAccelerate(Vehicle *v)
 	return (t < v->progress);
 }
 
-
-int32 EstimateShipCost(uint16 engine_type);
+int32 EstimateShipCost(EngineID engine_type)
+{
+	return ShipVehInfo(engine_type)->base_cost * (_price.ship_base>>3)>>5;
+}
 
 static void ShipEnterDepot(Vehicle *v)
 {
@@ -851,12 +853,11 @@ void ShipsYearlyLoop(void)
 	}
 }
 
-int32 EstimateShipCost(uint16 engine_type)
-{
-	return ShipVehInfo(engine_type)->base_cost * (_price.ship_base>>3)>>5;
-}
-
-// p1 = type to build
+/** Build a ship.
+ * @param x,y tile coordinates of depot where ship is built
+ * @param p1 ship type being built (engine)
+ * @param p2 unused
+ */
 int32 CmdBuildShip(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	int32 value;
@@ -870,8 +871,7 @@ int32 CmdBuildShip(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES);
 
 	value = EstimateShipCost(p1);
-	if (flags & DC_QUERY_COST)
-		return value;
+	if (flags & DC_QUERY_COST) return value;
 
 	/* The ai_new queries the vehicle cost before building the route,
 	 * so we must check against cheaters no sooner than now. --pasky */
@@ -932,12 +932,17 @@ int32 CmdBuildShip(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 		InvalidateWindow(WC_VEHICLE_DEPOT, v->tile);
 		RebuildVehicleLists();
 		InvalidateWindow(WC_COMPANY, v->owner);
+		InvalidateWindow(WC_REPLACE_VEHICLE, VEH_Ship); // updates the replace Ship window
 	}
-	InvalidateWindow(WC_REPLACE_VEHICLE, VEH_Ship); // updates the replace Ship window
 
 	return value;
 }
 
+/** Sell a ship.
+ * @param x,y unused
+ * @param p1 vehicle ID to be sold
+ * @param p2 unused
+ */
 int32 CmdSellShip(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	Vehicle *v;
@@ -946,8 +951,7 @@ int32 CmdSellShip(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 
 	v = GetVehicle(p1);
 
-	if (v->type != VEH_Ship || !CheckOwnership(v->owner))
-		return CMD_ERROR;
+	if (v->type != VEH_Ship || !CheckOwnership(v->owner)) return CMD_ERROR;
 
 	SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES);
 
@@ -960,14 +964,17 @@ int32 CmdSellShip(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 		InvalidateWindow(WC_COMPANY, v->owner);
 		DeleteWindowById(WC_VEHICLE_VIEW, v->index);
 		DeleteVehicle(v);
+		InvalidateWindow(WC_REPLACE_VEHICLE, VEH_Ship); // updates the replace Ship window
 	}
 
-	InvalidateWindow(WC_REPLACE_VEHICLE, VEH_Ship); // updates the replace Ship window
-
 	return -(int32)v->value;
 }
 
-// p1 = vehicle
+/** Start/Stop a ship.
+ * @param x,y unused
+ * @param p1 ship ID to start/stop
+ * @param p2 unused
+ */
 int32 CmdStartStopShip(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	Vehicle *v;
@@ -976,38 +983,41 @@ int32 CmdStartStopShip(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 
 	v = GetVehicle(p1);
 
-	if (v->type != VEH_Ship || !CheckOwnership(v->owner))
-		return CMD_ERROR;
+	if (v->type != VEH_Ship || !CheckOwnership(v->owner)) return CMD_ERROR;
 
 	if (flags & DC_EXEC) {
 		v->vehstatus ^= VS_STOPPED;
 		InvalidateWindowWidget(WC_VEHICLE_VIEW, v->index, STATUS_BAR);
 		InvalidateWindow(WC_VEHICLE_DEPOT, v->tile);
+		InvalidateWindowClasses(WC_SHIPS_LIST);
 	}
 
-	InvalidateWindowClasses(WC_SHIPS_LIST);
-
 	return 0;
 }
 
+/** Send a ship to the depot.
+ * @param x,y unused
+ * @param p1 vehicle ID to send to the depot
+ * @param p2 unused
+ */
 int32 CmdSendShipToDepot(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	Vehicle *v;
-	Depot *depot;
+	const Depot *dep;
 
 	if (!IsVehicleIndex(p1)) return CMD_ERROR;
 
 	v = GetVehicle(p1);
 
-	if (v->type != VEH_Ship || !CheckOwnership(v->owner))
-		return CMD_ERROR;
+	if (v->type != VEH_Ship || !CheckOwnership(v->owner)) return CMD_ERROR;
 
-	if (v->vehstatus & VS_CRASHED)
-		return CMD_ERROR;
+	if (v->vehstatus & VS_CRASHED) return CMD_ERROR;
 
+	/* If the current orders are already goto-depot */
 	if (v->current_order.type == OT_GOTO_DEPOT) {
 		if (flags & DC_EXEC) {
-
+			/* If the orders to 'goto depot' are in the orders list (forced servicing),
+			 * then skip to the next order; effectively cancelling this forced service */
 			if (HASBIT(v->current_order.flags, OFB_PART_OF_ORDERS))
 				v->cur_order_index++;
 
@@ -1015,18 +1025,19 @@ int32 CmdSendShipToDepot(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 			v->current_order.flags = 0;
 			InvalidateWindowWidget(WC_VEHICLE_VIEW, v->index, STATUS_BAR);
 		}
-	} else {
-		depot = FindClosestShipDepot(v);
-		if (depot == NULL)
-			return_cmd_error(STR_981A_UNABLE_TO_FIND_LOCAL_DEPOT);
+		return 0;
+	}
 
-		if (flags & DC_EXEC) {
-			v->dest_tile = depot->xy;
-			v->current_order.type = OT_GOTO_DEPOT;
-			v->current_order.flags = OF_NON_STOP | OF_HALT_IN_DEPOT;
-			v->current_order.station = depot->index;
-			InvalidateWindowWidget(WC_VEHICLE_VIEW, v->index, STATUS_BAR);
-		}
+	dep = FindClosestShipDepot(v);
+	if (dep == NULL)
+		return_cmd_error(STR_981A_UNABLE_TO_FIND_LOCAL_DEPOT);
+
+	if (flags & DC_EXEC) {
+		v->dest_tile = dep->xy;
+		v->current_order.type = OT_GOTO_DEPOT;
+		v->current_order.flags = OF_NON_STOP | OF_HALT_IN_DEPOT;
+		v->current_order.station = dep->index;
+		InvalidateWindowWidget(WC_VEHICLE_VIEW, v->index, STATUS_BAR);
 	}
 
 	return 0;
diff --git a/signs.c b/signs.c
index 74856cc2b..fe7166098 100644
--- a/signs.c
+++ b/signs.c
@@ -95,12 +95,12 @@ static SignStruct *AllocateSign(void)
 	return NULL;
 }
 
-/**
- *
- * Place a sign at the given x/y
- *
- * @param p1 player number
- * @param p2 not used
+/** Place a sign at the given coordinates. Ownership of sign has
+ * no effect whatsoever except for the colour the sign gets for easy recognition,
+ * but everybody is able to rename/remove it.
+ * @param x,y coordinates to place sign at
+ * @param p1 unused
+ * @param p2 unused
  */
 int32 CmdPlaceSign(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
@@ -108,15 +108,14 @@ int32 CmdPlaceSign(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 
 	/* Try to locate a new sign */
 	ss = AllocateSign();
-	if (ss == NULL)
-		return_cmd_error(STR_2808_TOO_MANY_SIGNS);
+	if (ss == NULL) return_cmd_error(STR_2808_TOO_MANY_SIGNS);
 
 	/* When we execute, really make the sign */
 	if (flags & DC_EXEC) {
 		ss->str = STR_280A_SIGN;
 		ss->x = x;
 		ss->y = y;
-		ss->owner = p1;
+		ss->owner = _current_player; // owner of the sign; just eyecandy
 		ss->z = GetSlopeZ(x,y);
 		UpdateSignVirtCoords(ss);
 		MarkSignDirty(ss);
@@ -128,37 +127,35 @@ int32 CmdPlaceSign(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	return 0;
 }
 
-/**
- * Rename a sign
- *
- * @param sign_id Index of the sign
- * @param new owner, if OWNER_NONE, sign will be removed, except in scenario editor, where signs have no owner
- * and ownership has no influence of any kind
+/** Rename a sign. If the new name of the sign is empty, we assume
+ * the user wanted to delete it. So delete it. Ownership of signs
+ * has no meaning/effect whatsoever except for eyecandy
+ * @param x,y unused
+ * @param p1 index of the sign to be renamed/removed
+ * @param p2 unused
  */
-int32 CmdRenameSign(int x, int y, uint32 flags, uint32 sign_id, uint32 owner)
+int32 CmdRenameSign(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
-	StringID str;
 	SignStruct *ss;
 
-	/* If GetDParam(0) == nothing, we delete the sign */
-	if (GetDParam(0) != 0 && (_game_mode == GM_EDITOR || owner != OWNER_NONE)) {
+	/* If GetDParam(0) != 0 means the new text for the sign is non-empty.
+	 * So rename the sign. If it is empty, it has no name, so delete it */
+	if (GetDParam(0) != 0) {
 		/* Create the name */
-		str = AllocateName((const char*)_decode_parameters, 0);
-		if (str == 0)
-			return CMD_ERROR;
+		StringID str = AllocateName((const char*)_decode_parameters, 0);
+		if (str == 0) return CMD_ERROR;
 
 		if (flags & DC_EXEC) {
-			ss = GetSign(sign_id);
-
-			MarkSignDirty(ss);
+			ss = GetSign(p1);
 
 			/* Delete the old name */
 			DeleteName(ss->str);
 			/* Assign the new one */
 			ss->str = str;
-			ss->owner = owner;
+			ss->owner = _current_player;
 
-			/* Update */
+			/* Update; mark sign dirty twice, because it can either becom longer, or shorter */
+			MarkSignDirty(ss);
 			UpdateSignVirtCoords(ss);
 			MarkSignDirty(ss);
 			InvalidateWindow(WC_SIGN_LIST, 0);
@@ -167,10 +164,9 @@ int32 CmdRenameSign(int x, int y, uint32 flags, uint32 sign_id, uint32 owner)
 			/* Free the name, because we did not assign it yet */
 			DeleteName(str);
 		}
-	} else {
-		/* Delete sign */
+	} else { /* Delete sign */
 		if (flags & DC_EXEC) {
-			ss = GetSign(sign_id);
+			ss = GetSign(p1);
 
 			/* Delete the name */
 			DeleteName(ss->str);
diff --git a/town_cmd.c b/town_cmd.c
index 60d1b5962..10f797876 100644
--- a/town_cmd.c
+++ b/town_cmd.c
@@ -1014,13 +1014,23 @@ static Town *AllocateTown(void)
 	return NULL;
 }
 
+/** Create a new town.
+ * This obviously only works in the scenario editor. Function not removed
+ * as it might be possible in the future to fund your own town :)
+ * @param x,y coordinates where town is built
+ * @param p1 unused
+ * @param p2 unused
+ */
 int32 CmdBuildTown(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
-	uint tile = TILE_FROM_XY(x,y);
+	TileIndex tile = TILE_FROM_XY(x,y);
 	TileInfo ti;
 	Town *t;
 	uint32 townnameparts;
 
+	/* Only in the scenario editor */
+	if (_game_mode != GM_EDITOR) return CMD_ERROR;
+
 	SET_EXPENSES_TYPE(EXPENSES_OTHER);
 
 	// Check if too close to the edge of map
@@ -1042,8 +1052,7 @@ int32 CmdBuildTown(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 
 	// Allocate town struct
 	t = AllocateTown();
-	if (t == NULL)
-		return_cmd_error(STR_023A_TOO_MANY_TOWNS);
+	if (t == NULL) return_cmd_error(STR_023A_TOO_MANY_TOWNS);
 
 	// Create the town
 	if (flags & DC_EXEC) {
@@ -1428,19 +1437,26 @@ static void ClearTownHouse(Town *t, uint tile) {
 	if (eflags & 0x10) DoClearTownHouseHelper(tile + TILE_XY(1,1));
 }
 
+/** Rename a town (server-only).
+ * @param x,y unused
+ * @param p1 town ID to rename
+ * @param p2 unused
+ */
 int32 CmdRenameTown(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	StringID str;
-	Town *t = GetTown(p1);
+	Town *t;
+
+	if (!IsTownIndex(p1)) return CMD_ERROR;
+
+	t = GetTown(p1);
 
 	str = AllocateNameUnique((const char*)_decode_parameters, 4);
-	if (str == 0)
-		return CMD_ERROR;
+	if (str == 0) return CMD_ERROR;
 
 	if (flags & DC_EXEC) {
-		StringID old_str = t->townnametype;
+		DeleteName(t->townnametype);
 		t->townnametype = str;
-		DeleteName(old_str);
 
 		UpdateTownVirtCoord(t);
 		_town_sort_dirty = true;
@@ -1673,18 +1689,32 @@ static TownActionProc * const _town_action_proc[] = {
 	TownActionBribe
 };
 
-// p1 = town
-// p2 = action
+extern uint GetMaskOfTownActions(int *nump, PlayerID pid, const Town *t);
+
+/** Do a town action.
+ * This performs an action such as advertising, building a statue, funding buildings,
+ * but also bribing the town-council
+ * @param x,y unused
+ * @param p1 town to do the action at
+ * @param p2 action to perform, @see _town_action_proc for the list of available actions
+ */
 int32 CmdDoTownAction(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 {
 	int32 cost;
+	Town *t;
+
+	if (!IsTownIndex(p1) || p2 > lengthof(_town_action_proc)) return CMD_ERROR;
+
+	t = GetTown(p1);
+
+	if (!HASBIT(GetMaskOfTownActions(NULL, _current_player, t), p2)) return CMD_ERROR;
 
 	SET_EXPENSES_TYPE(EXPENSES_OTHER);
 
 	cost = (_price.build_industry >> 8) * _town_action_costs[p2];
 
 	if (flags & DC_EXEC) {
-		_town_action_proc[p2](GetTown(p1), p2);
+		_town_action_proc[p2](t, p2);
 		InvalidateWindow(WC_TOWN_AUTHORITY, p1);
 	}
 
diff --git a/town_gui.c b/town_gui.c
index f55c4d2ed..e367faf42 100644
--- a/town_gui.c
+++ b/town_gui.c
@@ -27,48 +27,50 @@ static const Widget _town_authority_widgets[] = {
 extern const byte _town_action_costs[8];
 extern void DrawPlayerIcon(int p, int x, int y);
 
-static uint GetMaskOfTownActions(int *nump, Town *t)
+/** Get a list of available actions to do at a town.
+ * @param *nump if not NULL add put the number of available actions in it
+ * @param pid the player that is querying the town
+ * @param *t the town that is queried
+ * @return bitmasked value of enabled actions
+ */
+uint GetMaskOfTownActions(int *nump, PlayerID pid, const Town *t)
 {
 	int32 avail, ref;
-	int i, num;
+	int num = 0;
 	uint avail_buttons = 0x7F; // by default all buttons except bribe are enabled.
-	uint buttons;
+	uint buttons = 0;
 
-	if (_local_player != OWNER_SPECTATOR) {
+	if (pid != OWNER_SPECTATOR) {
+		int i;
 		// bribe option enabled?
 		if (_patches.bribe) {
 			// if unwanted, disable everything.
-			if (t->unwanted[_local_player]) {
+			if (t->unwanted[pid]) {
 				avail_buttons = 0;
-			} else if (t->ratings[_local_player] < 600)
-				avail_buttons |= (1 << 7); // only bribe if less than excellent
+			} else if (t->ratings[pid] < 600)
+				SETBIT(avail_buttons, 7); // only bribe if less than excellent
 		}
 
 		// Things worth more than this are not shown
-		avail = DEREF_PLAYER(_local_player)->player_money + _price.station_value * 200;
+		avail = DEREF_PLAYER(pid)->player_money + _price.station_value * 200;
 		ref = _price.build_industry >> 8;
 
-		for(i=0,buttons=0,num=0; i != lengthof(_town_action_costs); i++,avail_buttons>>=1) {
-			if (avail_buttons&1 && avail >= _town_action_costs[i] * ref) {
+		for (i = 0; i != lengthof(_town_action_costs); i++, avail_buttons >>= 1) {
+			if (HASBIT(avail_buttons, 0) && avail >= _town_action_costs[i] * ref) {
 				SETBIT(buttons, i);
 				num++;
 			}
 		}
 
-		// Disable build statue if already built
-		if(HASBIT(t->statues, _local_player))
-		{
+		/* Disable build statue if already built */
+		if (HASBIT(t->statues, pid)) {
 			CLRBIT(buttons, 4);
 			num--;
 		}
 
-	} else {
-		// no actions available for spectator
-		buttons = 0;
-		num = 0;
 	}
 
-	if (nump) *nump = num;
+	if (nump != NULL) *nump = num;
 	return buttons;
 }
 
@@ -86,13 +88,12 @@ static int GetNthSetBit(uint32 bits, int n)
 
 static void TownAuthorityWndProc(Window *w, WindowEvent *e)
 {
-	uint buttons;
-	int numact;
-	Town *t = GetTown(w->window_number);
+	switch (e->event) {
+	case WE_PAINT: {
+		const Town *t = GetTown(w->window_number);
+		int numact;
+		uint buttons = GetMaskOfTownActions(&numact, _local_player, t);
 
-	switch(e->event) {
-	case WE_PAINT:
-		buttons = GetMaskOfTownActions(&numact, t);
 		SetVScrollCount(w, numact + 1);
 
 		if (WP(w,def_d).data_1 != -1 && !HASBIT(buttons, WP(w,def_d).data_1))
@@ -176,16 +177,17 @@ static void TownAuthorityWndProc(Window *w, WindowEvent *e)
 			}
 		}
 
-		break;
+	} break;
 
 	case WE_CLICK:
-		switch(e->click.widget) {
+		switch (e->click.widget) {
 		case 3: { /* listbox */
+			const Town *t = GetTown(w->window_number);
 			int y = (e->click.pt.y - 0x6B) / 10;
 			if (!IS_INT_INSIDE(y, 0, 5))
 				return;
 
-			y = GetNthSetBit(GetMaskOfTownActions(NULL, t), y + w->vscroll.pos - 1);
+			y = GetNthSetBit(GetMaskOfTownActions(NULL, _local_player, t), y + w->vscroll.pos - 1);
 			if (y >= 0) {
 				WP(w,def_d).data_1 = y;
 				SetWindowDirty(w);
@@ -194,7 +196,7 @@ static void TownAuthorityWndProc(Window *w, WindowEvent *e)
 		}
 
 		case 6: { /* carry out the action */
-			DoCommandP(t->xy, w->window_number, WP(w,def_d).data_1, NULL, CMD_DO_TOWN_ACTION | CMD_MSG(STR_2054_CAN_T_DO_THIS));
+			DoCommandP(GetTown(w->window_number)->xy, w->window_number, WP(w,def_d).data_1, NULL, CMD_DO_TOWN_ACTION | CMD_MSG(STR_2054_CAN_T_DO_THIS));
 			break;
 		}
 		}
diff --git a/vehicle.c b/vehicle.c
index 91d76ce44..1d670174e 100644
--- a/vehicle.c
+++ b/vehicle.c
@@ -1299,9 +1299,9 @@ void AgeVehicle(Vehicle *v)
 }
 
 extern int32 EstimateTrainCost(const RailVehicleInfo *rvi);
-extern int32 EstimateRoadVehCost(byte engine_type);
-extern int32 EstimateShipCost(uint16 engine_type);
-extern int32 EstimateAircraftCost(uint16 engine_type);
+extern int32 EstimateRoadVehCost(EngineID engine_type);
+extern int32 EstimateShipCost(EngineID engine_type);
+extern int32 EstimateAircraftCost(EngineID engine_type);
 extern int32 CmdRefitRailVehicle(int x, int y, uint32 flags, uint32 p1, uint32 p2);
 extern int32 CmdRefitShip(int x, int y, uint32 flags, uint32 p1, uint32 p2);
 extern int32 CmdRefitAircraft(int x, int y, uint32 flags, uint32 p1, uint32 p2);