Git Product home page Git Product logo

kspcommunityfixes's People

Contributors

gotmachine avatar jonnyothan avatar jules-bertholet avatar nathankell avatar rcrockford avatar tinygrox avatar zhangyuesai avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar

kspcommunityfixes's Issues

EVA Kerbal science data and cargo parts recovery isn't correct in KSP 1.11+

GetAllProtoPartsIncludingCargo() will exclude protoparts whose AvailablePart.name == "kerbalEVA".

This method was introduced in 1.11.0 with the stock inventory system. Prior to that, callers (mainly
Funding.onVesselRecoveryProcessing() and ResearchAndDevelopment.onVesselRecoveryProcessing())
were directly iterating on the protopart list without any filtering.

The reason this filter exists is because when recovering a part, the inventory of the part crew is
added to the list by calling GetAllProtoPartsFromCrew(), which is getting stored parts in the "hidden"
kerbal inventory modules. But in the case of an EVA kerbal, that inventory module is on the part, so
its cargo parts would be added twice to the resulting list.

The stock implementation is causing two bugs :

  • Since it exclude the kerbalEVA part from the list, onVesselRecoveryProcessing() callbacks aren't
    processing it, which results in science data and IPartCostModifier modules costs not being recovered.
  • Because it only check for the exact kerbalEVA AP name, it only filter that part and let all other
    EVA variants go through (kerbalEVAfemale, kerbalEVAFuture, etc). So for all variants, cargo parts are
    recovered in double quantity, and science data / cost modifiers are recovered.

The kerbalEVA filter was added in 1.12.0, very likely in an attempt to fix the initial "EVA inventories are recovered twice" 1.11.0 issue (but no mention of that in the changelog). But this fix doesn't entirely fix the original issue and introduce new ones.

To properly fix this :

  • We undo the kerbalEVA protopart filter from GetAllProtoPartsIncludingCargo() introduced in 1.12.0, so those protoparts
    are returned (consistent with pre-1.12 behavior for all parts, and EVA variants behavior post-1.12)
  • We don't call GetAllProtoPartsFromCrew() to append the cargo parts if the protopart is an EVA Kerbal

Stock SAS improvements

The stock SAS has some obvious issues :

  1. UpdateVesselTorque() is called after ControlUpdate(), meaning it's always using one frame outdated torque results.
  2. The GetPotentialTorque() results are incorrectly accounted for. GetTotalVesselTorque() and further methods (UpdateMaximumAcceleration(), AutoTuneScalar() and CheckCoasting()) all ignore the fact that the available torque along an axis can be asymmetrical depending on the direction. At the GetTotalVesselTorque() level, only the maximum of the neg/pos torque is kept:
    zero.x = Mathf.Max(posTorque.x, negTorque.x);
    zero.y = Mathf.Max(posTorque.y, negTorque.y);
    zero.z = Mathf.Max(posTorque.z, negTorque.z);

Quick testing show that just changing Mathf.Max to Mathf.Min results in a significant reduction of the SAS overshooting its target when testing RCS-only attitude control with the stock Dynawing (which has an asymmetrical RCS setup).

But this is a band-aid, and while this reduce overshooting this will also produce over-actuation initially, so that's far from ideal and might not be desirable. Ideally, the implementation shouldn't discard the pos/neg information and use it where relevant.

A bigger scope project would be to entirely replace the stock SAS with an improved attitude controller. MechJeb implemented a new "better" attitude controller a while ago. But I fear the conclusions of my last extensive review and comparison of all the KSP attitude controllers lying around still hold true : despite its inefficiency, the stock SAS still has the benefit of being able to handle anything you throw at it without major stability issues.

Module indexing mismatch on config changes

When partmodules are added/removed in a part config following a mod installation/removal/update, the index of the persisted protomodules in existing saves won't match the prefab module index anymore. Note that this also apply to loading a ShipConstruct.

This mean that next time a changed part will be loaded from it persisted protopart, the protomodule list isn't matching the prefab module list. KSP has some provision to handle that. However, it fundamentally can't handle the case of multiple same type modules being present in a part.

The "reconnection" code is implemented in Part.LoadModule(ConfigNode node, ref int moduleIndex).
Before attempting to load a protomodule node, it check if :

  • moduleIndex is within the range of the part modules count
  • the module type at moduleIndex match

So this will be triggered for all modules whose index is greater than the index of the module that was added/removed from the part config.

If the conditions is true, it will attempt to find the right module by iterating on all the part modules, and will then load the node in the first found whose type match. This mean that if multiple modules of the same type exists, all protomodule nodes will be loaded in the first occurence, and other occurences will be reset to the prefab configuration.

There is no way to totally resolve that issue without requiring that all "multiple occurence" modules in a part config define a per-part unique identifier that would be propagated to the protopart. However, the stock "reconnection" logic could be improved to massively reduce the probability of an issue.

