Skip to content

Trajectories both versions#1

Merged
PiezPiedPy merged 3 commits intoPiezPiedPy:Trajectories-v2.0.0-API-updatefrom
Dunbaratu:trajectories_both_versions
Mar 30, 2018
Merged

Trajectories both versions#1
PiezPiedPy merged 3 commits intoPiezPiedPy:Trajectories-v2.0.0-API-updatefrom
Dunbaratu:trajectories_both_versions

Conversation

@Dunbaratu
Copy link

@Dunbaratu Dunbaratu commented Mar 30, 2018

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-update branch from our develop first before looking at this. The majority of the diffs you see here are not related to what I did and are just because develop has changed since this PR was first created.)

This probably still needs more changes - giving it back
to PiezPiedPy.
{
if (trHasTarget == null)
return null;
return (bool)trHasTarget.Invoke(null, new object[] { });
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually you have used a cast, so it will make no difference if I change the API, maybe bool is not nullable !

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using VS2017 and compilation went fine. build also works fine testing on my system.

@PiezPiedPy PiezPiedPy merged commit 37fa1e6 into PiezPiedPy:Trajectories-v2.0.0-API-update Mar 30, 2018
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.
PiezPiedPy pushed a commit that referenced this pull request Jul 15, 2020
PiezPiedPy pushed a commit that referenced this pull request Jul 15, 2020
PiezPiedPy pushed a commit that referenced this pull request Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants