From 769b8ae0961ee63f9229514c7ab93087ecb0eb2c Mon Sep 17 00:00:00 2001 From: Jonathan G Rennison Date: Tue, 24 Nov 2015 01:03:09 +0000 Subject: [PATCH] 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 | 2 + src/command_type.h | 1 + src/programmable_signals.cpp | 129 ++++++++++++++++++++++++++++++- src/programmable_signals.h | 6 +- src/programmable_signals_gui.cpp | 74 ++---------------- 5 files changed, 142 insertions(+), 70 deletions(-) diff --git a/src/command.cpp b/src/command.cpp index f0be03f2cf..c35f4863bd 100644 --- a/src/command.cpp +++ b/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 }; /*! diff --git a/src/command_type.h b/src/command_type.h index 5853909b8e..f7772092bb 100644 --- a/src/command_type.h +++ b/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) }; diff --git a/src/programmable_signals.cpp b/src/programmable_signals.cpp index bf7a5490a6..6bdb23545c 100644 --- a/src/programmable_signals.cpp +++ b/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(p1); + SignalProgramMgmtCode mgmt = static_cast(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(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(); +} diff --git a/src/programmable_signals.h b/src/programmable_signals.h index 5cf251c6ed..d69b4b352c 100644 --- a/src/programmable_signals.h +++ b/src/programmable_signals.h @@ -28,6 +28,11 @@ class SignalInstruction; class SignalSpecial; typedef SmallVector 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 diff --git a/src/programmable_signals_gui.cpp b/src/programmable_signals_gui.cpp index 90d11f5b69..6a26410845 100644 --- a/src/programmable_signals_gui.cpp +++ b/src/programmable_signals_gui.cpp @@ -256,8 +256,7 @@ public: case PROGRAM_WIDGET_REMOVE: { SignalInstruction *ins = GetSelected(); - if (this->GetOwner() != _local_company || !ins) - return; + if (ins == NULL) 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(); - this->RebuildInstructionList(); - - SignalInstruction *si = ((SignalSpecial*)sp->first_instruction)->next; - InsertInstruction(si, program->last_instruction->Id()); + DoCommandP(this->tile, this->track | (SPMC_CLONE << 3) | (track1 << 7), tile1, CMD_SIGNAL_PROGRAM_MGMT | CMD_MSG(STR_ERROR_CAN_T_INSERT_INSTRUCTION)); ResetObjectToPlace(); this->RaiseWidget(PROGRAM_WIDGET_COPY_PROGRAM); + this->RebuildInstructionList(); //OnPaint(); // this appears to cause visual artefacts return; }