Index: src/aircraft_cmd.cpp =================================================================== --- src/aircraft_cmd.cpp (revision 11784) +++ src/aircraft_cmd.cpp (working copy) @@ -281,7 +281,7 @@ if (!IsHangarTile(tile) || !IsTileOwner(tile, _current_player)) return CMD_ERROR; - SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES); + >>> Do not three white lines isn't good. Especially when they have trailing white space /* Prevent building aircraft types at places which can't handle them */ if (!CanAircraftUseStation(p1, tile)) return CMD_ERROR; Index: src/autoreplace_cmd.cpp =================================================================== --- src/autoreplace_cmd.cpp (revision 11784) +++ src/autoreplace_cmd.cpp (working copy) @@ -266,7 +264,7 @@ /* Ensure that the player will not end up having negative money while autoreplacing * This is needed because the only other check is done after the income from selling the old vehicle is substracted from the cost */ if (CmdFailed(tmp_move) || p->player_money < (cost.GetCost() + total_cost)) { - SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES); + >>> Pointless white line, again with trailing white space /* Pay back the loan */ sell_value.MultiplyCost(-1); SubtractMoneyFromPlayer(sell_value); Index: src/command.cpp =================================================================== --- src/command.cpp (revision 11784) +++ src/command.cpp (working copy) @@ -454,6 +454,7 @@ /* if toplevel, subtract the money. */ if (--_docommand_recursive == 0) { SubtractMoneyFromPlayer(res); + >>> Why? /* XXX - Old AI hack which doesn't use DoCommandDP; update last build coord of player */ if (tile != 0 && IsValidPlayer(_current_player)) { GetPlayer(_current_player)->last_build_coordinate = tile; @@ -635,9 +635,8 @@ goto show_error; } } - >>> Why? SubtractMoneyFromPlayer(res2); - + >>> Why? if (IsLocalPlayer() && _game_mode != GM_EDITOR) { if (res2.GetCost() != 0 && tile != 0) ShowCostOrIncomeAnimation(x, y, GetSlopeZ(x, y), res2.GetCost()); if (_additional_cash_required != 0) { @@ -686,7 +685,7 @@ return *this; } -CommandCost CommandCost::MultiplyCost(int factor) +CommandCost CommandCost::MultiplyCost(int64 factor) >>> Why int64? With 'plain' int is works okay too { this->cost *= factor; return *this; Index: src/command_type.h =================================================================== --- src/command_type.h (revision 11784) +++ src/command_type.h (working copy) @@ -14,28 +14,43 @@ * a possible error message/state together. */ class CommandCost { + ExpensesType expense_type; ///< the type of expence as shown on the finances view Money cost; ///< The cost of this action StringID message; ///< Warning message for when success is unset bool success; ///< Whether the comment went fine up to this moment + >>> Pointless extra white line + trailing space public: /** * Creates a command cost return with no cost and no error */ - CommandCost() : cost(0), message(INVALID_STRING_ID), success(true) {} + CommandCost() : expense_type(INVALID_EXPENSES), cost(0), message(INVALID_STRING_ID), success(true) {} /** * Creates a command return value the is failed with the given message */ - CommandCost(StringID msg) : cost(0), message(msg), success(false) {} + CommandCost(StringID msg) : expense_type(INVALID_EXPENSES), cost(0), message(msg), success(false) {} /** + * Creates a command cost with given expense type and start cost of 0 + * @param ex_t the expense type + */ + CommandCost(ExpensesType ex_t) : expense_type(ex_t), cost(0), message(INVALID_STRING_ID), success(true) {} + + /** * Creates a command return value with the given start cost * @param cst the initial cost of this command */ - CommandCost(Money cst) : cost(cst), message(INVALID_STRING_ID), success(true) {} + //CommandCost(Money cst) : cost(cst), message(INVALID_STRING_ID), success(true) {} >>> Just remove the whole function definition. /** + * Creates a command return value with the given start cost and expense type + * @param ex_t the expense type + * @param cst the initial cost of this command + */ + CommandCost(ExpensesType ex_t, Money cst) : expense_type(ex_t), cost(cst), message(INVALID_STRING_ID), success(true) {} + + /** * Adds the cost of the given command return value to this cost. * Also takes a possible error message when it is set. * @param ret the command to add the cost of. @@ -55,7 +70,7 @@ * @param cost factor to multiply the costs with * @return this class */ - CommandCost MultiplyCost(int factor); + CommandCost MultiplyCost(int64 factor); >>> See above /** * The costs as made up to this moment Index: src/economy.cpp =================================================================== --- src/economy.cpp (revision 11784) +++ src/economy.cpp (working copy) @@ -762,12 +761,11 @@ if (!p->is_active) continue; _current_player = p->index; - SET_EXPENSES_TYPE(EXPENSES_LOAN_INT); + >>> More pointless white lines + trailing spaces. - SubtractMoneyFromPlayer(CommandCost((Money)BigMulSU(p->current_loan, interest, 16))); + SubtractMoneyFromPlayer(CommandCost(EXPENSES_LOAN_INT, (Money)BigMulSU(p->current_loan, interest, 16))); - SET_EXPENSES_TYPE(EXPENSES_OTHER); - SubtractMoneyFromPlayer(_price.station_value >> 2); + SubtractMoneyFromPlayer(CommandCost(EXPENSES_OTHER, _price.station_value >> 2)); } } @@ -1515,7 +1513,7 @@ if (route_profit != 0) { front_v->profit_this_year += vehicle_profit; - SubtractMoneyFromPlayer(-route_profit); + SubtractMoneyFromPlayer( CommandCost(front_v->GetExpenseType(true), -route_profit ) ); >>> No spaces after '(' or before ')' if (IsLocalPlayer() && !PlayVehicleSound(front_v, VSE_LOAD_UNLOAD)) { SndPlayVehicleFx(SND_14_CASHTILL, front_v); @@ -1824,9 +1822,9 @@ PlayerID old_player = _current_player; for (i = 0; i != 4; i++) { if (p->share_owners[i] != PLAYER_SPECTATOR) { - SET_EXPENSES_TYPE(EXPENSES_OTHER); + >>> Useless line + trailing whitespace _current_player = p->share_owners[i]; - SubtractMoneyFromPlayer(CommandCost(-value)); + SubtractMoneyFromPlayer(CommandCost( EXPENSES_OTHER, -value) ); >>> Whitespace after/before ( and ) } } _current_player = old_player; @@ -1910,7 +1906,6 @@ /* Cannot sell shares of non-existent nor bankrupted company */ if (!p->is_active) return CMD_ERROR; - SET_EXPENSES_TYPE(EXPENSES_OTHER); >>> Two white lines? /* Those lines are here for network-protection (clients can be slow) */ if (GetAmountOwnedBy(p, _current_player) == 0) return CommandCost(); Index: src/economy_type.h =================================================================== --- src/economy_type.h (revision 11784) +++ src/economy_type.h (working copy) @@ -124,6 +124,7 @@ EXPENSES_SHIP_INC = 10, EXPENSES_LOAN_INT = 11, EXPENSES_OTHER = 12, + INVALID_EXPENSES = 0xFF, }; #endif /* ECONOMY_TYPE_H */ Index: src/landscape.cpp =================================================================== --- src/landscape.cpp (revision 11784) +++ src/landscape.cpp (working copy) @@ -523,9 +523,7 @@ * @param p2 unused */ CommandCost CmdLandscapeClear(TileIndex tile, uint32 flags, uint32 p1, uint32 p2) -{ - SET_EXPENSES_TYPE(EXPENSES_CONSTRUCTION); - +{ >>> Trailing whitespace! return _tile_type_procs[GetTileType(tile)]->clear_tile_proc(tile, flags); } Index: src/misc_cmd.cpp =================================================================== --- src/misc_cmd.cpp (revision 11784) +++ src/misc_cmd.cpp (working copy) @@ -387,7 +384,7 @@ /* Add money to player */ PlayerID old_cp = _current_player; _current_player = (PlayerID)p2; - SubtractMoneyFromPlayer(CommandCost(-amount.GetCost())); + SubtractMoneyFromPlayer( CommandCost(EXPENSES_OTHER, -amount.GetCost() ) ); >>> Whitespace w.r.t. ( and ) _current_player = old_cp; } Index: src/openttd.h =================================================================== --- src/openttd.h (revision 11784) +++ src/openttd.h (working copy) @@ -179,7 +179,6 @@ int32 top; byte width_1, width_2; }; - >>> Totally unneeded I guess enum { SORT_ASCENDING = 0, SORT_DESCENDING = 1, Index: src/players.cpp =================================================================== --- src/players.cpp (revision 11784) +++ src/players.cpp (working copy) @@ -190,30 +190,32 @@ static void SubtractMoneyFromAnyPlayer(Player *p, CommandCost cost) { - CommandCost tmp(p->player_money); - tmp.AddCost(-cost.GetCost()); - p->player_money = tmp.GetCost(); + if(cost.GetCost() == 0) return; >>> Space after the if + assert(cost.GetExpensesType() != INVALID_EXPENSES); + + if(p->player_money > cost.GetCost()) + { + p->player_money -= cost.GetCost(); + } + else + { + p->player_money = 0; + } >>> This whole if-else statement is wrong. Money must just be substracted from the account; the account must not be set to 0 when the amount of money would go negative -> whole if to p->player_money -= cost.GetCost() + p->yearly_expenses[0][cost.GetExpensesType()] += cost.GetCost(); - tmp = CommandCost(p->yearly_expenses[0][_yearly_expenses_type]); - tmp.AddCost(cost); - p->yearly_expenses[0][_yearly_expenses_type] = tmp.GetCost(); - if (HasBit(1 << EXPENSES_TRAIN_INC | 1 << EXPENSES_ROADVEH_INC | 1 << EXPENSES_AIRCRAFT_INC | - 1 << EXPENSES_SHIP_INC, _yearly_expenses_type)) { - tmp = CommandCost(p->cur_economy.income); - tmp.AddCost(-cost.GetCost()); - p->cur_economy.income = tmp.GetCost(); + 1 << EXPENSES_SHIP_INC, cost.GetExpensesType())) { + >>> Pointless white line + p->cur_economy.income += cost.GetCost(); } else if (HasBit(1 << EXPENSES_TRAIN_RUN | 1 << EXPENSES_ROADVEH_RUN | 1 << EXPENSES_AIRCRAFT_RUN | 1 << EXPENSES_SHIP_RUN | 1 << EXPENSES_PROPERTY | - 1 << EXPENSES_LOAN_INT, _yearly_expenses_type)) { - tmp = CommandCost(p->cur_economy.expenses); - tmp.AddCost(-cost.GetCost()); - p->cur_economy.expenses = tmp.GetCost(); + 1 << EXPENSES_LOAN_INT, cost.GetExpensesType())) { + p->cur_economy.expenses += cost.GetCost(); } InvalidatePlayerWindows(p); Index: src/rail_cmd.cpp =================================================================== --- src/rail_cmd.cpp (revision 11784) +++ src/rail_cmd.cpp (working copy) @@ -2475,7 +2458,7 @@ Foundation f_old = GetRailFoundation(tileh_old, rail_bits); - /* Do not allow terraforming if allowed_corner is part of anti-zig-zag foundations */ + /* Do allow terraforming if allowed_corner is part of anti-zig-zag foundations */ >>> Totally not in the scope of this path if (tileh_old != SLOPE_NS && tileh_old != SLOPE_EW && IsSpecialRailFoundation(f_old)) return autoslope_result; /* Everything is valid, which only changes allowed_corner */ @@ -2488,7 +2471,7 @@ if ((flags & DC_EXEC) != 0) SetRailGroundType(tile, RAIL_GROUND_BARREN); /* allow terraforming */ - return (was_water ? CommandCost(_price.clear_water) : CommandCost()); + return CommandCost(EXPENSES_CONSTRUCTION, (was_water) ? _price.clear_water : (Money)0) ; >>> Whitespace before semicolon, useless parens around was_water. } else { if (_patches.build_on_slopes && AutoslopeEnabled()) { switch (GetRailTileType(tile)) { Index: src/strings.cpp =================================================================== --- src/strings.cpp (revision 11784) +++ src/strings.cpp (working copy) @@ -342,6 +342,9 @@ int j; number *= spec->rate; + + + >>> Can it be more pointless? /* convert from negative */ if (number < 0) { Index: src/terraform_cmd.cpp =================================================================== --- src/terraform_cmd.cpp (revision 11784) +++ src/terraform_cmd.cpp (working copy) @@ -358,11 +357,10 @@ uint h, oldh, curh; CommandCost money; CommandCost ret; - CommandCost cost; + CommandCost cost(EXPENSES_CONSTRUCTION); if (p1 >= MapSize()) return CMD_ERROR; - SET_EXPENSES_TYPE(EXPENSES_CONSTRUCTION); >>> Two whitelines? /* remember level height */ oldh = TileHeight(p1); Index: src/town_cmd.cpp =================================================================== --- src/town_cmd.cpp (revision 11784) +++ src/town_cmd.cpp (working copy) @@ -3480,11 +3472,11 @@ if ((v->vehstatus & VS_STOPPED) == 0) { /* running costs */ - CommandCost cost(v->GetRunningCost() / 364); + CommandCost cost(EXPENSES_TRAIN_RUN, v->GetRunningCost() / 364); + >>> Two white lines? v->profit_this_year -= cost.GetCost() >> 8; - SET_EXPENSES_TYPE(EXPENSES_TRAIN_RUN); SubtractMoneyFromPlayerFract(v->owner, cost); InvalidateWindow(WC_VEHICLE_DETAILS, v->index); Index: src/vehicle.cpp =================================================================== --- src/vehicle.cpp (revision 11784) +++ src/vehicle.cpp (working copy) @@ -1696,8 +1706,8 @@ * Because of this, we can't estimate costs due to wagon removal and we will have to always return 0 and pay manually * Since we pay after each vehicle is replaced and MaybeReplaceVehicle() check if the player got enough money * we should never reach a condition where the player will end up with negative money from doing this */ - SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES); SubtractMoneyFromPlayer(ret); + >>> Pointless white line } }