KSP-RO / ProceduralParts

A continuation of StretchySRBs, which is a continuation of StretchyTanks
71 stars 79 forks source link

Some parts don't scale correctly in career mode under KSP 1.9.x, PP 2.0.6 #267

Closed Lafreakshow closed 4 years ago

Lafreakshow commented 4 years ago

I've noticed this with the separater and the nose cone, both of them are tiny and have very low upper limits.

If I'm unsterstanding correctly, the former should have an upper limit for the diameter of 1.75m with Engineering 101 unlocked but the actual upper limit is somewhere around 0.3m at that point. The latter has it's maximum height limited to 0.001m. Naturally, both parts also have the wrong default size, as it exceeds the limits. In both cases these limits also seem to change arbitrarily by a couple cm when changing the shape of the part, I guess this could be because they have their maximum volume limited as well, which would then also way too low. Nothing unusual to be seen in the KSP or Module Manager logs. Judging from the Module Manager cache, the parts have the correct limits set.

These issues don't happen in sandbox mode. So far I've only tested this with the first couple tech nodes unlocked (including Engineering 101 and stability). I have these issues in my 200+ mod install (including Community Tech Tree) and in a clean install with only PP installed. I will probably test this with a bunch of different combinations of unlocked tech nodes in the coming days and report back with the result.

Note that the Heatshield may have similar issues, it's limits seem too low to me. as well but I haven't looked into that particular part further.

DRVeyl commented 4 years ago

Thanks for identifying this. I posted a portion of the response in SmartParts, as it relates to that. I'll add a little ProcParts-specific thoughts here.

A maximum height of 0.001 is definitely not intended.

The much tighter restrictions on legal sizes in stock [career!] compared to RO likely exacerbate this issue. I had removed the minimum volume enforcement from ProcParts when reworking to create 2.0, to avoid extra complexity, and had determined that min length and min diameter were a sufficient mechanism for it. It was re-added for 2.0.4 for a specific use case, after I was more confident that the code would be reasonably parallel with the max volume limitation. 2.0.5, 2.0.6, and this issue indicate it wasn't the smoothest rollout.

I don't know that stock truly has a use case for minimum volume enforcement. It was related to the idea that you don't unlock the small monoprop tanks until later. A reasonable course might be to just remove the minVolume limitations in the current ProcParts parts -- which should result in the same experience as you've had with 2.0.0-2.0.3.

I have not re-examined the actual limitations defined in the parts; they retained the settings from before I took up 2.0. I'm happy to take feedback on if removing the minVolume settings is most appropriate, compared to perhaps broadening some of the part-specific values.

Lafreakshow commented 4 years ago

I can confirm, 2.0.3 works. SmartTanks is still partially broken but the issues with scaling don't exist and it also doesn't crash. In that Case I'll just downgrade to that in my main install and play with it. From how you described it is sounds like the changes post 2.0.3 aren't all that relevant to me so that works just fine.

Min volume restrictions sound sensible to me. I actually find myself often using tweakscale to essentially build regular sized probes and make them tiny before I unlock the tech for small probe parts and it feels kind of dirty. At the very least I don't think it would be break balance. But it is, of course, somewhat unrealistic so I can definitely see the use for this with RO,.

Sidenote: I almost have the feeling like the value that is supposed to be the minimum volume ends up being used as the maximum volume. I just noticed this:

