progsig: Fix remove and clone program functions being completely broken.

Remove function only removed instructions from the local machine, and
was therefore not MP safe.
Clone function failed to work correctly for non-trivial cases,
and sometimes caused an array out of bounds assertion.

These are replaced by a new commandproc which does each operation
as a single action, which is therefore MP safe.

Remove an unused struct field.

src/command.cpp

@@ -201,6 +201,7 @@ CommandProc CmdOpenCloseAirport;
 CommandProc CmdInsertSignalInstruction;
 CommandProc CmdModifySignalInstruction;
 CommandProc CmdRemoveSignalInstruction;
+CommandProc CmdSignalProgramMgmt;
 
 #define DEF_CMD(proc, flags, type) {proc, #proc, (CommandFlags)flags, type}
 
@@ -362,6 +363,7 @@ static const Command _command_proc_table[] = {
 	DEF_CMD(CmdInsertSignalInstruction,                        0, CMDT_LANDSCAPE_CONSTRUCTION), // CMD_INSERT_SIGNAL_INSTRUCTION
 	DEF_CMD(CmdModifySignalInstruction,                        0, CMDT_LANDSCAPE_CONSTRUCTION), // CMD_MODIFY_SIGNAL_INSTRUCTION
 	DEF_CMD(CmdRemoveSignalInstruction,                        0, CMDT_LANDSCAPE_CONSTRUCTION), // CMD_REMOVE_SIGNAL_INSTRUCTION
+	DEF_CMD(CmdSignalProgramMgmt,                              0, CMDT_LANDSCAPE_CONSTRUCTION), // CMD_SIGNAL_PROGRAM_MGMT
 };
 
 /*!

src/command_type.h

@@ -332,6 +332,7 @@ enum Commands {
 	CMD_INSERT_SIGNAL_INSTRUCTION,    ///< insert a signal instruction
 	CMD_MODIFY_SIGNAL_INSTRUCTION,    ///< modifies a signal instruction
 	CMD_REMOVE_SIGNAL_INSTRUCTION,    ///< removes a signal instruction
+	CMD_SIGNAL_PROGRAM_MGMT,          ///< removes a signal program management command
 
 	CMD_END,                          ///< Must ALWAYS be on the end of this list!! (period)
 };

src/programmable_signals.cpp

@@ -476,7 +476,7 @@ CommandCost CmdInsertSignalInstruction(TileIndex tile, DoCommandFlag flags, uint
 	SignalProgram *prog = GetExistingSignalProgram(SignalReference(tile, track));
 	if (!prog)
 		return_cmd_error(STR_ERR_PROGSIG_NOT_THERE);
-	if (instruction_id > prog->instructions.Length())
+	if (instruction_id >= prog->instructions.Length())
 		return_cmd_error(STR_ERR_PROGSIG_INVALID_INSTRUCTION);
 
 	bool exec = (flags & DC_EXEC) != 0;
@@ -700,3 +700,130 @@ CommandCost CmdRemoveSignalInstruction(TileIndex tile, DoCommandFlag flags, uint
 	InvalidateWindowData(WC_SIGNAL_PROGRAM, (tile << 3) | track);
 	return CommandCost();
 }
+
+static void CloneInstructions(SignalProgram *prog, SignalInstruction *insert_before, SignalInstruction *si)
+{
+	while(true) {
+		if(si == NULL) break;
+		switch(si->Opcode()) {
+			case PSO_SET_SIGNAL: {
+				SignalSet *set = new SignalSet(prog, ((SignalSet*)si)->to_state);
+				set->Insert(insert_before);
+
+				si = ((SignalSet*)si)->next;
+				break;
+			}
+
+			case PSO_IF: {
+				SignalIf *if_ins = new SignalIf(prog);
+				if_ins->Insert(insert_before);
+
+				CloneInstructions(prog, if_ins->if_true, ((SignalIf*)si)->if_true);
+				CloneInstructions(prog, if_ins->if_false, ((SignalIf*)si)->if_false);
+
+				SignalCondition *src_cond = ((SignalIf *) si)->condition;
+				SignalConditionCode code = src_cond->ConditionCode();
+				switch (code) {
+					case PSC_ALWAYS:
+					case PSC_NEVER:
+						if_ins->SetCondition(new SignalSimpleCondition(code));
+						break;
+
+					case PSC_NUM_GREEN:
+					case PSC_NUM_RED: {
+						SignalVariableCondition *cond = new SignalVariableCondition(code);
+						cond->comparator = ((SignalVariableCondition *) src_cond)->comparator;
+						cond->value = ((SignalVariableCondition *) src_cond)->value;
+						if_ins->SetCondition(cond);
+						break;
+					}
+
+					case PSC_SIGNAL_STATE: {
+						SignalStateCondition *src = ((SignalStateCondition *) src_cond);
+						if_ins->SetCondition(new SignalStateCondition(SignalReference(prog->tile, prog->track), src->sig_tile, src->sig_track));
+						break;
+					}
+
+					default: NOT_REACHED();
+				}
+
+				si = ((SignalIf*)si)->after;
+				break;
+			}
+
+			case PSO_LAST:
+			case PSO_IF_ELSE:
+			case PSO_IF_ENDIF:
+				return;
+
+			default:
+				NOT_REACHED();
+		}
+	}
+}
+
+/** Insert a signal instruction into the signal program.
+ *
+ * @param tile The Tile on which to perform the operation
+ * @param p1 Flags and information
+ *   - Bits 0-2    Which track the signal sits on
+ *   - Bits 3-6    Management code
+ *   For clone action:
+ *   - Bits 7-9    Which track the clone source signal sits on
+ * @param p2
+ *   For clone action:
+ *   - Tile of clone source signal
+ * @param text unused
+ */
+CommandCost CmdSignalProgramMgmt(TileIndex tile, DoCommandFlag flags, uint32 p1, uint32 p2, const char *text)
+{
+	bool exec = (flags & DC_EXEC) != 0;
+
+	Track track = Extract<Track, 0, 3>(p1);
+	SignalProgramMgmtCode mgmt = static_cast<SignalProgramMgmtCode>(GB(p1, 3, 4));
+
+	if (!IsValidTrack(track) || !IsPlainRailTile(tile) || !HasTrack(tile, track)) {
+		return CMD_ERROR;
+	}
+
+	if (!IsTileOwner(tile, _current_company)) return_cmd_error(STR_ERROR_AREA_IS_OWNED_BY_ANOTHER);
+
+	SignalProgram *prog = GetExistingSignalProgram(SignalReference(tile, track));
+	if (!prog) return_cmd_error(STR_ERR_PROGSIG_NOT_THERE);
+
+	switch (mgmt) {
+		case SPMC_REMOVE:
+			if (exec) {
+				prog->first_instruction->Remove();
+			}
+			break;
+
+		case SPMC_CLONE: {
+			TileIndex src_tile = p2;
+			Track src_track = Extract<Track, 7, 3>(p1);
+			if (!IsValidTrack(src_track) || !IsPlainRailTile(src_tile) || !HasTrack(src_tile, src_track)) {
+				return CMD_ERROR;
+			}
+
+			if (!IsTileOwner(src_tile, _current_company)) return_cmd_error(STR_ERROR_AREA_IS_OWNED_BY_ANOTHER);
+
+			SignalProgram *src_prog = GetExistingSignalProgram(SignalReference(src_tile, src_track));
+			if (!src_prog) return_cmd_error(STR_ERR_PROGSIG_NOT_THERE);
+
+			if (exec) {
+				prog->first_instruction->Remove();
+				CloneInstructions(prog, prog->last_instruction, ((SignalSpecial*) src_prog->first_instruction)->next);
+			}
+			break;
+		}
+
+		default:
+			return CMD_ERROR;
+	}
+	if (exec) {
+		AddTrackToSignalBuffer(tile, track, GetTileOwner(tile));
+		UpdateSignalsInBuffer();
+		InvalidateWindowData(WC_SIGNAL_PROGRAM, (tile << 3) | track);
+	}
+	return CommandCost();
+}

