Opcodes can now abort execution#1180
Conversation
Fixes KSP-KOS#1120 Opcode.cs * Add new property AbortProgram to the Opcode class with a default value of false. * Add new property AbortContext to the Opcode class with a default value of false. * Add Execute method to OpcodeEOP, which sets AbortProgram to true * Add Execute method to OpcodeEOF, which sets AbortContext to true CPU.cs * Call the execute method on all opcodes, including EOF and EOP. * Implement opcodes breaking execution when aborting the program. * Implement opcodes stoping execution when aborting the context. Misc.cs * Shutdown and Reboot now abort the program using the new Opcode AbortProgram property.
CPU.cs * Add logic to prevent the cpu from throwing exceptions if the program is aborted from within a trigger. * The code makes sure the trigger still exists before executing it * The code also checks for a consistent number of opcodes in the program context before moving the instruction pointer.
CPU.cs * Add documentation to explain why the pointers are being checked.
There was a problem hiding this comment.
I'm not comfortable with layering a good feature on top of a bad misfeature. It's true we can't RUN from a trigger, but that's unfortunate, not desirable. Making use of that as a core assumption in implementing something else useful makes it harder to fix the misfeature later.
There was a problem hiding this comment.
It's been a while since I wrote this part. I will have to review to see if there's a good way to get the Abort info back out of ExecuteInstruction As implemented, we just get the binary "continue or not" return value. The problem is that right now the program has already been set back up after the execution of the reboot call opcode. So If we reset the instruction pointer, it causes a misalignment. Maybe we could switch to returning an enum instead of a bool.
For what it's worth, I'm not a fan of run working inside of triggers. Interrupt and event driven programming should avoid long and/or complicated execution if possible, and I would say running a new program is by definition complicated. But I won't argue with you on the feature. I know that in my early kerbscript days I tried to do it, but that was before user functions. I'm sure it would be useful to some one else, so leaving the door open for the feature is a good idea.
There was a problem hiding this comment.
I guess for now we can accept it and flag it as a thing to test in the future after triggers stop being limited by IPU. (the IPU limit is the main reason why the RUN command isn't allowed in the trigger, but there may be other reasons that still hold up after the IPU reason is gone. We'll see when that comes up later. At least the comment in the code will make it clear if this starts breaking later on why it broke.)
|
I tried it, tested it, and see nothing to change in the code, so I'll pull it as-is right on the website. |
Opcodes can now abort execution
Fixes #1120
Modify the Opcode class so that any opcode can abort program or context execution and change the cpu to use this new property. This allows the shutdown and reboot functions to prevent execution of future opcodes as described in the referenced bug.