[LOG 04:07:49.653] [ProceduralParts] OnStart(Editor) for proceduralNoseCone (ProceduralParts.ProceduralPart)
[LOG 04:07:49.654] [ProceduralParts] TechLimits (TechRequired=) diameter=(0.125, 1.5) length=(0.01, 0.625) volume=(0, 0.01)
[LOG 04:07:49.656] [ProceduralAbstractShape] Invoking OnPartVolumeChanged for proceduralNoseCone (Part) volName:Tankage vol:0.000323388434480876
[LOG 04:08:00.639] [ProceduralAbstractShape] OnShapeDimensionChanged: bottomDiameter from 0.1880864 to 0.188
[LOG 04:08:00.639] [ProceduralAbstractShape] Invoking OnPartVolumeChanged for proceduralNoseCone (Part) volName:Tankage vol:0.000323147920425981
[LOG 04:08:00.643] [ProceduralAbstractShape] OnShapeDimensionChanged: length from 0.01409898 to 0.014
[LOG 04:08:00.643] [ProceduralAbstractShape] Invoking OnPartVolumeChanged for proceduralNoseCone (Part) volName:Tankage vol:0.00032087936415337
[LOG 04:08:03.831] [ProceduralAbstractShape] OnShapeDimensionChanged: length from 0.014 to 0.4362684
[LOG 04:08:03.831] [ProceduralAbstractShape] Invoking OnPartVolumeChanged for proceduralNoseCone (Part) volName:Tankage vol:0.00999925099313259
[LOG 04:08:09.900] [ProceduralAbstractShape] OnShapeDimensionChanged: bottomDiameter from 0.188 to 0.1879864
[LOG 04:08:09.900] [ProceduralAbstractShape] Invoking OnPartVolumeChanged for proceduralNoseCone (Part) volName:Tankage vol:0.009998076595366

Note that while the limits for diameter and length are correct, the upper volume limit for the nose cone is indeed, for some reason, 0.01. ModuleManager Cache says the cone has the proper values as defined by it's part.cfg. In the lof you can also see how I randomly changed the size of the cone and the mod (correctly, I assume) limited me to the max volume of 0.01. so I would guess the problem must be in the code, somewhere where the limits get recalculated.

I had a quick look at the code (by which I mean I stared at my screen for two hours instead of going to bed at 2am because I have an appointment at 10 while slowly going mad from flashbacks of higher mathematics in uni) but I am barely able to follow it and utterly unqualified to look for mistakes in the formulas.

Another note: The issue stops after the entire tech tree is researched. However, when I researched only the nodes (and their requirements) needed to get the max limits for the nose cone the issue persisted. I will probably test the other parts later to see if I can get similar logs out of it. This issue doesn't affect all parts so maybe I can find the pattern behind it and narrow it down to a class or so.

DRVeyl commented 4 years ago

Actually, that was extremely helpful.

The volumeMax default value is 0.01 when establishing the tech-based size limits. I expected some tech level, even a start one, to update it. Most parts, this is true. Nosecone, there are no definitions for max volume anywhere. Bad assumption I made when fixing the issue from 2.0.5, when that default value was NaN and bad things happened. Looks like if I had made it 0 instead of 0.01, line 45 of TechLimit.cs would have simply avoided this problem.

As an aside, the changes after 2.0.3 are not very relevant to stock, true. Excepting if there is value in the "min volume" mechanic without just min length/diameter.

RO actually generally just disables all of the size restrictions. RP-1, the career mode, does this as well, opting for some home-brewed mechanics to charge the player based on part sizes. The reason to turn the minVolume enforcement back on ends up more convenience than gameplay -- they want to close off some user-errors when configuring certain parts by ensuring a minimum volume is preserved.

Lafreakshow commented 4 years ago

Interesting. Seems I actually managed to track this bug to its source. Well more like I ponted at the sign and you were able to read it.

I did another round of testing with this new information, This time, I fiddled with the part settings a bit more, most importantly: I set maxVolume to 0 . it did... something...

First off, the decoupler:

