(svn r8279) -Regression (r8274): Removing signals could fail under certain circumstances (not the proper bits of p1/p2 were set to trackbits). Moved trackbits back to bits 0..2, ctrl to 3, semaphore to 4. Also lower the decision of the ctrl-override of signal/semaphore from the GUI into the cmd function since we are passing the ctrl-bit anyways. Bug(s) found by Frostregen; thanks

2 files changed, 25 insertions, 29 deletions

diff --git a/src/rail_cmd.cpp b/src/rail_cmd.cpp
index 242145607..a5643a48f 100644
--- a/src/rail_cmd.cpp
+++ b/src/rail_cmd.cpp
@@ -605,17 +605,17 @@ int32 CmdBuildTrainDepot(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
  * have any signals, bit 4 (cycle signal-type) is ignored
  * @param tile tile where to build the signals
  * @param p1 various bitstuffed elements
- * - p1 = (bit 0)   - 1 = override signal/semaphore, or pre/exit/combo signal (CTRL-toggle)
- * - p1 = (bit 1)   - 0 = signals, 1 = semaphores
- * - p1 = (bit 2-4) - track-orientation, valid values: 0-5 (Track enum)
+ * - p1 = (bit 0-2) - track-orientation, valid values: 0-5 (Track enum)
+ * - p1 = (bit 3)   - 1 = override signal/semaphore, or pre/exit/combo signal (CTRL-toggle)
+ * - p1 = (bit 4)   - 0 = signals, 1 = semaphores
  * @param p2 used for CmdBuildManySignals() to copy direction of first signal
  * TODO: p2 should be replaced by two bits for "along" and "against" the track.
  */
 int32 CmdBuildSingleSignal(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 {
-	bool pre_signal = HASBIT(p1, 0);
-	SignalVariant sigvar = HASBIT(p1, 1) ? SIG_SEMAPHORE : SIG_ELECTRIC;
-	Track track = (Track)GB(p1, 2, 3);
+	Track track = (Track)GB(p1, 0, 3);
+	bool pre_signal = HASBIT(p1, 3);
+	SignalVariant sigvar = (pre_signal ^ HASBIT(p1, 4)) ? SIG_SEMAPHORE : SIG_ELECTRIC;
 	int32 cost;
 
 	if (!ValParamTrackOrientation(track) || !IsTileType(tile, MP_RAILWAY) || !EnsureNoVehicle(tile))
@@ -700,9 +700,9 @@ int32 CmdBuildSingleSignal(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
  * @param tile start tile of drag
  * @param p1  end tile of drag
  * @param p2 various bitstuffed elements
- * - p2 = (bit  0)    - 1 = override signal/semaphore, or pre/exit/combo signal (CTRL-toggle)
- * - p2 = (bit  1)    - 0 = signals, 1 = semaphores
- * - p2 = (bit  2- 4) - track-orientation, valid values: 0-5 (Track enum)
+ * - p2 = (bit  0- 2) - track-orientation, valid values: 0-5 (Track enum)
+ * - p2 = (bit  3)    - 1 = override signal/semaphore, or pre/exit/combo signal (CTRL-toggle)
+ * - p2 = (bit  4)    - 0 = signals, 1 = semaphores
  * - p2 = (bit  5)    - 0 = build, 1 = remove signals
  * - p2 = (bit 24-31) - user defined signals_density
  */
@@ -713,9 +713,9 @@ static int32 CmdSignalTrackHelper(TileIndex tile, uint32 flags, uint32 p1, uint3
 	bool error = true;
 	TileIndex end_tile;
 
-	bool mode = HASBIT(p2, 0);
-	bool semaphores = HASBIT(p2, 1);
-	Track track = (Track)GB(p2, 2, 3);
+	Track track = (Track)GB(p2, 0, 3);
+	bool mode = HASBIT(p2, 3);
+	bool semaphores = HASBIT(p2, 4);
 	bool remove = HASBIT(p2, 5);
 	Trackdir trackdir = TrackToTrackdir(track);
 	byte signal_density = GB(p2, 24, 8);
@@ -759,10 +759,9 @@ static int32 CmdSignalTrackHelper(TileIndex tile, uint32 flags, uint32 p1, uint3
 	for (;;) {
 		// only build/remove signals with the specified density
 		if (signal_ctr % signal_density == 0) {
-			int p1 = 0;
-			SB(p1, 0, 1, mode);
-			SB(p1, 1, 1, semaphores);
-			SB(p1, 2, 3, TrackdirToTrack(trackdir));
+			uint32 p1 = GB(TrackdirToTrack(trackdir), 0, 3);
+			SB(p1, 3, 1, mode);
+			SB(p1, 4, 1, semaphores);
 			ret = DoCommand(tile, p1, signals, flags, remove ? CMD_REMOVE_SIGNALS : CMD_BUILD_SIGNALS);
 
 			/* Be user-friendly and try placing signals as much as possible */
@@ -796,13 +795,13 @@ int32 CmdBuildSignalTrack(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 /** Remove signals
  * @param tile coordinates where signal is being deleted from
  * @param various bitstuffed elements, only track information is used
- * - p1 = (bit  0)    - override signal/semaphore, or pre/exit/combo signal (CTRL-toggle)
- * - p1 = (bit  1)    - 0 = signals, 1 = semaphores
- * - p1 = (bit  2- 4) - track-orientation, valid values: 0-5 (Track enum)
+ * - p1 = (bit  0- 2) - track-orientation, valid values: 0-5 (Track enum)
+ * - p1 = (bit  3)    - override signal/semaphore, or pre/exit/combo signal (CTRL-toggle)
+ * - p1 = (bit  4)    - 0 = signals, 1 = semaphores
  */
 int32 CmdRemoveSingleSignal(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 {
-	Track track = (Track)GB(p1, 2, 3);
+	Track track = (Track)GB(p1, 0, 3);
 
 	if (!ValParamTrackOrientation(track) ||
 			!IsTileType(tile, MP_RAILWAY) ||
diff --git a/src/rail_gui.cpp b/src/rail_gui.cpp
index bb5dc839c..1fe4691b3 100644
--- a/src/rail_gui.cpp
+++ b/src/rail_gui.cpp
@@ -182,10 +182,9 @@ static void GenericPlaceSignals(TileIndex tile)
 	}
 
 	if (!_remove_button_clicked) {
-		uint32 p1 = 0;
-		SB(p1, 0, 1, _ctrl_pressed);
-		SB(p1, 1, 1, _ctrl_pressed ^ (_cur_year < _patches.semaphore_build_before));
-		SB(p1, 2, 3, i);
+		uint32 p1 = GB(i, 0, 3);
+		SB(p1, 3, 1, _ctrl_pressed);
+		SB(p1, 4, 1, _cur_year < _patches.semaphore_build_before);
 
 		DoCommandP(tile, p1, 0, CcPlaySound1E,
 			CMD_BUILD_SIGNALS | CMD_AUTO | CMD_MSG(STR_1010_CAN_T_BUILD_SIGNALS_HERE));
@@ -365,17 +364,15 @@ static void HandleAutodirPlacement(void)
 static void HandleAutoSignalPlacement(void)
 {
 	TileHighlightData *thd = &_thd;
-	byte trackstat = thd->drawstyle & 0xF; // 0..5
+	uint32 p2 = GB(thd->drawstyle, 0, 3); // 0..5
 
 	if (thd->drawstyle == HT_RECT) { // one tile case
 		GenericPlaceSignals(TileVirtXY(thd->selend.x, thd->selend.y));
 		return;
 	}
 
-	int p2 = 0;
-	SB(p2,  0, 1, _ctrl_pressed);
-	SB(p2,  1, 1, _ctrl_pressed ^ (_cur_year < _patches.semaphore_build_before));
-	SB(p2,  2, 3, trackstat);
+	SB(p2,  3, 1, _ctrl_pressed);
+	SB(p2,  4, 1, _cur_year < _patches.semaphore_build_before);
 	SB(p2, 24, 8, _patches.drag_signals_density);
 
 	/* _patches.drag_signals_density is given as a parameter such that each user