Instead of blindly applying the config to the first module whose type match, it could count how many modules of the same type exists in each module list (the saved and instance ones), and load the nth node into the nth module of the same type. This wouldn't work in case the config change is removing or adding a module that exist/existed in multiple occurences, but at least this will cover all cases of another module type being responsible for the indexing mismatch.

A further refinement to cover that last case could be to Save() the part modules, and to compare the resulting ConfigNodes to the saved ones to determine the index of the added/removed module. Wouldn't be 100% fail-proof, but would further reduce a mismatch probability.

The difficulty for implementing this is that we don't have access to the saved module nodes list from Part.LoadModule(ConfigNode node, ref int moduleIndex), so the logic would have to be moved further up the call stack, in
ShipConstruct.LoadShip() and ProtoPartModuleSnapshot.ConfigurePart().

Moreover, modules in a ShipConstruct part node aren't grouped in a subnode, they are put side by side with all other part nodes (resources, actions, etc...), which make inserting/removing module nodes a bit awkward.

MapObject icons appear in flight view

image

This is why I was calling OrbitRenderer.LateUpdate manually - it needs to go through once to set the MapObjects visibility properly. Might be able to make this patch a little more complex to account for that?

fix processing an experiment in the lab and sending to KSC in the same time

Now it's possible to start process experiment in the lab, and sent to KSC at the same time.
Also it is (probably) possible to processing same experiment in different labs and having unlimited science points

fix:
start processing an experiment in the lab makes the experiment be obtained - so no more science from lab or recovering in the KSC.

Separate "App icon scale" setting

The "App Scale" setting is scaling both the toolbar icons and stock apps GUI (stock alarm app, etc).
Scaling only App icon could be helpful, for fitting more icons on the right side of the screen at ones, but without making all app GUI smaller.

Performance issue: ProgressTracking.Update seems to be O(bodies * vessels)

In my "twitch plays KSP" stream I have a game save with galaxies unbound + lots of other planet packs (320 bodies in total) and LOTS of kerbals. It was running slow so I decided to attach a profiler and was astonished to see this:
image

A brand new save in the same universe has ProgressTracking.Update at about 5ms, which is still astonishingly high but is likely related to the number of bodies.

I haven't looked into what this function is doing, but it would be awesome to optimize this.

ModuleDockingPort issues

As of 1.12.3, docking ports are still a mess :

  • Using the rotation feature cause coordinate drift of children parts
  • The "Rotation Locked" toggle doesn't prevent to set the rotation slider, which lead the internal logic being half executed. The rotation is applied to the the joint but orgPos/orgRot aren't updated, leading to the weird stuff happening when the vessel is reloaded
  • The "Rotation Locked" toggle can be in different states on each docking port, which also lead to weird stuff happening
  • Nullrefs in ModuleDockingNode.get_VisualTargetAngle() on scene load or part unpack
  • The docking port moving part saved rotation sometimes seem to be applied after the joints are created, causing a "torque kick" which can lead to the joint breaking or phantom forces on load (unclear what is happening exactly, but there is definitely something weird).

NullReferenceException caused by Robotic Parts

Not 100% sure this isn't caused by another mod, but my console is flooded with NullReferenceExceptions and the log points to KSPCommunityFixes.

[EXC 10:33:34.877] NullReferenceException: Object reference not set to an instance of an object
KSPCommunityFixes.BugFixes.RoboticsDrift+ServoInfo.PristineCoordsUpdate ()
KSPCommunityFixes.BugFixes.RoboticsDrift.BaseServo_RecurseCoordUpdate_Prefix (Expansions.Serenity.BaseServo __instance, Part p, UnityEngine.ConfigurableJoint ___servoJoint, UnityEngine.GameObject ___movingPartObject)
Expansions.Serenity.BaseServo.Expansions.Serenity.BaseServo.RecurseCoordUpdate_Patch1(Expansions.Serenity.BaseServo,Part,Part)
Expansions.Serenity.BaseServo.ApplyCoordsUpdate()
Expansions.Serenity.BaseServo.Update()
UnityEngine.DebugLogHandler:LogException(Exception, Object)
ModuleManager.UnityLogHandle.InterceptLogHandler:LogException(Exception, Object)
UnityEngine.Debug:CallOverridenDebugHandler(Exception, Object)

Originally posted by @Lucaspec72 in #13 (comment)

PQS.sphere.sx structure is non-linear

The bug can be summarized as follows:

PQS.sphere.sx is supposed to be a variable of linear structure spanning from 0 to 1 indicating a longitude point (similar to how PQS.sphere.sy functions properly for latitude). It does not. Range 0-0.75 behaves normally, but the final quadrant of longitude is in fact migrated nonlinearly into the -0.25 to 0 part of the value. This causes obvious issues, and the root cause is not only diffilcult to diagnose, but if truly corrected at the source probably would break other game facilities that have long worked around this bug.