[LOG 20:52:49.082] [ProceduralParts] TechLimits (TechRequired=) diameter=(0.01, 1.75) length=(0.2, 0.2) volume=(0, Infinity)
[LOG 20:52:49.082] [ProceduralAbstractShape] Invoking OnPartVolumeChanged for proceduralStackDecoupler (Part) volName:Tankage vol:0.244392931461334
[LOG 20:52:52.779] [ProceduralAbstractShape] OnShapeDimensionChanged: fillet from 0.05 to 0.2
[LOG 20:52:52.779] [ProceduralAbstractShape] Invoking OnPartVolumeChanged for proceduralStackDecoupler (Part) volName:Tankage vol:0.229184523224831
[LOG 20:52:55.115] [ProceduralAbstractShape] OnShapeDimensionChanged: fillet from 0.2 to 0
[LOG 20:52:55.115] [ProceduralAbstractShape] Invoking OnPartVolumeChanged for proceduralStackDecoupler (Part) volName:Tankage vol:0.245436936616898
[LOG 20:52:56.633] [ProceduralAbstractShape] OnShapeDimensionChanged: diameter from 1.25 to 1.75
[LOG 20:52:56.633] [ProceduralAbstractShape] Invoking OnPartVolumeChanged for proceduralStackDecoupler (Part) volName:Tankage vol:0.481056392192841
[LOG 20:52:56.634] [ProceduralAbstractShape] Moved surface attachment node from (0.6, 0.0, 0.0) (local) to (0.9, 0.0, 0.0)
[LOG 20:52:58.635] [ProceduralAbstractShape] OnShapeDimensionChanged: diameter from 1.75 to 1.25

Relevant piece of config:

// Only the diameter is tweakable, not the length or the curve
lengthMin = 0.2
lengthMax = 0.2

diameterMin = 0.0
diameterMax = 0.0

// If you want to be able to tweak the fillet curve, comment this line
allowCurveTweaking = false

TECHLIMIT {
    // As for TR-18A
    name = engineering101
    diameterMin = 1
    diameterMax = 1.75
    volumeMin = 0.0
    volumeMax = 0.0
}

Min and max volume are effectively disabled here and that seems to be how the part is behaving too. However, the minDiameter doesn't seem to have any effect. I can freely make it as small as 1cm. Also note that I can adjust the fillet here which, if I'm understanding the config correctly, shouldn't be allowed. Looks familar eh? So I set MinDiameter to 0 by default to see if maybe acts the same way. But alas, it doesn't. That just reduced the diameter to actually 0. I tested this with the nose cone as well and after sorting out a typo where I accidentally set the minVolume to 1 instead of 0, resulting in the topDiameter of the nose cone being stuck at 1.35m for some reason, I got the same results.

My conclusion here is that the TECHLIMIT blocks don't seem to stick well and this doesn't just affect the parts I reported here but all parts. It just happens to not be that noticeable with the others. It seems to me that a random set of limits is applied to every part. I mean it is consistent over restarts, but it doesn't seem to be the furthest down TECHLIMIT nor the first but some combination of the others. Or something... It's kind of confusing.

I've thought about going through all parts, setting all limits in the TECHLIMITS blocks to easily recognizable values and this way find out which of them stick and which don't and whether there is a pattern in it. Basically find out where exactly the values seen in game come from but since that's a lot of work and I have busy weekend coming up, I'm afraid that will have to wait a while.

So I actually think we're dealing with two seperate bugs in the same section of code. The default values for the volumes being what made a few parts become tiny and some other bug resulting in the TECHLIMITS not being properly applied. I've considered the possibility of Module Manager somehow being involved but according the MM cache, the parts have the correct settings applied to them and there is nothing related to this in the log either.

Outside of testing all the parts like I alluded to a paragraph ago I'm afraid this is all I can do for now. I am a programmer but I've never done anything with C# so gettting into the code is dificult for me and I don't have the tools set up to modify it. However, when you get around to this, I will happily volunteer to repeat all these tests and see if the fix actually fixes it. Don't feel pressured though, 2.0.3 is working perfectly for my stock needs so I'm just rolling with that.

For now, I think I may try out RP1. I never really looked into it before but skimming through the code and some other repos this past week has made me curious. It may also result in me getting into KSP modding in the near future...

DRVeyl commented 4 years ago

The intent of the TECHLIMIT system, I believe, was to start at the base limits defined in the ProceduralPart PartModule (ie, outside the TECHLIMIT struture) and then apply TECHLIMITS as a patch if you have the requisite tech, and the limit expands what you are allowed to do. https://github.com/KSP-RO/ProceduralParts/blob/master/Source/TechLimit.cs#L51-L60 shows the application; the minimum only gets smaller, the maximum larger.

The default value of allowCurveTweaking=true is probably in error and it should be false (since it can be enabled by TECHLIMIT but not disabled).

