From 5d443a248217eb2aa22c8060774c9a0b6b51b4b2 Mon Sep 17 00:00:00 2001 From: tron Date: Sat, 22 Jan 2005 14:52:20 +0000 Subject: [PATCH] (svn r1585) Rewrite CmdBuildSignals() This addresses several issues: - Correct comments - Check input parameters for validity (and don't assert() them) - Reorder checks if action is possible to produce more meaningful error messages - Correct bug where money was charged for an action which should be free - Kill warning about uninitialized variable, because the variable exists no more - Make more clear how the function works (at least i hope so) --- rail_cmd.c | 143 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 86 insertions(+), 57 deletions(-) diff --git a/rail_cmd.c b/rail_cmd.c index df2c59a463..f2c344e0f3 100644 --- a/rail_cmd.c +++ b/rail_cmd.c @@ -862,96 +862,125 @@ int32 CmdRenameWaypoint(int x, int y, uint32 flags, uint32 p1, uint32 p2) } -/* build signals, alternate between double/single, signal/semaphore, pre/exit/combo -signals - p1 = (lower 3 bytes) - track-orientation - p1 = (byte 4) - semaphores/signals - p2 = used for CmdBuildManySignals() to copy style first signal +/* build signals, alternate between double/single, signal/semaphore, + * pre/exit/combo-signals + * p1 bits 0-2 - track-orientation, valid values: 0-5 + * p1 bit 3 - choose semaphores/signals or cycle normal/pre/exit/combo + * depending on context + * p2 = used for CmdBuildManySignals() to copy style of first signal */ int32 CmdBuildSignals(int x, int y, uint32 flags, uint32 p1, uint32 p2) { - uint tile; + TileIndex tile = TILE_FROM_XY(x, y); + bool semaphore; + bool pre_signal; + uint track = p1 & 0x7; byte m5; int32 cost; - int track = p1 & 0x7; - byte a,b,c,d; - assert(track >= 0 && track < 6); // only 6 possible track-combinations + if (!(track < 6) || // only 6 possible track-combinations + !IsTileType(tile, MP_RAILWAY) || + !EnsureNoVehicle(tile)) + return CMD_ERROR; - SET_EXPENSES_TYPE(EXPENSES_CONSTRUCTION); + // Protect against invalid signal copying + if (p2 != 0 && (p2 & _signals_table_both[track]) == 0) + return CMD_ERROR; - tile = TILE_FROM_XY(x,y); + m5 = _map5[tile]; - if (!EnsureNoVehicle(tile)) + if (m5 & 0x80 || // mustn't be a depot + !HASBIT(m5, track)) // track must exist return CMD_ERROR; - // must be railway, and not a depot, and it must have a track in the suggested position. - if (!IsTileType(tile, MP_RAILWAY) || (m5 = _map5[tile], m5 & 0x80) || !HASBIT(m5, track)) - return CMD_ERROR; + if (!CheckTileOwnership(tile)) return CMD_ERROR; _error_message = STR_1005_NO_SUITABLE_RAILROAD_TRACK; - // check rail combination { byte m = m5 & RAIL_BIT_MASK; - if (m != RAIL_BIT_DIAG1 && m != RAIL_BIT_DIAG2 && m != RAIL_BIT_UPPER && m != RAIL_BIT_LOWER && - m != RAIL_BIT_LEFT && m != RAIL_BIT_RIGHT && m != (RAIL_BIT_UPPER|RAIL_BIT_LOWER) && m != (RAIL_BIT_LEFT|RAIL_BIT_RIGHT)) + if (m != RAIL_BIT_DIAG1 && + m != RAIL_BIT_DIAG2 && + m != RAIL_BIT_UPPER && + m != RAIL_BIT_LOWER && + m != RAIL_BIT_LEFT && + m != RAIL_BIT_RIGHT && + m != (RAIL_BIT_UPPER | RAIL_BIT_LOWER) && + m != (RAIL_BIT_LEFT | RAIL_BIT_RIGHT)) return CMD_ERROR; } - // check ownership - if (!CheckTileOwnership(tile)) - return CMD_ERROR; + SET_EXPENSES_TYPE(EXPENSES_CONSTRUCTION); - // calculate masks for.. - a = _signals_table[track]; // .. signal for this track in one direction - b = _signals_table[track + 8]; // .. signal for this track in the other direction - c = a | b; // .. 2-way signal for this track + // Same bit, used in different contexts + semaphore = pre_signal = HASBIT(p1, 3); - // If it had signals previously it is free to change orientation/pre-exit-combo signals - cost = 0; - if (!(m5 & RAIL_TYPE_SIGNALS)) { + if ((_map3_lo[tile] & _signals_table_both[track]) == 0) { + // build new signals cost = _price.build_signals; } else { - d = _map3_lo[tile] & c; // mask of built signals. it only affects &0xF0 - if (d == 0) cost += _price.build_signals; // no signals built yet - // if converting signals<->semaphores, charge the player for it - if (p2 && ((HASBIT(p1, 3) && !HASBIT(_map3_hi[tile], 2)) || (!HASBIT(p1, 3) && HASBIT(_map3_hi[tile], 2)) ) ) - cost += _price.build_signals + _price.remove_signals; + if (p2 != 0 && + ((semaphore && !HASBIT(_map3_hi[tile], 2)) || + (!semaphore && HASBIT(_map3_hi[tile], 2)))) { + // convert signals <-> semaphores + cost = _price.build_signals + _price.remove_signals; + } else { + // it is free to change orientation/pre-exit-combo signals + cost = 0; + } } if (flags & DC_EXEC) { - - - if (!(m5 & RAIL_TYPE_SIGNALS)) { // if there are no signals yet present on the track + if (!(m5 & RAIL_TYPE_SIGNALS)) { + // there are no signals at all on this tile yet _map5[tile] |= RAIL_TYPE_SIGNALS; // change into signals _map2[tile] |= 0xF0; // all signals are on _map3_lo[tile] &= ~0xF0; // no signals built by default - _map3_hi[tile] = (p1 & 8) ? 4 : 0;// initial presignal state, semaphores depend on ctrl key - d = 0; // no existing signals - if (!p2) - goto ignore_presig; + _map3_hi[tile] = semaphore ? 4 : 0; } - if (!p2) { // not called from CmdBuildManySignals - if (!HASBIT(p1, 3)) { // not CTRL pressed - ignore_presig: - // Alternate between a|b, b, a - if ( d != 0 && d != a) - c = (d == c)?b:a; - - _map3_lo[tile] = (_map3_lo[tile]&~(a|b)) | c; - } else // CTRL pressed - _map3_hi[tile] = (_map3_hi[tile] & ~3) | ((_map3_hi[tile] + 1) & 3); - } else { - /* If CmdBuildManySignals is called with copying signals, just copy the style of the first signal - * given as parameter by CmdBuildManySignals */ - switch (track) { - case 2: case 4: _map3_lo[tile] = (p2&0xC0) | (_map3_lo[tile]&~0xC0); break; - case 3: case 5: _map3_lo[tile] = (p2&0x30) | (_map3_lo[tile]&~0x30); break; - default : _map3_lo[tile] = (p2&0xF0) | (_map3_lo[tile]&0xF); + if (p2 == 0) { + if ((_map3_lo[tile] & _signals_table_both[track]) == 0) { + // build new signals + _map3_lo[tile] |= _signals_table_both[track]; + } else { + if (pre_signal) { + // cycle between normal -> pre -> exit -> combo -> ... + byte type = (_map3_hi[tile] + 1) & 0x03; + _map3_hi[tile] &= ~0x03; + _map3_hi[tile] |= type; + } else { + // cycle between two-way -> one-way -> one-way -> ... + switch (track) { + case 3: + case 5: { + byte signal = (_map3_lo[tile] - 0x10) & 0x30; + if (signal == 0) signal = 0x30; + _map3_lo[tile] &= ~0x30; + _map3_lo[tile] |= signal; + break; + } + + default: { + byte signal = (_map3_lo[tile] - 0x40) & 0xC0; + if (signal == 0) signal = 0xC0; + _map3_lo[tile] &= ~0xC0; + _map3_lo[tile] |= signal; + break; + } + } + } } + } else { + /* If CmdBuildManySignals is called with copying signals, just copy the + * style of the first signal given as parameter by CmdBuildManySignals */ + _map3_lo[tile] &= ~_signals_table_both[track]; + _map3_lo[tile] |= p2 & _signals_table_both[track]; // convert between signal<->semaphores when dragging - HASBIT(p1, 3) ? SETBIT(_map3_hi[tile], 2) : CLRBIT(_map3_hi[tile], 2); + if (semaphore) + SETBIT(_map3_hi[tile], 2); + else + CLRBIT(_map3_hi[tile], 2); } MarkTileDirtyByTile(tile);