The only known bug from this is that Kopernicus LongitudeRange values ignore the final quadrant, as a direct result of this nonlinear value representation. A patch can be made to fix this in PQSLandControl. In that class, there is this line:

this.vLon = this.sphere.sx + (double)this.longitudeBlend * this.longitudeSimplex.noise(data.directionFromCenter);

I propose that line be replaced with:

double correctedSphereSx; if (this.sphere.sx > 0.0) { correctedSphereSx = this.sphere.sx; } else { correctedSphereSx = this.sphere.sx + 1.0; } this.vLon = correctedSphereSx + (double)this.longitudeBlend * this.longitudeSimplex.noise(data.directionFromCenter);

That should work around the nonlinearity in the one known symptomatic instance, while not mucking about with any stock systems that may already work around the core bug. Feel free to provide your thoughts.

Vessels sorting in the Launchpad/Runway GUI by "last changed"

When there is long going career and >100 saved vessels, there is a problem with finding the vessel you want to launch in the Launchpad GUI, so people don't use it.
Sorting by "last changed" will make it useful, because usually you want to launch vessel, that you was editing lately in the VAB/SPH.

Asteroid/Comet mass/resources reverting to default after mining them to exhaustion

This can be easily reproduced by :

  • attaching a mining setup to a brand new asteroid and using a drain valve to throw the ore away.
  • engaging mining
  • timewarping until asteroid depletion
  • quicksaving-quickreloading
  • the asteroid/comet is now back to full mass and 100% resources

The key for a consistent repro is to timewarp relatively fast, inducing a relatively large per-timestep amount of mass removed.

This seems to happen because of the very weird logic used to keep track of asteroid mass. It seems that during the last update step where there is still something to mine, if the per-timestep mined amount exceed some threshold, it leaves the data in persisted state that will be interpreted as "that asteroid part was just discovered, initialize its mass/resources" on the next load.

KCPCF breaking Infernal Robotics - Next PAW float edit entries

I'm getting log spam while a PAW for any IR robotic part is open, in the editor or flight with KSPCF:

ItemPrefab for control type 'UI_FloatEditEx' not found.

Tested with only IR installed, and the float edit fields show up as normal. Eg Force, Acceleration, Max Speed, and Min and Max limits. Then added in only KSPCF, and the float edit entries stop showing and the errors start.

Log in case it's useful: https://gist.github.com/Rodg88/a689bf8f53f445238d666bbfcb196e2c

Robotics Drift

Robotics Drift doesn't seem to have been completely fixed: Not sure what is causing it, but while some parts seem to not be drifting, some parts still are ( attached image). The only difference I can think of between the ones that are drifting and the ones that are not is that I believe the ones that are drifting were built in EVA.
image

`PartIcon` fixes and customization

