Merged
Conversation
…t_system' into test_PR_2292
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.
Interrupt priorities
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.
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.
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.
No description provided.