Long story short, I need to re-examine the TECHLIMIT system and the default configs for the ProcParts parts. Also to require sane values; that Validate() method still looks full of potential holes. Shouldn't take long, just a bit of grunt work.

I don't believe there's any issue with MM.

Check out the Realism Overhaul KSP forum thread, and their Discord server. Welcome to the community. :)

DRVeyl commented 4 years ago

Mind checking out https://github.com/KSP-RO/ProceduralParts/releases/tag/v2.1.0_rc1 ?

I have completely removed the TECHLIMIT system in favor of using stock's PARTUPGRADEs. This tends to work out well since we are only upgrading the limits on configuration fields in the VAB.

Lafreakshow commented 4 years ago

Sounds good I'll try that. Does this imply that upgrads need to be enabled in the dificulty settings of is it independent of that?

DRVeyl commented 4 years ago

I believe that applies only to sandbox. But yes, seems like it needs to be enabled right now for that.

image

Lafreakshow commented 4 years ago

I just tested it and the part upgrades have to be enabled in Career too else the parts are limited to their initial values. Thankfully this can be enabled at any time in a running save so it's not that big of a problem.

Aside from that the issues with the volume limit seem to be gone. It still causes some weird values like a max size that is slightly below what one would expect but I'm pretty sure thats just the system working as intended because the other dimension is already high enough that the volume limit was exceeded.

The one other thing I noticed is that the nosecone and heatshield were behaving weirdly still. Without any additional nodes unlocked, the nosecone had it's top diameter fixed so that it was impossible to make it pointy. Outside of that it worked though. And as soon as I unlocked the first upgrade for the nosecone, the top diameter could be changed all the way to zero.

screenshot1

Though I imagine you didn't exactly focus on these details while making this build so I expected some weirdness and inconsistency, especially considering you swapped out and entire system.

In the end though, the bug that brought me here is no longer there as far as I can tell and this change has the very nice side effect that it is now possible to see in the techtree when a part has it's limited increased. This is has bugged me basically ever since tech based limits became a thing.

I wonder if this change will break existing vessels. If I understand the cfg file synstax correctly, you took care not to overwrite any existing dimension values. In any case I will continue testing in a bit more detail over the next day. I would just throw it into my current save and see if anything explodes but I started relatively recently and don't think I've used any PP parts yet. Non that would be currently in flight anyway.

I just began to wonder, it is probably possible to change the volume limits with a module manager patch and the same should go for disableing the whole update system, which kind of against the whole realism spirit but still, more options are always good. Why else would I have like 100 textures for PP installed when I really only use two per part. Perhaps I get around to doing that for myself sometime. Might even be worth releasing those patches. I don't have a huge problem with the volume limits, in fact I think they are a good altenative to limiting the dimensions. But the fact my tank can't be 1.25m x 1.50m but instead has to be 1.2345m x 1.50m is triggering my need for round numbers.

Chryseus commented 4 years ago

There seems to be another little bug where the textures reset back to the legacy textures when you load a saved ship or enter the VAB, for example this ship was saved with plain metal:

DRVeyl commented 4 years ago