src/programmable_signals.h

@@ -28,6 +28,11 @@ class SignalInstruction;
 class SignalSpecial;
 typedef SmallVector<SignalInstruction*, 4> InstructionList;
 
+enum SignalProgramMgmtCode {
+	SPMC_REMOVE,      ///< Remove program
+	SPMC_CLONE,       ///< Clone program
+};
+
 /** The actual programmable signal information */
 struct SignalProgram {
 	SignalProgram(TileIndex tile, Track track, bool raw = false);
@@ -212,7 +217,6 @@ class SignalStateCondition: public SignalCondition {
 		SignalReference this_sig;
 		TileIndex sig_tile;
 		Trackdir sig_track;
-		SignalState state;
 };
 
 // -- Instructions

src/programmable_signals_gui.cpp

@@ -256,8 +256,7 @@ public:
 
 			case PROGRAM_WIDGET_REMOVE: {
 				SignalInstruction *ins = GetSelected();
-				if (this->GetOwner() != _local_company || !ins)
+				if (ins == NULL) return;
-					return;
 
 				uint32 p1 = 0;
 				SB(p1, 0, 3, this->track);
@@ -336,15 +335,13 @@ public:
 				ScrollMainWindowToTile(this->tile);
 				// this->RaiseWidget(PROGRAM_WIDGET_GOTO_SIGNAL);
 			} break;
-			case PROGRAM_WIDGET_REMOVE_PROGRAM: {
-				if (this->GetOwner() != _local_company)
-					return;
-				program->first_instruction->Remove();
 
+			case PROGRAM_WIDGET_REMOVE_PROGRAM: {
+				DoCommandP(this->tile, this->track | (SPMC_REMOVE << 3), 0, CMD_SIGNAL_PROGRAM_MGMT | CMD_MSG(STR_ERROR_CAN_T_MODIFY_INSTRUCTION));
 				this->RebuildInstructionList();
 			} break;
-			case PROGRAM_WIDGET_COPY_PROGRAM: {
 
+			case PROGRAM_WIDGET_COPY_PROGRAM: {
 				this->ToggleWidgetLoweredState(PROGRAM_WIDGET_COPY_PROGRAM);
 				this->SetWidgetDirty(PROGRAM_WIDGET_COPY_PROGRAM);
 				if (this->IsWidgetLowered(PROGRAM_WIDGET_COPY_PROGRAM)) {
@@ -356,62 +353,6 @@ public:
 		}
 	}
 
-	void InsertInstruction(SignalInstruction *si, uint32 next)
-	{
-		uint64 p1 = 0;
-		while(true) {
-			if(si == NULL) break;
-			switch(si->Opcode()) {
-				case PSO_SET_SIGNAL: {
-					SB(p1, 0, 3, this->track);
-					SB(p1, 3, 16, next);
-					SB(p1, 19, 8, si->Opcode());
-
-					DoCommandP(this->tile, p1, 0, CMD_INSERT_SIGNAL_INSTRUCTION | CMD_MSG(STR_ERROR_CAN_T_INSERT_INSTRUCTION));
-					this->RebuildInstructionList();
-					si = ((SignalSet*)si)->next;
-				} break;
-
-				case PSO_IF: {
-					SB(p1, 0, 3, this->track);
-					SB(p1, 3, 16, next);
-					SB(p1, 19, 8, si->Opcode());
-
-					DoCommandP(this->tile, p1, 0, CMD_INSERT_SIGNAL_INSTRUCTION | CMD_MSG(STR_ERROR_CAN_T_INSERT_INSTRUCTION));
-					this->RebuildInstructionList();
-
-					SignalInstruction *s = ((SignalIf*)si)->if_true;
-					while(s->Opcode() != PSO_IF_ELSE) {
-						if(s->Opcode() == PSO_IF) s = ((SignalIf*)s)->after;
-						if(s->Opcode() == PSO_SET_SIGNAL) s = ((SignalSet*)s)->next;
-						else break;
-					}
-					InsertInstruction(((SignalIf*)si)->if_true, s->Id());
-					this->RebuildInstructionList();
-
-					s = ((SignalIf*)si)->if_false;
-					while(s->Opcode() != PSO_IF_ENDIF) {
-						if(s->Opcode() == PSO_IF) s = ((SignalIf*)s)->after;
-						if(s->Opcode() == PSO_SET_SIGNAL) s = ((SignalSet*)s)->next;
-						else break;
-					}
-					InsertInstruction(((SignalIf*)si)->if_false, s->Id());
-					this->RebuildInstructionList();
-
-					si = ((SignalIf*)si)->after;
-				} break;
-
-				case PSO_LAST:
-				case PSO_IF_ELSE:
-				case PSO_IF_ENDIF:
-					return;
-
-				default:
-					NOT_REACHED();
-			}
-		}
-	}
-
 	virtual void OnPlaceObject(Point pt, TileIndex tile1) OVERRIDE
 	{
 		if (this->IsWidgetLowered(PROGRAM_WIDGET_COPY_PROGRAM)) {
@@ -446,13 +387,10 @@ public:
 				ShowErrorMessage(STR_ERROR_INVALID_SIGNAL, STR_ERROR_NOT_AN_EXIT_SIGNAL, WL_INFO);
 				return;
 			}
-			program->first_instruction->Remove();
+			DoCommandP(this->tile, this->track | (SPMC_CLONE << 3) | (track1 << 7), tile1, CMD_SIGNAL_PROGRAM_MGMT | CMD_MSG(STR_ERROR_CAN_T_INSERT_INSTRUCTION));
-			this->RebuildInstructionList();
-
-			SignalInstruction *si = ((SignalSpecial*)sp->first_instruction)->next;
-			InsertInstruction(si, program->last_instruction->Id());
 			ResetObjectToPlace();
 			this->RaiseWidget(PROGRAM_WIDGET_COPY_PROGRAM);
+			this->RebuildInstructionList();
 			//OnPaint(); // this appears to cause visual artefacts
 			return;
 		}