Basically reproduce what this old mod does :

  • Fix bounds calcs for getting icon size (doesn't account for SkinnedMeshRenderers correctly)
  • Allow defining a custom scale factor in part config
  • Allow defining a custom rotation pivot transform in part config
  • Allow defining a custom default model orientation in part config

Could be implemented as a PartLoader.CreatePartIcon() replacement.

Mark auto-saved ship in launchpad launch dialog

Problem:
When launching a craft in the launchpad/runway launch dialog, there are sometimes two crafts with the same name. One of them is an auto-saved craft and one is manually saved, but there is no way to tell which is which. Often they differ in subtle ways, such as staging, and selecting the wrong one might result in a failure.
launchpad

Solution:
Mark an auto-saved ship in the launchpad/runway launch dialog with a text that says "[Auto-saved ship]". This is already done in the VAB load dialog (see the first and last crafts):
vab

PartDatabase.cfg resetting on launch (stock bug?)

Every time I launch KSP, PartDatabase.cfg gets deleted early on the in launch cycle (just after the white screen ends and the loading screens start), causing it to regenerate all part's drag cubes, adding about 5 minutes to my load times.

[LOG 19:27:09.237] PartLoader: Part 'Squad/Parts/Utility/radialAttachmentPoint/radialAttachmentPoint/stackPoint1' has no database record. Creating.
[LOG 19:27:09.237] [DragCubeSystem]: Drag cubes not found or cannot be read for part Part. Generating New drag cubes.
[LOG 19:27:09.238] DragCubeSystem: Creating drag cubes for part 'stackPoint1'

the ksp.log https://gist.github.com/Rodg88/bbdf79d6493f6a6f978553f403e4b7e0
the part database generated during that log https://gist.github.com/Rodg88/5f37a476c03bd23b2e268733de8baee0

Vessel deformation after decoupling events due to autostruts

Steps to reproduce :

  • In the editor :
    • Have a craft which is subject to some visible in-physics deformation
    • Setup autostruts in a way the ensure there are extra joints where deformation will happen
    • Have a decoupler anywhere
  • In the flight scene :
    • Engage the decoupler while there is some visible in physics deformation due to an external force (gravity...)
  • The deformation is now more "stable" (ie, the vessel doesn't return to the original state if the external force is removed)

Extra info :

  • After reproducing, the deformation disappear by engaging/disengaging timewarp
  • After reproducing, the deformation disappear if the vessel is saved and reloaded (F5+F9)
  • After reproducing, the deformation disappear if autostruts are disabled manually in every part PAW.
  • Credit to @Lisias for stumbling on this first, see net-lisias-ksp/KSP-Recall#27

Root cause :

This is caused by the Part.VesselModified() callback for GameEvents.onVesselWasModified calling Part.CycleAutoStrut() for all parts on the vessel.

Cycling autostruts mean all autostruts joints are immediately destroyed, and a coroutine is set to rebuild them in the next FixedUpdate.
This mean all autostruts joints on the vessel will be created anew, using the current in-physics part positions, essentially "freezing" any current deformation in place.

Note that GameEvents.onVesselWasModified is called by other things, meaning that issue can also happen in a variety of other cases : part destruction, docking/undocking, jettisoning fairings, breaking antennas/solar panels, and likely other cases from various mods.

Conclusion and fix

While this is a relatively minor issue (since the deformation is temporary and reversible), this might be a major root cause of the perceived quirkiness of autostruts. Autostruts joints essentially "freeze" the parts in a non-pristine position, and beside the vessel deformation issue, this mean autostrut joints are fighting against the regular joints, leading to more instability in the overall physic/rigidbody simulation. To some extent, it is also an aggravating factor for BG robotics drift (see #13) since robotics override the original part positions based on the current in-physics deformation.

The implemented fix runs just after the autostrut joint (re)creation logic and :

  • Alter the joint anchor to target the original pristine part position.
  • Add a rotation offset to the joint, compensating the difference between the part current in-physics rotation and the original pristine rotation.

Fix shortcomings

The patch doesn't handle the case of autostruts targeting BG robotic parts, as in that case the joint target is the moving rigidbody for which a pristine pos/rot doesn't exists. These autostruts are untouched and follow stock behavior.
The patch should be relatively safe from a modding ecosystem perspective, but it might fundamentally conflict with other robotic implementation such as USI Construction or Infernal Robotics, some testing is needed

Reproduction pics :

image

Robotics drift

Issue summary

When using a Breaking Grounds DLC robotic part, all child parts are affected by non-recoverable position/rotation drift.
Specifically, child parts of robotic parts have their "pristine" (as defined in the editor) coordinates updated to the current in-physics coordinates, which can differ significantly when the vessel is under stress due to forces like gravity, atmospheric drag, engine and RCS thrust, etc.

  • This mean every time the game re-apply the saved "not-so-pristine-anymore" coordinates (ie, when timewarping or unloading the vessel), the original parts position/rotation are definitely lost and the vessel is permanently deformed.
  • The deformation gets worse over time, as every time the vessel goes in and out of physics, the in-physics forces apply a new deformation on top of initial one.

For the sake of clarity, two videos comparing the behavior of children parts of robotic parts and demonstrating the simplest way to reproduce that issue.

Expected behavior when timewarping (no robotic part)
The physics constraint is reverted and the parts "snap" back to their pristine positions :

2022-03-10.01-07-42-604.mp4

Robotics behavior when timewarping
The physics constraint doesn't revert, and the resulting deformation is now permanent :

2022-03-10.01-12-41-605.mp4

Root cause

This is caused by BaseServo.ApplyCoordsUpdate() calling recursively the Part.UpdateOrgPosAndRot() method on all children parts. This method update the Part.orgPos and Part.orgRot fields (which are the "pristine" persisted part coordinates relative to the vessel root part) according the the current Part.transform.position and Part.transform.rotation values.

BaseServo.ApplyCoordsUpdate() is called continuously while a servo is moving, and on various occasions during the start / load / save events.

Implemented fix

The current fix override the recursive call done in BaseServo.ApplyCoordsUpdate() to implement a custom logic. Instead of updating Part.orgPos and Part.orgRot based on the current in-physics part coordinates, it computes the servo induced translation/rotation offset, and apply that offset recursively to the current Part.orgPos and Part.orgRot values.

Shortcomings and potential issues

  • The patch currently doesn't cover the other coordinate update done in BaseServo.RecurseAttachNodeUpdate(). I haven't investigated as the current solution is satisfactory, and that code is probably fine as a result of the patch restoring pristine moving part coordinates prior to that method being called.
  • The "pristine" coordinates we are computing might get slowly out of sync with the in-physics ones due to compound FP precision inconsistencies between our computations and the Unity ones. This being said, stress testing show part positions are visually stable after several hours of continuous servo operation.
  • Computing the pristine coordinates adds some computational overhead when servos are in motion, but this is relatively insignificant compared to what those modules do already.
  • This is a behavior change that can have an adverse impact in some gameplay scenarios, especially while landed and under significant gravity. The stock "issue" also mean that all stress from gravity is "released" on a save/load or timewarp cycle. You get a deformed vessel, but it is now closer to being "at rest", which can help avoiding kraken events on scene load or when getting out of timewarp.

Texture loader optimizations

Possible optimizations :

  • Make @thumb textures non-readable by patching the Texture2D.Apply() call in DatabaseLoaderTexture_PNG with the makeNoLongerReadable argument set to true. Those textures are the generated cargo part thumbnails and it can be reasonably expected that nobody will ever attempt a GetPixels() on them. This should save something like 50 Mb of memory usage in a stock game, and a lot more in modded games.
  • Reimplement the List<TextureInfo> GameDatabase.databaseTexture as a dictionary and replace the GameDatabase.*Texture*() methods to use it. That list can get quite huge, and the loader is performing a lot of iterations on it to find a specific texture by name. This would likely also benefit for everything else using those methods.
  • Implement a "converted textures cache" that save the converted to DDS PNG/JPG textures on disk, instead of re-compressing them at every launch.

1.4.0 VAB exception

One time in the log, right-clicking parts does not show PAW anymore.

NullReferenceException: Object reference not set to an instance of an object
  at KSPCommunityFixes.UIPartActionNumericFloatEdit.Setup (UIPartActionWindow window, Part part, PartModule partModule, UI_Scene scene, UI_Control control, BaseField field) [0x0001c] in <8f81c7b736764f37a0edca532f4ee0dc>:0 
  at UIPartActionWindow.AddFieldControl (BaseField field, Part part, PartModule module) [0x00073] in <cd473063d3a2482f8d93d388d0c95035>:0 
  at UIPartActionWindow.CreatePartList (System.Boolean clearFirst) [0x002bc] in <cd473063d3a2482f8d93d388d0c95035>:0 
  at UIPartActionWindow.Setup (Part part, UIPartActionWindow+DisplayType type, UI_Scene scene) [0x0006a] in <cd473063d3a2482f8d93d388d0c95035>:0 
  at UIPartActionController.CreatePartUI (Part part, UIPartActionWindow+DisplayType type, UI_Scene scene) [0x000f7] in <cd473063d3a2482f8d93d388d0c95035>:0 
  at UIPartActionController.SelectPart (Part part, System.Boolean allowMultiple, System.Boolean overrideSymmetry) [0x001a7] in <cd473063d3a2482f8d93d388d0c95035>:0 
  at UIPartActionController.HandleMouseClick (UnityEngine.Camera cam, System.Boolean allowMultiple) [0x0003f] in <cd473063d3a2482f8d93d388d0c95035>:0 
  at UIPartActionController+<MouseClickCoroutine>d__39.MoveNext () [0x00082] in <cd473063d3a2482f8d93d388d0c95035>:0 
  at UnityEngine.SetupCoroutine.InvokeMoveNext (System.Collections.IEnumerator enumerator, System.IntPtr returnValueAddress) [0x00026] in <12e76cd50cc64cf19e759e981cb725af>:0 
UnityEngine.DebugLogHandler:Internal_LogException(Exception, Object)
UnityEngine.DebugLogHandler:LogException(Exception, Object)
ModuleManager.UnityLogHandle.InterceptLogHandler:LogException(Exception, Object)
UnityEngine.Logger:LogException(Exception, Object)
UnityEngine.Debug:CallOverridenDebugHandler(Exception, Object)

Leaving VAB and reentering fixed the issue, but... it was reproducable with that part, right-clicking it:
SmartParts/Parts/Smart-Controller/km_smart_srb.cfg

Log and stuff:
KSP logs 2021-11-20-01.zip

highlight specified AG in the "Action Sets" window

highlight specified AG in the "Action Sets" window.

  • show only active set is probably enough:
    Custom01, Custom02, Custom03

    C01, C02, C03
    
  • or also show all sets of the "Activate Action Sets" feature:

    Default:
    C01, C02, C03
    Case 1:
    C01, C03, C05
    
  • or also try to detect on runtime the most popular Custom AG: Solar, Antennas, Drills, Termal, ISRU, and name them:

    C01 - Solar
    C02
    C03 - ISRU
    

Max physics delta time and scene load kraken

While it is true that Unity physics stability isn't affected by the max physics delta time, there is a purely KSP related issue that can occasionally result in vessels being torn apart on scene loads when the max delta time is increased.

I never managed to identify the precise cause, but it likely related to KSP using an IEnumerator implementation of the Part.Start() method (could also be caused by another coroutine, there are quite a few used in various subsytems/partmodules initialization methods).

When you increase the max delta time, this allow Unity to put many more FixedUpdates between each Update(). This will be especially true on scene loads, when your CPU is overloaded by all the initialization code.

Since those IEnumerators are tied to the Update() cycle, this mean that many more physics updates (FixedUpdate) will happen before things are fully initialized.

There are checks in place that prevent physics and all the related stuff (FlightIntegrator, floating origin, etc) to really kick in before everything is fully initialized (the "physics easing" thing), but there is likely something they missed, somewhere.

It would have been much more safe for them to implement the IEnumerators/Coroutines in the FixedUpdate() cycle (ie, doing yield return new WaitForFixedUpdate() instead of yield return null)

This is all theoretical, I never really took the time to verify it, but increasing the max delta time definitely increase the likeliness of Kraken events on scene loads, and that is a very likely root cause.

TODO

  • Try to find a reproducible case (large part count vessel ? heavily modded install ? autostruts ?)
  • Create flight scene addon that temporarily force the max physics delta time to 0.02 for the 4-5 first frames on scene load

Scaled robotic parts propagate their scale onto child parts in the editor

Reproduction steps :

  1. Add this MM patch :
@PART[hinge_01]
{
	@rescaleFactor = 2
}
  1. Launch the game, go to the VAB
  2. Place a "G11 Hinge"
  3. Attach any part to it
  4. Move the hinge target angle in the PAW

This results in the child part(s) suddenly being scaled 200%.
This is an editor-only, mainly cosmetic issue, but quite annoying nevertheless.

Cause

This happen because when moving a servo part in the editor, this re-parent the child part(s) to the servo "moving" object, which is inside the servo part model hierarchy.
Meaning if the servo part model is scaled 2x, the child now needs to be scaled 0.5x to keep it's original scale.

But the KSP code explicitly force the scale to 1x after reparenting (I guess because the parts root transform scale isn't supposed to be anything other than 1x).

See BaseServo.SetChildParentTransform().

Fix

Removing the "force scale to 1x" bit on re-parenting seems to work with no immediate side effect, allowing non-1x scale robotic parts to be manipulated in the editor. I guess this works correctly because KSP used to mess around with scale for mirror symmetry, so there is some "revert back to identity scale on part disconnect" code lying around.

It does introduce a breaking change to an unspoken stock convention. But I wouldn't expect anything to rely on parts root transform scale being always 1x, especially in the editor (famous last words).

KSP memory leaks

KSP is leaking managed memory from various places, but mainly due to GameEvents delegates that are never removed.

Worst offender by far is the PartSet class which is holding Part references, is instantiated (massively) and owned by a bunch of classes (ShipConstruct, Vessel, Part...) and is subscribing to 2 GameEvents. It attempts to unsubscribe from those GameEvents with a GC finalizer, which obviously doesn't work since the GameEvents subscriptions are preventing those objects from being collected in the first place.

There are many other offenders, with the end result being that all Vessel or Part ever instantiated will actually never be collected. Since those are rather top level objects themself holding a rather large reference tree, the end result is an ever ballooning heap allocated memory. On average, for a stock game (figures will likely be much higher with mods), every part ever instantiated will cause a permanent leak of ~0.2MB.

As shown in the following table, quick-loading the same flight scene with a ~150 parts vessel causes a leak of about ~30 MB per scene switch. After 17 scene switches, about 450 MB of memory is forever lost.

Scene load # GameEvents callbacks (Stock) GameEvents callbacks (KSPCF) Allocated heap (MB) (Stock) Allocated heap (MB) (KSPCF) Leaks (MB) (Stock) Leaks (MB) (KSPCF)
1 3142 1020 742,0 783,5
2 5268 1024 805,1 856,4
3 7394 1046 834,2 878,8 29,1 22,3
4 9520 1032 850,5 882,9 16,3 4,1
5 11646 1036 877,8 874,6 27,3 -8,2
6 13772 1040 906,5 852,6 28,7 -22,1
7 15898 1044 933,7 858,9 27,2 6,3
8 18024 1048 966,9 849,7 33,2 -9,2
9 20150 1052 1012,4 880,8 45,6 31,2
10 22276 1056 1090,6 850,4 78,1 -30,4
11 24402 1060 1121,3 810,7 30,7 -39,8
12 26528 1064 1150,0 827,2 28,7 16,6
13 28654 1068 1179,6 849,1 29,7 21,8
14 30780 1072 1212,4 891,1 32,8 42,1
15 32906 1076 1239,0 850,4 26,6 -40,7
16 35032 1080 1270,8 849,4 31,7 -1,0
17 37158 1084 1301,5 850,0 30,7 0,6

Note that aside from increasing memory usage, this also cause performance degradation :

  • Heap will become more fragmented and GC collections will take longer.
  • "Dead" GameEvents delegate are still called, slowing down event processing and potentially causing weird side issues.

The KSPCF MemoryLeaks patch add a callback when a scene is exited, and walks through all GameEvents delegates to remove entries whose owner is a destroyed UnityEngine.Object derivative. There are just to many cases of leaked GameEvents, so instead of trying to fix every one of them, this is a cheap way to work around the whole issue. Moreover, this will also cover leaks coming from plugins (which I suspect are even worse offenders than stock).

A specific handling is added for the PartSet GameEvents leaks, since the class doesn't derive from UnityEngine.Object.

The patch also implement various concrete fixes for a bunch of non-GameEvents related leaks, mainly caused by static fields misuse and lazyness.

Bugfix - Locked autostruts on landing gears

Landing gears and wheels using stock module have their autostrut settings locked to "enabled" and "to most massive part". It's not safe in some situations(mostly docking of spaceplanes with big stuff) and may even cause krakens with legs being displaced on rocket spawn. It seems to be hardcoded, can't fix it through configs.

Vessel effects audio have a pos/rot offset in KSP 1.12

This causes random audio loss and L/R channel weirdness.

Issue uncovered by @ensou04
See forum post

The issue seems to be in the FlightCamera.EnableCamera() and FlightCamera.DisableCamera() methods.

They are re-parenting the AudioListenerGameObject using the Transform.SetParent(Transform parent) overload that set the worldPositionStays parameter to true, which will induce a local position/rotation offset when reparenting.

Patching those methods to use the Transform.SetParent(Transform parent, bool worldPositionStays) overload with that last parameter set to false should prevent any offset to ever be introduced.

Test build with a fix :
KSPCommunityFixes_1.15.0_PRERELEASE.zip

PAW groups for stock entries

Add stock PAW items that don't already belong to groups, and allow to customize the default collapsed/extended state of groups in the KSP settings.

ModuleDockingNode error on vessel unpacking

[LOG 13:40:18.382] Unpacking Transfer Station
[EXC 13:40:18.383] NullReferenceException: Object reference not set to an instance of an object
    ModuleDockingNode.get_VisualTargetAngle () (at <39c0323fb6b449a4aaf3465c00ed3c8d>:0)
    ModuleDockingNode.OnPartUnpack () (at <39c0323fb6b449a4aaf3465c00ed3c8d>:0)
    UnityEngine.DebugLogHandler:LogException(Exception, Object)
    ModuleManager.UnityLogHandle.InterceptLogHandler:LogException(Exception, Object)
    UnityEngine.Component:SendMessage(String, SendMessageOptions)
    Part:Unpack()
    Vessel:GoOffRails()
    OrbitPhysicsManager:LateUpdate()

Reported by @flart on the forum thread, KSPCF version at the time of posting : 1.9.1
Might not be related to KSPCF at all, I think I saw that error too in a stock game, but worth investigating.
Seems harmless anyway.

Performance issue: OrbitRendererBase.LateUpdate is expensive in games with lots of bodies and vessels

This is hitting about 6ms in my profile captures. I have about 1000 objects with orbits (mostly asteroids, but this isn't a crazy number if you have a lot of debris or are playing Galaxies Unbound). This code skips the heavyweight work when the map mode isn't open, but really we should just disable the updates on all these objects until the map or tracking station is opened.

(I'm working on this one)

Stress deforms robotic parts

In the editor, one arm is loaded with weight:
20220523195249_1
let it wobble around for a bit:
20220523195316_1
Cheat to orbit:
20220523195337_1
Time warp, you can see the base of the 'heavy' arm is shifted:
20220523195353_1
and continues to be shifted in warp without the weight attached anymore:
20220523195426_1

Launchsites menu longer in the VAB/SPH

Based on how many launchsites in the list, make the list expand, for example up until 10 lines.
Probably the same applied to the stock save folder list in the VAB/SPH

Add name of the vessel to the title of an alarm

By default when creating new Ap/Pe, Change SOI, maneuver alarms, the title just have "Maneuver", etc,
while KAC conveniently added a name of a vessel to the title of the alarm "Maneuver for My Craft 1"

Proposal for some .cfg based fixes

Some fixes for your consideration:

Missing stock agencies: there are 2 agencies in the base game without AGENCY nodes or logos (Light Year Tire Company + Stratus Corporation). These agencies are listed as the manufacturers of several parts each. When part test contracts are offered the system seems to prefer using the manufacturer as the agency providing the contract and with these parts it's unable to do so. A while ago I contacted the author of Kerbalized Flags who provided some great logos for them and gave permission to do whatever with them.

LightYear256

Stratus256

Another widely mentioned agency is Clamp-O-Tron, it's not listed as a manufacturer (aside from the radial attachment port being found behind their factory) but I think everyone is familiar with their docking ports.

ClampOTron256

When trying to complete all agency and manufacturer fields I found one was missing for the parachutes, so I asked the logo author if they wanted to make another one and they came up with Free Fall Parachutes. This one would be the most out of scope since it's not mentioned anywhere in the base game but it's still worth including here.

freefall256

All these are included in a mod I'm working on (linked above) but I think they'd be more suited to this project, particularly with the logos being community-sourced.

Inconsistent/missing part manufacturers: Several parts have inconsistent and missing manufacturer values, this is an easy fix and if using the added agencies above almost every part's manufacturer can be a named agency.

There's room to debate who should manufacture the Flea and Hammer boosters, the winglet and the service bays so my choices there could be changed.

Inaccurate bulkhead profiles: A lot of parts have inaccurate bulkhead profile values (mostly surface attach parts), also mk0 and mk1 profiles are missing if these are considered to be separate from size0 and size1. The linked file contains fixes for these. The mk0 and mk1 fixes may be unsuitable given that the base game's only use for this field seems to be the editor sorting list, and mk0 + mk1 profiles would be missing icons, so you may only want the surface attach fixes here.

Let me know if you want any of these and I can do up a pull request and remove them from my mod.

Random error on craft launch, results in corrupted game state and craft file

From forum user @Rakete
KSPCF version at the time of posting : 1.9.1

I think i might have found a nasty sideeffect. Since I installed the community fix pack sometimes KSP eats my craftfiles. I usually save my builds before launching testwise. Since I installed the fixes, sometimes the game refuses to load the flightscene foe launch and throws me back to the KSC-outside. Because I have saved the craft before launch it is as messed up as the autosaved vehicle and refuses to show the staging (shows its elements horizontally in a green box starting with a + symbol...). No interactions with the displayed craft in the vab are possible, so that i can't ve repaired. It doesn't even close the load window, after loading a messed up craft file. Something in the crafteditor seems not to work. I could not reproduce it. Sometimes it happens if I use one of my standardlifters and remove two of four boosters. Hadn't had it after uninstalling the community fixes.

Unfortunately i have no log at hand, when this happens, as it happens random for me. Also i have no messed up craft at hand, cause I deleted it in a fury. :-) unfortunately... I ate a craft i was working on 5 hours. My fault not to save different itterations ร—D. It drove me nuts today.

Gonna need more info on that one... Not much I can do without at least a log file...

Engine plate decouplers cause the top-attached part to be shielded from airstream

ModuleDecouple on engine plates is setting all attached parts as "shielded from airstream", excepted the part attached to the node defined in the bottomNodeName of the ModuleJettison module.

The intent is to disable drag (as well as some interactions like deploying animated parts) on all other node-attached parts, but "all other node-attached parts" will also typically include the part attached to the engine plate top node, which is clearly an unwanted oversight.

Fundamentally, ModuleDecouple is missing a way to define exclusions or inclusions for airstream-shielded nodes in its config. Implementing such a config option is difficult and would require concrete patches for every stock and modded part using isEnginePlate = true.

Alternatively, we can use some heuristics to determine which nodes should be excluded. For all stock engine plates, excluding nodes with id = "top" would work, but there is no guarantee this would hold true with modded parts.

Another heuristic would be to exclude all nodes whose orientation is significantly different than the node identified by the bottomNodeName of the ModuleJettison module. This might fail in case of a complex modded part setup, but as of writing a quick github search show no potential issue.

RoboticsDrift doesn't work with model hierarchies having a position/rotation offset

All the stock robotics parts have a model hierarchy where the movingTransform (defined by servoTransformName in configs) has a zero position and rotation relative to the model root.

But this doesn't necessarily hold true for modded parts. The model hierarchy can have local pos/rot offsets, and the model itself can be translated/rotated relative to the part transform by defining a non-zero position/rotation in the part config MODEL{} node.

Failing to handle that case causes the orgRot/orgPos update to produce garbage results, resulting in wrong part positions when timewarping and after reloading a saved vessel. This can be seen in the BDB Hokulani OCO-RT90 Truss Structure. That part is victim of both issues, as it uses a MODEL{} rotation and has local position offsets between the model root and the movingTransform.

We need to fix things in 2 separate places. The position offset should likely be used when calling RecurseChildCoordsUpdate() the first time :

RecurseChildCoordsUpdate(part, Vector3d.zero);

The rotation offset should likely be accounted for at the GetLocalToVesselSpace() level :
protected QuaternionD GetLocalToVesselSpace()
{
// for some reason not normalizing can end up with a quaternion with near infinity components
// when it should be identity, leading to infinity and NaN down the line...
return (QuaternionD.Inverse(servo.part.orgRot) * (QuaternionD)servo.part.vessel.rootPart.orgRot).Normalize();
}

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.