(svn r3829) Reduce the use of _error_message by directly returning error codes instead of using this global variable

aircraft_cmd.c

@@ -315,12 +315,7 @@ bool IsAircraftHangarTile(TileIndex tile)
 
 bool CheckStoppedInHangar(const Vehicle* v)
 {
-	if (!(v->vehstatus & VS_STOPPED) || !IsAircraftHangarTile(v->tile)) {
+	return v->vehstatus & VS_STOPPED && IsAircraftHangarTile(v->tile);
-		_error_message = STR_A01B_AIRCRAFT_MUST_BE_STOPPED;
-		return false;
-	}
-
-	return true;
 }
 
 
@@ -346,8 +341,8 @@ int32 CmdSellAircraft(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 
 	v = GetVehicle(p1);
 
-	if (v->type != VEH_Aircraft || !CheckOwnership(v->owner) || !CheckStoppedInHangar(v))
+	if (v->type != VEH_Aircraft || !CheckOwnership(v->owner)) return CMD_ERROR;
-		return CMD_ERROR;
+	if (!CheckStoppedInHangar(v)) return_cmd_error(STR_A01B_AIRCRAFT_MUST_BE_STOPPED);
 
 	SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES);
 

station_cmd.c

@@ -764,8 +764,7 @@ int32 CheckFlatLandBelow(TileIndex tile, uint w, uint h, uint flags, uint invali
 		*/
 		if (IsSteepTileh(tileh) ||
 				((_is_old_ai_player || !_patches.build_on_slopes) && tileh != 0)) {
-			_error_message = STR_0007_FLAT_LAND_REQUIRED;
+			return_cmd_error(STR_0007_FLAT_LAND_REQUIRED);
-			return CMD_ERROR;
 		}
 
 		flat_z = z;
@@ -775,8 +774,7 @@ int32 CheckFlatLandBelow(TileIndex tile, uint w, uint h, uint flags, uint invali
 					(invalid_dirs&2 && !(tileh & 6) &&	h_cur == 1) ||
 					(invalid_dirs&4 && !(tileh & 3) && w_cur == 1) ||
 					(invalid_dirs&8 && !(tileh & 9) && (uint)h_cur == h)) {
-				_error_message = STR_0007_FLAT_LAND_REQUIRED;
+				return_cmd_error(STR_0007_FLAT_LAND_REQUIRED);
-				return CMD_ERROR;
 			}
 			cost += _price.terraform;
 			flat_z += 8;
@@ -787,8 +785,7 @@ int32 CheckFlatLandBelow(TileIndex tile, uint w, uint h, uint flags, uint invali
 			// first tile
 			allowed_z = flat_z;
 		} else if (allowed_z != flat_z) {
-			_error_message = STR_0007_FLAT_LAND_REQUIRED;
+			return_cmd_error(STR_0007_FLAT_LAND_REQUIRED);
-			return CMD_ERROR;
 		}
 
 		// if station is set, then we have special handling to allow building on top of already existing stations.
@@ -796,20 +793,18 @@ int32 CheckFlatLandBelow(TileIndex tile, uint w, uint h, uint flags, uint invali
 		// on exactly that station.
 		if (station != NULL && IsTileType(tile_cur, MP_STATION)) {
 			if (_m[tile_cur].m5 >= 8) {
-				_error_message = ClearTile_Station(tile_cur, DC_AUTO); // get error message
+				return ClearTile_Station(tile_cur, DC_AUTO); // get error message
-				return CMD_ERROR;
 			} else {
 				StationID st = _m[tile_cur].m2;
 				if (*station == INVALID_STATION) {
 					*station = st;
 				} else if (*station != st) {
-					_error_message = STR_3006_ADJOINS_MORE_THAN_ONE_EXISTING;
+					return_cmd_error(STR_3006_ADJOINS_MORE_THAN_ONE_EXISTING);
-					return CMD_ERROR;
 				}
 			}
 		} else {
 			ret = DoCommandByTile(tile_cur, 0, 0, flags, CMD_LANDSCAPE_CLEAR);
-			if (CmdFailed(ret)) return CMD_ERROR;
+			if (CmdFailed(ret)) return ret;
 			cost += ret;
 		}
 	END_TILE_LOOP(tile_cur, w, h, tile)
@@ -969,7 +964,8 @@ int32 CmdBuildRailroadStation(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	est = INVALID_STATION;
 	// If DC_EXEC is in flag, do not want to pass it to CheckFlatLandBelow, because of a nice bug
 	//  for detail info, see: https://sourceforge.net/tracker/index.php?func=detail&aid=1029064&group_id=103924&atid=636365
-	if (CmdFailed(ret = CheckFlatLandBelow(tile_org, w_org, h_org, flags&~DC_EXEC, 5 << axis, _patches.nonuniform_stations ? &est : NULL))) return CMD_ERROR;
+	ret = CheckFlatLandBelow(tile_org, w_org, h_org, flags & ~DC_EXEC, 5 << axis, _patches.nonuniform_stations ? &est : NULL);
+	if (CmdFailed(ret)) return ret;
 	cost = ret + (numtracks * _price.train_station_track + _price.train_station_length) * plat_len;
 
 	// Make sure there are no similar stations around us.
@@ -1021,7 +1017,8 @@ int32 CmdBuildRailroadStation(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 		// Now really clear the land below the station
 		// It should never return CMD_ERROR.. but you never know ;)
 		//  (a bit strange function name for it, but it really does clear the land, when DC_EXEC is in flags)
-		if (CmdFailed(CheckFlatLandBelow(tile_org, w_org, h_org, flags, 5 << axis, _patches.nonuniform_stations ? &est : NULL))) return CMD_ERROR;
+		ret = CheckFlatLandBelow(tile_org, w_org, h_org, flags, 5 << axis, _patches.nonuniform_stations ? &est : NULL);
+		if (CmdFailed(ret)) return ret;
 
 		st->train_tile = finalvalues[0];
 		if (!st->facilities) st->xy = finalvalues[0];
@@ -1305,6 +1302,7 @@ int32 CmdBuildRoadStop(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	RoadStop *prev = NULL;
 	TileIndex tile;
 	int32 cost;
+	int32 ret;
 	bool type = !!p2;
 
 	/* Saveguard the parameters */
@@ -1317,8 +1315,9 @@ int32 CmdBuildRoadStop(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	if (!(flags & DC_NO_TOWN_RATING) && !CheckIfAuthorityAllows(tile))
 		return CMD_ERROR;
 
-	cost = CheckFlatLandBelow(tile, 1, 1, flags, 1 << p1, NULL);
+	ret = CheckFlatLandBelow(tile, 1, 1, flags, 1 << p1, NULL);
-	if (CmdFailed(cost)) return CMD_ERROR;
+	if (CmdFailed(ret)) return ret;
+	cost = ret;
 
 	st = GetStationAround(tile, 1, 1, -1);
 	if (st == CHECK_STATIONS_ERR) return CMD_ERROR;
@@ -1522,6 +1521,7 @@ int32 CmdBuildAirport(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	Town *t;
 	Station *st;
 	int32 cost;
+	int32 ret;
 	int w, h;
 	bool airport_upgrade = true;
 
@@ -1553,8 +1553,9 @@ int32 CmdBuildAirport(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	w = _airport_size_x[p1];
 	h = _airport_size_y[p1];
 
-	cost = CheckFlatLandBelow(tile, w, h, flags, 0, NULL);
+	ret = CheckFlatLandBelow(tile, w, h, flags, 0, NULL);
-	if (CmdFailed(cost)) return CMD_ERROR;
+	if (CmdFailed(ret)) return ret;
+	cost = ret;
 
 	st = GetStationAround(tile, w, h, -1);
 	if (st == CHECK_STATIONS_ERR) return CMD_ERROR;

train_cmd.c

@@ -776,18 +776,14 @@ int32 CmdBuildRailVehicle(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 
 
 /* Check if all the wagons of the given train are in a depot, returns the
- * number of cars (including loco) then. If not, sets the error message to
+ * number of cars (including loco) then. If not it returns -1 */
- * STR_881A_TRAINS_CAN_ONLY_BE_ALTERED and returns -1 */
 int CheckTrainStoppedInDepot(const Vehicle *v)
 {
 	int count;
 	TileIndex tile = v->tile;
 
 	/* check if stopped in a depot */
-	if (!IsTileDepotType(tile, TRANSPORT_RAIL) || v->cur_speed != 0) {
+	if (!IsTileDepotType(tile, TRANSPORT_RAIL) || v->cur_speed != 0) return -1;
-		_error_message = STR_881A_TRAINS_CAN_ONLY_BE_ALTERED;
-		return -1;
-	}
 
 	count = 0;
 	for (; v != NULL; v = v->next) {
@@ -797,7 +793,6 @@ int CheckTrainStoppedInDepot(const Vehicle *v)
 		if (!IsArticulatedPart(v)) count++;
 		if (v->u.rail.track != 0x80 || v->tile != tile ||
 				(IsFrontEngine(v) && !(v->vehstatus & VS_STOPPED))) {
-			_error_message = STR_881A_TRAINS_CAN_ONLY_BE_ALTERED;
 			return -1;
 		}
 	}
@@ -983,7 +978,7 @@ int32 CmdMoveRailVehicle(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 
 		// check if all vehicles in the source train are stopped inside a depot.
 		src_len = CheckTrainStoppedInDepot(src_head);
-		if (src_len < 0) return CMD_ERROR;
+		if (src_len < 0) return_cmd_error(STR_881A_TRAINS_CAN_ONLY_BE_ALTERED);
 
 		// check the destination row if the source and destination aren't the same.
 		if (src_head != dst_head) {
@@ -992,7 +987,7 @@ int32 CmdMoveRailVehicle(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 			if (dst_head != NULL) {
 				// check if all vehicles in the dest train are stopped.
 				dst_len = CheckTrainStoppedInDepot(dst_head);
-				if (dst_len < 0) return CMD_ERROR;
+				if (dst_len < 0) return_cmd_error(STR_881A_TRAINS_CAN_ONLY_BE_ALTERED);
 
 				assert(dst_head->tile == src_head->tile);
 			}
@@ -1217,7 +1212,9 @@ int32 CmdSellRailWagon(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	first = GetFirstVehicleInChain(v);
 
 	// make sure the vehicle is stopped in the depot
-	if (CheckTrainStoppedInDepot(first) < 0) return CMD_ERROR;
+	if (CheckTrainStoppedInDepot(first) < 0) {
+		return_cmd_error(STR_881A_TRAINS_CAN_ONLY_BE_ALTERED);
+	}
 
 	if (IsMultiheaded(v) && !IsTrainEngine(v)) return_cmd_error(STR_REAR_ENGINE_FOLLOW_FRONT_ERROR);
 
@@ -1593,8 +1590,6 @@ static void ReverseTrainDirection(Vehicle *v)
 
 	if (v->type != VEH_Train || !CheckOwnership(v->owner)) return CMD_ERROR;
 
-	_error_message = STR_EMPTY;
-
 //	if (v->u.rail.track & 0x80 || IsTileDepotType(v->tile, TRANSPORT_RAIL))
 //		return CMD_ERROR;
 

tree_cmd.c

@@ -130,6 +130,7 @@ void GenerateTrees(void)
  */
 int32 CmdPlantTree(int ex, int ey, uint32 flags, uint32 p1, uint32 p2)
 {
+	StringID msg = INVALID_STRING_ID;
 	int32 cost;
 	int sx, sy, x, y;
 
@@ -158,7 +159,7 @@ int32 CmdPlantTree(int ex, int ey, uint32 flags, uint32 p1, uint32 p2)
 				case MP_TREES:
 					// no more space for trees?
 					if (_game_mode != GM_EDITOR && GetTreeCount(tile) == 3) {
-						_error_message = STR_2803_TREE_ALREADY_HERE;
+						msg = STR_2803_TREE_ALREADY_HERE;
 						continue;
 					}
 
@@ -172,7 +173,7 @@ int32 CmdPlantTree(int ex, int ey, uint32 flags, uint32 p1, uint32 p2)
 
 				case MP_CLEAR:
 					if (!IsTileOwner(tile, OWNER_NONE)) {
-						_error_message = STR_2804_SITE_UNSUITABLE;
+						msg = STR_2804_SITE_UNSUITABLE;
 						continue;
 					}
 
@@ -213,13 +214,17 @@ int32 CmdPlantTree(int ex, int ey, uint32 flags, uint32 p1, uint32 p2)
 					break;
 
 				default:
-					_error_message = STR_2804_SITE_UNSUITABLE;
+					msg = STR_2804_SITE_UNSUITABLE;
 					break;
 			}
 		}
 	}
 
-	return (cost == 0) ? CMD_ERROR : cost;
+	if (cost == 0) {
+		return_cmd_error(msg);
+	} else {
+		return cost;
+	}
 }
 
 typedef struct TreeListEnt {

tunnelbridge_cmd.c

@@ -282,8 +282,8 @@ int32 CmdBuildBridge(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 
 	/* Try and clear the start landscape */
 
-	if (CmdFailed(ret = DoCommandByTile(ti_start.tile, 0, 0, flags, CMD_LANDSCAPE_CLEAR)))
+	ret = DoCommandByTile(ti_start.tile, 0, 0, flags, CMD_LANDSCAPE_CLEAR);
-		return CMD_ERROR;
+	if (CmdFailed(ret)) return ret;
 	cost = ret;
 
 	// true - bridge-start-tile, false - bridge-end-tile
@@ -295,7 +295,7 @@ int32 CmdBuildBridge(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	/* Try and clear the end landscape */
 
 	ret = DoCommandByTile(ti_end.tile, 0, 0, flags, CMD_LANDSCAPE_CLEAR);
-	if (CmdFailed(ret)) return CMD_ERROR;
+	if (CmdFailed(ret)) return ret;
 	cost += ret;
 
 	// false - end tile slope check
@@ -336,15 +336,13 @@ int32 CmdBuildBridge(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 
 		tile += delta;
 
-		_error_message = STR_5009_LEVEL_LAND_OR_WATER_REQUIRED;
+		if (GetTileSlope(tile, &z) != 0 && z >= ti_start.z) {
-		if (GetTileSlope(tile, &z) != 0 && z >= ti_start.z) return CMD_ERROR;
+			return_cmd_error(STR_5009_LEVEL_LAND_OR_WATER_REQUIRED);
+		}
 
 		switch (GetTileType(tile)) {
 			case MP_WATER:
-				if (!EnsureNoVehicle(tile)) {
+				if (!EnsureNoVehicle(tile)) return_cmd_error(STR_980E_SHIP_IN_THE_WAY);
-					_error_message = STR_980E_SHIP_IN_THE_WAY;
-					return CMD_ERROR;
-				}
 				if (_m[tile].m5 > 1) goto not_valid_below;
 				m5 = 0xC8;
 				break;
@@ -368,7 +366,7 @@ int32 CmdBuildBridge(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 not_valid_below:;
 				/* try and clear the middle landscape */
 				ret = DoCommandByTile(tile, 0, 0, flags, CMD_LANDSCAPE_CLEAR);
-				if (CmdFailed(ret)) return CMD_ERROR;
+				if (CmdFailed(ret)) return ret;
 				cost += ret;
 				m5 = 0xC0;
 				break;

unmovable_cmd.c

@@ -69,14 +69,16 @@ int32 CmdBuildCompanyHQ(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	TileIndex tile = TileVirtXY(x, y);
 	Player *p = GetPlayer(_current_player);
 	int cost;
+	int32 ret;
 
 	SET_EXPENSES_TYPE(EXPENSES_PROPERTY);
 
-	cost = CheckFlatLandBelow(tile, 2, 2, flags, 0, NULL);
+	ret = CheckFlatLandBelow(tile, 2, 2, flags, 0, NULL);
-	if (CmdFailed(cost)) return CMD_ERROR;
+	if (CmdFailed(ret)) return ret;
+	cost = ret;
 
 	if (p->location_of_house != 0) { /* Moving HQ */
-		int32 ret = DestroyCompanyHQ(p->location_of_house, flags);
+		ret = DestroyCompanyHQ(p->location_of_house, flags);
 		if (CmdFailed(ret)) return CMD_ERROR;
 		cost += ret;
 	}

water_cmd.c

@@ -236,43 +236,45 @@ int32 CmdBuildCanal(int x, int y, uint32 flags, uint32 p1, uint32 p2)
 	BEGIN_TILE_LOOP(tile, size_x, size_y, TileXY(sx, sy)) {
 		if (GetTileSlope(tile, NULL) != 0) return_cmd_error(STR_0007_FLAT_LAND_REQUIRED);
 
-			// can't make water of water!
+		// can't make water of water!
-			if (IsTileType(tile, MP_WATER)) {
+		if (IsTileType(tile, MP_WATER)) continue;
-				_error_message = STR_1007_ALREADY_BUILT;
-			} else {
-				/* is middle piece of a bridge? */
-				if (IsTileType(tile, MP_TUNNELBRIDGE) && _m[tile].m5 & 0x40) { /* build under bridge */
-					if (_m[tile].m5 & 0x20) // transport route under bridge
-						return_cmd_error(STR_5800_OBJECT_IN_THE_WAY);
 
-					if (_m[tile].m5 & 0x18) { // already water under bridge
+		/* is middle piece of a bridge? */
-						return_cmd_error(STR_1007_ALREADY_BUILT);
+		if (IsTileType(tile, MP_TUNNELBRIDGE) && _m[tile].m5 & 0x40) { /* build under bridge */
-					}
+			if (_m[tile].m5 & 0x20) // transport route under bridge
+				return_cmd_error(STR_5800_OBJECT_IN_THE_WAY);
 
-					if (flags & DC_EXEC) {
+			if (_m[tile].m5 & 0x18) { // already water under bridge
-						// change owner to OWNER_WATER and set land under bridge bit to water
+				return_cmd_error(STR_1007_ALREADY_BUILT);
-						ModifyTile(tile, MP_MAP5 | MP_MAPOWNER, OWNER_WATER, _m[tile].m5 | 0x08);
-					}
-				} else {
-					/* no bridge, try to clear it. */
-					int32 ret = DoCommandByTile(tile, 0, 0, flags, CMD_LANDSCAPE_CLEAR);
-
-					if (CmdFailed(ret)) return CMD_ERROR;
-					cost += ret;
-
-					if (flags & DC_EXEC) {
-						MakeWater(tile);
-						MarkTileDirtyByTile(tile);
-					}
-				}
-
-				if (flags & DC_EXEC) MarkTilesAroundDirty(tile);
-
-				cost += _price.clear_water;
 			}
+
+			if (flags & DC_EXEC) {
+				// change owner to OWNER_WATER and set land under bridge bit to water
+				ModifyTile(tile, MP_MAP5 | MP_MAPOWNER, OWNER_WATER, _m[tile].m5 | 0x08);
+			}
+		} else {
+			/* no bridge, try to clear it. */
+			int32 ret = DoCommandByTile(tile, 0, 0, flags, CMD_LANDSCAPE_CLEAR);
+
+			if (CmdFailed(ret)) return ret;
+			cost += ret;
+
+			if (flags & DC_EXEC) {
+				MakeWater(tile);
+				MarkTileDirtyByTile(tile);
+			}
+		}
+
+		if (flags & DC_EXEC) MarkTilesAroundDirty(tile);
+
+		cost += _price.clear_water;
 	} END_TILE_LOOP(tile, size_x, size_y, 0);
 
-	return (cost == 0) ? CMD_ERROR : cost;
+	if (cost == 0) {
+		return_cmd_error(STR_1007_ALREADY_BUILT);
+	} else {
+		return cost;
+	}
 }
 
 static int32 ClearTile_Water(TileIndex tile, byte flags)