Trajectories both versions#1
Merged
PiezPiedPy merged 3 commits intoPiezPiedPy:Trajectories-v2.0.0-API-updatefrom Mar 30, 2018
Merged
Trajectories both versions#1PiezPiedPy merged 3 commits intoPiezPiedPy:Trajectories-v2.0.0-API-updatefrom
PiezPiedPy merged 3 commits intoPiezPiedPy:Trajectories-v2.0.0-API-updatefrom
Conversation
…te' into trajectories_both_versions
This probably still needs more changes - giving it back to PiezPiedPy.
Dunbaratu
commented
Mar 30, 2018
| { | ||
| if (trHasTarget == null) | ||
| return null; | ||
| return (bool)trHasTarget.Invoke(null, new object[] { }); |
Author
There was a problem hiding this comment.
Weirdly, I tried to do the above using the C# shorthand:
(bool?)trHasTarget?.Invoke(null, new object[] { });
and the compiler complained. I'm not sure why, as we are using this syntax feature elsewhere.
Owner
There was a problem hiding this comment.
That will because I have not set HasTarget as a nullable type.
public static bool HasTarget()
I will update it to:
public static bool? HasTarget()
Owner
There was a problem hiding this comment.
actually you have used a cast, so it will make no difference if I change the API, maybe bool is not nullable !
Owner
There was a problem hiding this comment.
I'm using VS2017 and compilation went fine. build also works fine testing on my system.
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.
This is what I did so far trying to support both old and new Trajectories API. It does not include any of the checks for version number that you mentioned.
(Suggestion: update your
Trajectories-v2.0.0-API-updatebranch from ourdevelopfirst before looking at this. The majority of the diffs you see here are not related to what I did and are just becausedevelophas changed since this PR was first created.)