Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing TweakScale patches #71

Merged
merged 1 commit into from Jun 17, 2019
Merged

Fixing TweakScale patches #71

merged 1 commit into from Jun 17, 2019

Conversation

Lisias
Copy link

@Lisias Lisias commented Jun 10, 2019

Fixing TweakScale patches to prevent triggering the Fatal Alert from #34.

This was reported on Forum

There're more patches being applied with wildcards, but they are not borking anything for now. I opted to not touch what's not broken yet.

This also fixes the Osaul Radial Cockpit (SXTOsualRadCockpit) that got terribly small due a patch. Now the cockpit is size compatible with the other Osaul cockpits.

Screen Shot 2019-06-10 at 00 21 49

@ncanceill
Copy link

While you are at it:

  • The patch for the SXTInlineAirIntake here is duplicated here
  • The patch for the SXTtruckbox here is duplicated by the wildcard patch here

@ncanceill
Copy link

Did you update the PR since my comment? or had you already fixed the inline intake and truck box? (in which case, sorry, I need to learn how to read)

Anyhow, this now fixes everything for me except the three seaplane floats

Moreover, I added percent signs everywhere to avoid duplication, so my patch now reads:

@PART[SXTfloat*]:NEEDS[TweakScale] // 3 parts
{
    %MODULE[TweakScale]
    {
        %type = free
    }
}

However, the three float parts still display two scaling gauges in the PAW and trigger NullRefs in TweakScale (see my comment in TweakScale #41)

@Lisias can you reproduce this? According to my ModuleManager log, SXT/Patches/ModCompatibility/SXT_TweakScale/@PART[SXTfloat*]:NEEDS[TweakScale] is the only patch applied to these parts, and I cannot find anything that would duplicate the TweakScale module...

@linuxgurugamer
Copy link
Owner

I'm going to wait until this is settled before merging

@linuxgurugamer
Copy link
Owner

I appreciate all the work you guys are doing. This is an old mod with an old patch, which I don't have time to look at right now. Please make sure I know everyone who contributed to this update, both testing and actually changing the code

@Lisias
Copy link
Author

Lisias commented Jun 11, 2019

Okie dokie. I will check these fixes on my side, and apply a new pull request.

In the mean time I would advise against using the % on widlcards. This will make the problem impossible to detect. We are talking about unintentional double patching, and this will corrupt the savegames and crafts the same way, but this time in a way that TweakScale cannot detect.

It's better to fix the problem than to patch the symptoms!

@Lisias
Copy link
Author

Lisias commented Jun 11, 2019

@ncanceill
Copy link

@Lisias thanks for the explanation

I hope we can track down the problem with the seaplane floats

One last thing about the comment you added about the radial cockpit needing type surface instead of type stack: I agree, but why keep it commented? would the changer break existing craft or something?

@ncanceill
Copy link

I confirm that the issue with the seaplane floats is not directly due to double patching, because removing the @PART[SXTfloat*]:NEEDS[TweakScale] patch completely remove the Tweakscale module from the part

Moreover, the issue is not due to the wildcard, because it persists when using @PART[SXTfloatFront,SXTfloatMid,SXTfloatOutboard]:NEEDS[TweakScale] to patch instead of the wildcard

So this is likely an issue related to the FireSpitter buoyancy module, either in ModuleManger or in Tweakscale, maybe related to this

I would advise on replacing the other wildcards in this file with the actual part names before merging:

  • SXTfloat*-> SXTfloatFront,SXTfloatMid,SXTfloatOutboard
  • SXT5mTank0* -> SXT5mTank0,SXT5mTank0cap
  • SXTelevon* -> SXTelevonSmall,SXTelevonSmallHalf,SXTelevonLarge,SXTelevonVeryLarge
  • SXTAirbrake* -> SXTAirbrake,SXTAirbrakeLarge,SXTAirbrakeLarger
  • SXTengineattachment* -> SXTengineattachment,SXTengineattachment2
  • SXTmk2* -> SXTMk2LinearAerospike,SXTmk2cargoadapter,SXTmk2SAScore,SXTmk2225degree,SXTmk2adaptorIntake
  • SXTAJ10* -> SXTAJ10,SXTAJ10Mid

@Lisias
Copy link
Author

Lisias commented Jun 17, 2019

The problem with getting rid of all wildcard is the need to manually update the TweakScale patch every time you add a new part.

This is a maintenance decision, and is unrelated to the problem at hand - so my reluctance on adding them to this Pull Request, what is exclusively about the TweakScale/TweakScale#49

About the FSbuoyancy, it's yet another issue as linked above. Also unrelated to the problem at hand.

It worth to mention that workarounds to allow current savegames to go on are already available:

https://github.com/net-lisias-ksp/TweakScale/blob/dev/orthodox/Extras/TweakScale/BreakingParts/SXT.cfg

@Lisias
Copy link
Author

Lisias commented Jun 17, 2019

Unless any other issue arise, I think this pull request is good for merging.

@ncanceill
Copy link

@Lisias I concur!

@linuxgurugamer please merge this at your convenience

In my opinion, the risk of wildcards introducing bugs outweighs the overhead of modifying the patches every now and then, but this decision is not up to me

@linuxgurugamer linuxgurugamer merged commit 1f88b63 into linuxgurugamer:master Jun 17, 2019
@Lisias
Copy link
Author

Lisias commented Jun 25, 2019

I appreciate all the work you guys are doing. This is an old mod with an old patch, which I don't have time to look at right now. Please make sure I know everyone who contributed to this update, both testing and actually changing the code

Last week I was walking over hot coals due some Real Life issues. And that were the good times, I'm seating on them right now. =P

I just forgot to mention the patches here. (sigh) Sorry.

In a way or another, there're a lot of legacy issues that we plain inherited with the Add'Ons. It's just the way it is - people borks. Things goes ugly when these borks goes unnoticed and unfixed.

Well… There're some fewer borks unnoticed nowadays, and they are being fixed. I think we are doing good (besides the bruises).

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.

None yet

3 participants