You have Legacy Textures set to enabled. The bug here might be I'm not quite hiding those TexturesUnlimited selectors early enough. Dialing those TU textures won't have an effect if LegacyTextures is enabled. (Similarly, dialing the legacy texture choices won't do anything if it is set to disabled.)

DRVeyl commented 4 years ago

I didn't have to do anything special starting a new career with the unlocks. I suspect what happened is with the existing save, the tech nodes were already unlocked, but the PARTUPGRADEs had not been purchased. I think you would just have to go to R&D and "buy" them again -- tho the cost should be free.

How the max volume limit interplays with the individual dimensions may not be intuitive.

Re: nosecone, that is a little confusing. The first two upgrades for it only affect maximum length and diameter. The final upgrade at aerospaceTech removes the diameter limits. You're referring to the structural nosecone, not the deprecated cone tank, right? The configuration is such that the top diameter is supposed to be fixed at 0. I think there is a bug where it is getting locked at [the currently-unlocked min diameter] instead of 0. Will investigate. There would be an equivalent issue with the heatshield, although this is less of a problem since the length limitations are much more strict there (so you effectively always have something flat.)

Yep, the highlighting of when you actually get ProcParts improvements in the tech tree is definitely a plus!

Existing vessels are safe from any change: the fields that are affected by the upgrades are only used to configure the controls in the editor (VAB/SPH). They have no effect outside of there. (So they would affect a saved .craft file when you next load it into the editor, but no vessels that have already launched.)

Everything is MM patch-able. (The TECHLIMITs previously also were, I haven't made anything new here.) I actually added some patches that remove the limits entirely, which are applied when RO is detected -- RO doesn't care about the limits, and the career mod RP-1 does its own thing for restrictions.

An example patch that does what you're suggesting about removing the volume limit:

@PART:HAS[@MODULE[ProceduralPart]]:FINAL
{
    @MODULE[ProceduralPart]
    {
        %maxVolume = Infinity
        @UPGRADES
        {
            @UPGRADE,* { !maxVolume = DEL }
        }
    }
}
Chryseus commented 4 years ago

Yeah the legacy textures keeps getting set to enabled by itself when it loads, changing the sides texture temporarily fixes it although legacy textures still shows as being enabled, disabling the legacy textures also does nothing until you change the sides texture.

DRVeyl commented 4 years ago

The only way to change the Legacy Textures state is to click its button. Manipulating the different Texture fields will redraw the texture but will NOT change the mode. When you load again in the flight scene or the VAB, the mode is what determines the texture you will get.

Clearly the interaction between two competing methods for drawing textures in two different code bases is adding confusion and I need to fix that both controllers can be visible at the same time. Or are you identifying that when you change the Legacy Textures state by pressing the button that when you come back to the VAB, the button is not how you set it?

Lafreakshow commented 4 years ago

I think there is a bug where it is getting locked at [the currently-unlocked min diameter] instead of 0.

This sounds exactly like what I'm experiencing. Perhaps unlocking any of the upgrades forces an update which sets the value to what it is supposed to be. Leaves the question why it has this faulty value in the first place.... I saw that the value is set to 0 and constant in the part config. For the heck of it I tried setting that same value in different places as this at least somewhat worked when I was troubleshooting the volume issue but it doesn't make a difference here.

The heatshield does have the same issue. I forgot to mention that before. My screenshot doesn't show it well because of the angle, but that heatshield under the cone doesn't have much of a curve at all, it's almost cylindrical. In fact it has the stock end cap texture on the underside which looks pretty funny if you ask me. A bit like it was already in space and its underside is burned up.

And yes, It was indeed the structural cone. That was actually my first thought as well so I made doubly sure.

Existing vessels are safe from any change: the fields that are affected by the upgrades are only used to configure the controls in the editor (VAB/SPH). They have no effect outside of there. (So they would affect a saved .craft file when you next load it into the editor, but no vessels that have already launched.)

I expected something like this. It's actually really good because I can test the update under real world conditions with relatively little risk.

I will look into MM patches more in the near future. I just modified one that adds a kOS processor to every command pod. I just made it be less overpowered (Seriously, they were bigger and cheaper than the massive standalone kOS drive) but I saw the power in the patches and how relatively easy they are to write, so that is probably the first thing I'll do now that I am somehow gradually being pulled in by the KSP modding community. It's like it was with skyrim. As soon as I realise that it is completely within my abilities, I can't stop tweaking stuff and not long after I find myself learning yet another programming language. Though C# is probably a lot more likely to be useful outside of modding than Papyrus. Less buggy too...


Or are you identifying that when you change the Legacy Textures state by pressing the button that when you come back to the VAB, the button is not how you set it?

I had issues like this before. Sometimes it seems like the editor just refuses to remember things properly. The worst case I had of this was Sounding Rocket Motors regularly forgetting their thrust settings, resulting in some very entertaining RUDs. In my experience these things get more likely the more mods there are, the older the savefile is and are less severe on more powerful computers. Seems like at some point, the engine just gets overwhelmed. Which is to be expected when you push it lightyears beyond anything it was designed for I guess.

In that case though, this wouldn't be an issue with Procedural Parts. It would more likely be an interaction between some mods or plain too many mods in the install. In any case, it is dificult to troubleshoot without any information on how to reproduce the issue.

Lafreakshow commented 4 years ago

Another issue just cropped up, I suspect it's a mod conflict though so I'll keep it short. Procedural Parts seems to think that I'm running RealFuels, which I'm not. In any case this results in the RealFuels Liquid Tank being available and the SRBs values are completely off (giving only 122m/s² deltaV when a comparatively sized stock SRB gives over 2k m/s² with the same load). I've had a look at the logs so far but didn't see anythin obvious so I'm making a tiny script to help find related log entries and take a deeper look then, but perhaps someone has an idea.

The only thing I could think of being related is ModularFuelTanks, but I've tried it with only PP and MFT and didn't have this problem. Should I perhaps open a new issue for this?

DRVeyl commented 4 years ago

This sounds exactly like what I'm experiencing. Perhaps unlocking any of the upgrades forces an update which sets the value to what it is supposed to be. Leaves the question why it has this faulty value in the first place.... I saw that the value is set to 0 and constant in the part config. For the heck of it I tried setting that same value in different places as this at least somewhat worked when I was troubleshooting the volume issue but it doesn't make a difference here.

Fault is in code. Implemented the Constant setting as not user-changeable, but still restricted it to within the configurable limits. Makes some sense that Constant was also intended to be outside the limits range.

In fact it has the stock end cap texture on the underside which looks pretty funny if you ask me.

Aww, I did that intentionally for heatshields to default the end-cap texture to the same as the sides. Unless I got the setting wrong?

--

I did find the issue with forceLegacyTextures setting on all the time, and believe I have fixed it. Thanks, @Chryseus.

--

Another issue just cropped up, I suspect it's a mod conflict though so I'll keep it short. Procedural Parts seems to think that I'm running RealFuels, which I'm not. In any case this results in the RealFuels Liquid Tank being available and the SRBs values are completely off (giving only 122m/s² deltaV when a comparatively sized stock SRB gives over 2k m/s² with the same load). I've had a look at the logs so far but didn't see anythin obvious so I'm making a tiny script to help find related log entries and take a deeper look then, but perhaps someone has an idea.

This kind of issue is related to the pre-release that spawned all this. Could open a new issue, could just continue to address in this thread. Please provide KSP.log. I changed some of the RF detection and that may have been in error. The thing to search for in your log, btw, will be things with :NEEDS[RealFuels] still passing...

Lafreakshow commented 4 years ago

Aww, I did that intentionally for heatshields to default the end-cap texture to the same as the sides. Unless I got the setting wrong?

I was wondering if that was on purpose. It didn't look as arbitrary as the bugs so far. I like it actually. It'd be great if we could adjust it, make it round like it used to be and such. I don't recall if the bottom diameter is adjustable right now but that is pretty much my only complaint about this. I wonder which is more realistic though, All real heatshields I know of are curved to direct the shock zone away from the vessel but I'm by no means an expert or even marginaly educated about rocketry.

It did have the side texture by default, so the ablator texture as the rest of the part, I had to change it to make it look funny. Which is great actually, more customisation options are always good.

The thing to search for in your log, btw, will be things with :NEEDS[RealFuels] still passing...

This was immensly helpful. I had noticed these lines before but I wasn't aware that Module Manager lists patches that won't be applied separately. Turns out when the log file has over 100k lines, a few things go unseen. Now I know that ti is module manager thinking that I have RealFules installed and thus all patches that depend on it are applied. So it seems the patches included with PP do exactly what they are supposed to do. MM is just giving false information. Looks like CRFP - CarnationRED Flexible Parts was responsible. It had precisely two configs with :FOR[RealFuels] in it which somehow made MM think "ah, yes! RealFuels must be installed!". Without the mod, everything is back to normal but I suspect if I edit those patches, that the mod itself will work just fine.

This only cost me the whole day. Fortunately I still haven't regained my sanity from last week or I would have paid that too.

DRVeyl commented 4 years ago

Please post an issue on the FlexibleParts GitHub. It's a really bad practice to use :FOR[anotherMod], for exactly the reason of breaking MM patching logic. They need to use :BEFORE or :AFTER if they are referring to a different mod.

Lafreakshow commented 4 years ago

I was about to do that but it turns out there is a nwere version with this already fixed. And it's even that new. The reason I didn't use that is because it was marked compatible for 10.1 and up and I'm still playing on 1.9. So basically I could have saved an entire day of work if I had paid more attention to which version I installed a few day ago. Meh. Still, you'd think this is the kind of gamebreaking bug that deserves a backport.

DRVeyl commented 4 years ago

Maintaining previous versions can get tiresome. And the ProcParts patches were overly cautious as :NEEDS[RealFuels,!ModularFuelTanks] which is I believe redundant. I changed the patches to just :NEEDS[RealFuels] for this new version... thus exposing that issue.

DRVeyl commented 4 years ago

New RC: https://github.com/KSP-RO/ProceduralParts/releases/tag/v2.1.0_rc2

I believe it addresses the issues with forceLegacyTextures resetting, and also addresses the cone shapes that should have had one end restricted to 0 size. Please take a look and let me know how things stand?

Lafreakshow commented 4 years ago

Quick test looks very good. I assume it is intentional that the nodecone has an attachment node at the top and that it's top diameter is adjustable when another node in the techtree is unlocked, same deal as the heatshiel basically. In that case It works perfectly, Defaults are reasonable and upgrades don't break anything (nothing obvious at least). I've only looked at the heatshield and nosecone right now but I will integrate this version into my main install and play with it a bit. If there are suddenly issues with the other parts I should notice rather quickly. I'll also take a look at my logs to make sure the patches are applied properly, the whole RealFuels thing has made me kind of paranoid. Will report back if I notice anything that seems buggy.

DRVeyl commented 4 years ago

The attachment node is intended.

There should only be one diameter to adjust, however. After an upgrade gets applied, are both ends becoming unlocked? That would be incorrect. (And I see why it would be doing that, gr. Need to be conscientious about repeated calls from KSP to OnLoad for upgrade handling.)

DRVeyl commented 4 years ago

I ammended the release in place to fix that. Redownload it and try again please.

Lafreakshow commented 4 years ago

The behaviour hasn't changed. It still goes as follows:

Unlock Stability -> Enter VAB-> Place nosecone -> only bottom diameter is editable -> leave VAB-> unlock Aerodynamics -> enter VAB-> part from previous session is still there and now has top and bottom diameter editable -> add new nosecone -> this one is working as intended, only bottom diameter and length are editable.

Seems to me like newly created parts work fine but already existing parts have the restriction on the top diameter removed for some reason.

Interesting issue. Reminds me of how vessels used to explode randomly on load sometimes because the laws of physics weren't applied correctly.

DRVeyl commented 4 years ago

Edited once again. Please re-download.

Lafreakshow commented 4 years ago

That did it. Works perfectly now.

Chryseus commented 4 years ago

Pretty much everything seems to be working now. One thing that is a little bit of a problem is the nose cone diameter only goes up to 2.5m until you reach aerospace tech, since you can build 5m rockets by then this seems quite on the small side, I suggest unlocking 3m at advanced aerodynamics and 5m at experimental aerodynamics, the whole tech tree could do with a going over.

DRVeyl commented 4 years ago

Glad it worked. Not sure it was a good solution, tho... trying something that is more straight-forward. Try this one, updated again at 0010Z.

2.5m->3m seems a little underwhelming, but 2.5m->5m seems a reasonable step before unlimited. Would you still suggest that at experimental or at advanced aerodynamics?

Happy to take input on stock career balancing for this. :) That is probably worth opening a new issue.

Lafreakshow commented 4 years ago

I noticed another issue just now (I'm so sorry that this is evolving into a bit of a rework). The nosecone has a fixed weight of 1 ton. I noticed this when I replaced the (supposedly relatively heavy) parachute on my sounding rocket upper stage with a procedural nose cone and the weight almost doubled. Changing the size or shape doesn't seem to have any impact. I think 1 ton is the default mass for the liquid tank and If I understand the cfg files correctly, that's where the nosecone is sort of inherenting some of it's settings. So perhaps another case of a value not getting updated later on.

I get the feeling that the part cfg files are a pretty wild mess, if you excuse my language. Not that they're done badly, just that over time consistency degraded, as all codebases tend to do eventually. Especially considering that you understandably focus on RO and RP1 which have their own configs for many things. I will collect my thoughts and observations regarding balance and consistency and bring then into the issue opened yesterday related to that. I'm mostly mentioning this here because I think it is something that should be kept in mind as some these issues may be due to that rather than bugs in the code. Not this particular one with the nosecone weight though, I feel like the weight should scale with size, which is doesn't. That doesn't sound like something inconsistent config files can cause.

Outside of this, the scaling in terms of shape and limits works perfectly now so technically, the orignial issue I came here with is resolved. Yay!

DRVeyl commented 4 years ago

Ok, so it seems like I'm getting fairly close to making the pre-release into an actual release, closing out this issue, and starting to look at the balance one[s] that it has spawned.

Can you clarify if this mass issue with the nosecone mass [or I suspect any structural part] is something that was created between the pre-release and v2.0.6? Or between the pre-release and 2.0.3? Or has it just been an unreported issue for quite a while and can be fixed perhaps separately?

Lafreakshow commented 4 years ago

in 2.0.3 the mass is scailng correctly. I can confirm this because I just quickly fired up those versions (yay SSD and lowest graphics settings ever). Funnily, during this I noticed that (in the latest pre-release) the length of the nosecone in sandbox with all upgrades enabled is limited to 1.5m max. Not sure if that's intentional as in prior versions, it had it's limits removed in the late game.

In 2.0.6 the mass also scales correctly. At least in Sandbox mode. (in career it shoud be close to zero, according to logic, since in this version the nosecone is basically a 2d object. Not relevant but I find the thought funny).

DRVeyl commented 4 years ago

Very strange. Re-testing, I get an appropriate mass for the procedural nosecone. It matches the info in the TankContentSwitcher, and the Engineer's Report accounts the correct mass. Do you have similar info in the TankContentSwitcher PAW group?

As for the nosecone, looking at the history for it, it was never configured to go above 1.5m length. The first upgrade extended length to 1.5m, then the next two extended only diameter, to 2.5m and then uncapped.

image

Lafreakshow commented 4 years ago

Well it turns out I'm stupid. Well kind of, still an issue, just a different kind. It appears that this particular issue is a mod incompatibility. I'm not entirely sure which mod but I very strongly suspect ModularFuelTanks, as that's the only mod that changes other parts and was installed in both installs I used for testing. Pretty stupid of me not to clean the testing install... It starts even faster now too.

Anyway point being I threw all mods except Procedural Parts out and the issue was gone. I only now realized this might be an option because I don't even have the TankContentSwitcher entry and so I checked what other mods I had installed. I only noticed after seeing your screenshot. Rest assured though, I only installed Modular Fuel Tanks in my testing install yesterday, so most of my previous observations are unaffected.

Still, Procedural Parts includes a patch for ModularFuelTanks compatibility doesn't it? Seems that doesn't work quite right. I think RealFuels depends on ModularFuelTanks so this is a pretty serious issue considering they are all part of RP1.

Lafreakshow commented 4 years ago

Yep, it's Modular Fuel Tanks. Just tested it. screenshot6

DRVeyl commented 4 years ago

Thankyou. Tried to get too clever with the MFT patching. Fixed now, and updated the pre-release.

(BTW, the relationship between RF and MFT is "its complicated." They're a shared codebase, but install separate components, so it's entirely possible to break MFT configs but not RF ones.)

Lafreakshow commented 4 years ago

Seems to work as expected now. Wonderful.

DRVeyl commented 4 years ago

Marking this as closed with the official release of 2.1.0; please continue any balancing feedback in a separate issue. Many thanks for bringing this up, and the help with resolving!