PR review changes.#2
Merged
PiezPiedPy merged 3 commits intoPiezPiedPy:Trajectories-v2.0.0-API-updatefrom Apr 4, 2018
Merged
Conversation
Dunbaratu
commented
Apr 4, 2018
| public static bool isVerTwo = false; | ||
| public static bool IsVerTwo { get { return isVerTwo; } private set { isVerTwo = value; } } | ||
| public static bool isVerTwoTwo = false; | ||
| public static bool IsVerTwoTwo { get { return isVerTwoTwo; } private set { isVerTwoTwo = value; } } |
Author
There was a problem hiding this comment.
I made this change so that the code will compile on older versions of C#. The ability to provide initializer values for Properties was added in C# 6.0, and Unity 2017.1.3 still ships with an older Monodevelop that can't compile it. This is relevant for kOS developers using Linux for whom Microsoft's Visual Studio isn't an option.
Owner
|
@Dunbaratu I reviewed the docs, look good to me. |
PiezPiedPy
pushed a commit
that referenced
this pull request
Jul 15, 2020
Change #1: AddTrigger opcode will now treat steering lock triggers as the same if they come from the same point in the program, like they're supposed to (and like they used to do). My pilot.ks program was bogging down very very slowly, to about 3 FPS with the new interrupt priority code. I found the cause: When a program did ``lock throttle`` in a loop, like so: ``` until false { lock throttle to 1. wait 0. } ``` Then each time the "lock throttle" statement was visited, it would insert a NEW trigger for it because it failed to recognize that the previous one was already pending in the trigger list. (The Equals() test was calling it a new trigger, because it was getting a new instance ID in the OpcodeTrigger.Execute() logic. Because it wasn't "Equal" to the existing one, the uniqueness test for inserting a trigger failed to deny the insertion.) Therefore after the program runs a while instead of having like 1 throttle trigger there were hundreds of them, clogging the execution, and making the trigger logic itself rather slow (doing sequential walks through the trigger lists between each opcode with hundreds of triggers in the list instead of typically about 10-ish like there should be in a typical program). Change #2: The new logic on how triggers move from the active list to the callstack means they no longer execute in reverse order because they don't all get stacked up at once (instead they move over one at a time, waiting for the prev one to finish, unless a higher priority one is there that is supposed to stack on top). What this means is that when moving from the trigger list to the stack, it shouldn't be doing so in reverse order like it used to.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@PiezPiedPy If you approve of these edits, then I can merge it.
Mostly I want you to look at this so you can confirm that my user documentation edits (to the
Trajectories.rstfile) are telling the truth and not misleading the users.Also, is the fact that there's no way for a script to un-set the target once
SETTARGET()has been called a problem? I don't know the Trajectories mod well enough to know if being unable to unset the target once it's been set is normal.