Issue 1492 steering manager disassociation #1544
Merged
Conversation
This file contains 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
hvacengi
added
the
Not Ready
This was referenced Mar 23, 2016
hvacengi
force-pushed
the
issue-1492-steering_manager_disassociation
branch
4 times, most recently
from
494c0e6
to
aafb646
Compare
kOSVesselModule.cs * Implement new vessel module to manage vessel specific values * Manage steering manager IFlightControlParameter.cs * New interface for flight controls kOS.csproj * Reference the new files FlightControlManager.cs * Stop using SteeringManagerProvider, and reference the new vessel module instead FlightStats.cs * Get the STEERINGMANGER from the vessel module instead of the provider SteeringManager.cs * Implement IFlightControlParameter interface * Stop using provider
SteeringManagerProvider.cs * Removed abandoned class kOS.csproj * Remove reference to SteeringManagerProvider.cs IFlightControlParameter.cs * Added methods for UpdateValue, GetValue, GetShared, and CopyFrom. * Probably will eventually remove the Value property FlightControlManager.cs * Update to ControlPartId kOSVesselModule.cs * Switch to Vessel.id based tracking to simplify dictionary lookup * Add initialized flag to prevent random instances of FixedUpdate being called before Start. * Don't remove parts from partLookup when clearing the childParts list. * Clear partLookup in Destroy if no other VesselModules remain (should only occur if switching to a vessel out of range of the current vessel, or when changing scenes) * Copy flight control parameters when experiencing a staging or docking event. * Manage enable/disabling parameters for vessel changes too. SteeringManager.cs * Implement new interface methods * Retask old copy logic * Fix stopwatch that broke in the last commit.
SteeringManager.cs * Reduce the value of EPSILON to help with fine steering control. * Fixes a particular issue where large torque values and *very* small MoI's resulted in steering based on the integral component which was prone to running away uncontrollably.
SteeringManager.cs * Use UpdateHandler.CurrentFixedTime instead of planetarium time (smaller value, less chance for floating point errors) * Update the state vectors and display vectors if SAS is enabled * Hide angular velocity, rcs, and engine thrust display vectors when SAS is enabled. * Don't throw an exception when getting target direction, throwing an exception won't trigger the kOS logger. Instead, log the exception directly. PIDLoop.cs * Update integral/derivative logic to use the previous dTerm and iTerm if dt is not greater than 0.
IFlightControlParameter.cs * Remove Value property and UpdateValue method that supported setting the value without passing SharedObjects FlightControlManager.cs * Remove references to removed properties SteeringManager.cs * Removed MoI adjustment logic, the stock MoI calculation has been improved to be much more accurate. Leaving torque adjustment for now to account for control surfaces. * Fixes KSP-KOS#1588 check the rcs module's new pitch, yaw, and roll enable properties kOSVesselModule.cs * Change Awake method to OnAwake to prevent breaking KSP1.1's ability to save fields on VesselModule
hvacengi
force-pushed
the
issue-1492-steering_manager_disassociation
branch
from
aafb646
to
a627a22
Compare
Moved SteeringManager.cs from kOS/Binding to kOS/Control
Change SteeringManager.cs namespace and add reference to kOS.Control namespace where needed.
SteeringManager.cs * Convert steering logic to use ITorqueProvider * Disabled torque adjustment by default.
…492-steering_manager_disassociation
kOSVesselModule.cs * Add event handler to destroy the module when the vessel docks to another vessel. This was preventing the module from re-initializing and subscribing to autopilot events. * Refactor method names to match kOS style recomendations. * Add XML documentation to all methods.
SteeringManager.cs * Remove old logic that's no longer in use (engine/rcs vectors, torque calculation, ForceVector class) * Throw deprecation exceptions for SHOWTHRUSTVECTORS and SHOWRCSVECTORS
Fixes KSP-KOS#1608 PIDLoop.cs * fix issue where ResetI was not resetting the ITerm itself.
hvacengi
force-pushed
the
issue-1492-steering_manager_disassociation
branch
from
6fa0568
to
89b55da
Compare
hvacengi
added
the
Pending Documentation
Update documentation to reflect deprecating SHOWRCSVECTORS and SHOWTHRUSTVECTORS.
hvacengi
removed
Not Ready
|
This should now be ready for review and merging. I will leave the migration of other steering parameters to another PR so that these rather important fix can get merged for the next release. |
SteeringManager.cs * Provide temporary fix for MoI calculation, to be removed after KSP's next update.
hvacengi
added
the
Breaking
|
This is technically breaking because I depreciated 2 suffixes (SHOWTHRUSTVECTORS and SHOWRCSVECTORS) |
…492-steering_manager_disassociation
SteeringManager.cs * Move conversion of the "set" value from inside the autopilot update and into the actual set method itself. This means that a conversion exception will break execution and display the error (on the next physics tick) preventing further script execution. KSPLogger.cs * Remove requirement for null or empty filepath for noting that an exception is in the built-in steering update (previously this line was not getting triggered unless called from the interpreter).
hvacengi
force-pushed
the
issue-1492-steering_manager_disassociation
branch
from
a1ed8b0
to
dc2277f
Compare
|
I've updated this to throw an exception when setting the I also fixed it so the trace log will identify the error as part of the built in steering update. I really wanted to make it properly identify the line causing the error, but that's really complicated given how the trigger is created. |
kOSVesselModule.cs * Changed logger calls to `SuperVerbose` instead of `LogError` so that they are hidden under "release" builds.
| // We only want to copy the parameters themselves once (because they are vessel dependent | ||
| // not part dependent) but we still need to iterate through each part since "control" | ||
| // itself is part dependent. | ||
| if (partLookup[id].ID != BaseId) paramDestination.CopyFrom(paramOrigin); |
There was a problem hiding this comment.
We may want to change this logic to only copy "enabled" parameters, or perhaps only "enabled" parameters who's controlling part is on the new vessel. This could be done pretty easily by moving the copy line into the if block below. I'm open to discussion on it though.
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.
Fixes #123
Fixes #1492
Fixes #1587
Fixes #1608
This is my "progress" update for the code to fix the issue of disassociation of the steering manager from the CPU vessel. It serves as a fairly sweeping overhaul for how the steering manager is used, so that the manager itself exists only once and is managed by a
VesselModule. Eventually (possibly before this PR is complete) I intend to move everything that is "vessel" specific rather than "part" specific into this module (or at least to be managed by the module). While working on some other items, I realized I was starting to diverge into some similar changes as what I did in this branch, so I wanted to make sure that I got it posted and published so that every one can kick me to make sure it gets finished.Features:
VesselModuleIFlightControlParameterinterfaceSteeringManagertoControlnamespace and implement `IFlightControlParameterITorqueProviderinterface for calculating potential torquePIDLoop