Skip to content

Update from upstream#1

Merged
jonnyboyC merged 518 commits intojonnyboyC:developfrom
KSP-KOS:develop
May 21, 2019
Merged

Update from upstream#1
jonnyboyC merged 518 commits intojonnyboyC:developfrom
KSP-KOS:develop

Conversation

@jonnyboyC
Copy link
Owner

No description provided.

Dunbaratu and others added 30 commits June 6, 2018 18:24
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.
It turns out there was a problem buried deep in
the compiler, and it was introduced first with commit
number #06506dc, with Pull Request #2156.

That commit altered the boilerplate code inserted for
triggered locks (steering, throttle, etc), reducing
the number of instructions that existed in the
boilerplate code chunk.  But just prior to those
edits, on the line above, there was a br.true +4.
Becuse of those edits, there was now one fewer
opcode to skip over and the br.true +4 needed to be
changed to a br.true +3, and it hadn't been.

The result is that when the function $steering*
already exists, and it tries to skip over the
setting of it, it skips the wrong distance missing
a "push", and thus everything on the stack is
mis-aligned after that.

The reason it hasn't been noticed before is that usually,
this case doesn't happen in typical cases.  It requires
running a boot file that runs a compiled KSM file to
encounter the case where that br.true will fire off. Most
of the time it was false and not executing, thus the wrong
size of how far it jumps wasn't having any effect.
The comparison used to see if a trigger was present
to remove it was using the '==' operator instead of the
Equals() method.  This can cause C# to do a reference-equals
test when what you wanted was the overridden meaning of
Equals() defined for the class.  (Because we keep making
new instances of recurring triggers, this might have been
preventing the trigger removal since the next instance to
execute is a different instance from the previous one even
though they are "Equal()" to each other.)
When doing the equality check it only tested for "zero"
Instance count (meaning don't-care) on one side of the
two operands.  That would mean that aaa.Equals(bbb) gave a
different answer from bbb.Equals(aaa), which is a
non-intuitive messy situation.
This should prove this fixed it, because the "wait" that forces
the IPU boundary to come in the problem spot is still there,
but the problem stops happening.
Fixes #2231 by shortening the test string.
This fixes raw control so it works again, BUT makes the
original bug still happen with raw control.  I can't fix it.
The design prevents any memory from being retained about which
CPU is resposible for each axis of raw control setting.

The FlightControl class (derived from IFlightControl) is
the class that handles raw steering for the vessel, and that
class has NO memory of which CPU did the setting.  This is
part of the deliberate design so more than one CPU can control
the raw controls (one CPU could move the pitch while a different
CPU moves the yaw, for example).

So the system cannot detect if the setting a CPU did was from the CPU
that just got decoupled or not.

To fix it would require a massive overhaul of raw controls so the
system remembers which CPU last called a SetSuffix for which
axis of control.
The ability to detect the need to re-default the lock steering
doesn't seem to be always happening right when the part count
changes.  Sometimes the problem isn't detected till a moment later,
which I assume must be a race condition.  So I added the ability
to flag the problem when detected to force it to go through
the work again to check for stale controls.  This seems to fix
the problem more consistently.
Added explanation on how to find the KSP folder.
Replaced ` if boolean { ` with ` if boolean = true { ` so its easier to read for beginners (sorry dunbaratu)
Removed nonsense \ at line 577
…hyperbolic orbits must now show TA clamped to [-180;180] and unclamped mean anomaly at epoch
/Ships/Script/ changed to Ships/Script/
included dunbaratu's comments and added explanation of what a physics tick is
things it was talking about were not commands.
Dunbaratu and others added 28 commits April 9, 2019 00:40
The word "IKOSTargetable" was accidentally deleted from
the declaration header of class BodyTarget by my PR #2460.

The SET TARGET command has a check to see if the value being
set is an actual target object or just a string name that
needs to be looked up.  That check was based on seeing if
the value could be cast to IKOSTargetable or not.  When
that was accidentally removed from BodyTarget's header,
it broke that check.
I had inserted a check to throw an exception if you
try to SET TARGET to an illegal body, one the game
won't allow because you currently orbit it.  In retrospect,
that removes a feature people might have been using, which
is that trying to set the target to your current SOI body is
a roundabout way to unset the target (the game will refuse the
attempt and deselect the target, but the kOS script would
be fine with it and wouldn't crash).

Changing that into an exception means previously working code
could break, so I removed the check again.
To fix #2488 - KSP calls all solar panels "ModuleDeployableSolarPanel" even if
they aren't deployable.   The only way to tell if it's actually deployable
(versus fixed in place) is to see if it had an animation defined.

It seems the protection against calling Extend() or Retract() on
panels that aren't deployable only existed in the UI where
KSP disabled the gui fields if the animation isn't there.
That same protection didn't exist in the lower level API, so
I had to add the same protections in our code.
Fixes #2496 by changing link to the more
reliable internal Sphinx :ref: link style.
…earance' into testing_dialog_anchor_point_fix
Fix for regressions in Activate and Shutdown.
Clarified naming of engine lists.
@jonnyboyC jonnyboyC merged commit 42664d9 into jonnyboyC:develop May 21, 2019
jonnyboyC pushed a commit that referenced this pull request Nov 29, 2019
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.