The SRBs you get in the stock tech tree gradually get better Isp as you progress through the tree, but not the case in SSTU. This should be pretty fixable with part upgrades. Might even add a few more upgrades, since stock SRBs are nowhere near as performant as modern real-world solid rockets.

This is simple and I'd be willing to do it. Just wanted to get your feedback first.

Yes.

However I believe that part-upgrade support would need some additions to the PartModule; the way the MSRB works is incompatible with the direct part-module-stat-upgrade mechanics and it will need to use them as simple triggers with plugin-determined effects.

Not sure I really want to go down that route, and would rather find a way to allow the part-upgrades to function as they are supposed to.

As with fixing the part-upgrade support for the rest of the engines, it is merely something I haven't had time (or motivation) to investigate.

Hmm, might have to look into this some.

I see that the burn time assumes an Isp. It should be possible to get the fuel flow rate from the engine itself. Are there any other assumptions that might break?

The burn time using a hard-coded ISP is...erm.. obviously incorrect, but was probably a valid assumption when I was doing initial prototype/development on the module. It should (and needs to be) set to the current ISP from the engine; which should be easy enough to get from the modules' ISP curve (or get the pre-calculated flow-rate as you suggest). Thanks for pointing that one out for me, fixing it up now :)

Humm... I was thinking of manipulating engine-thrust, which is far more problematic.

Engine ISP actually shouldn't be a problem. If you can patch it on the stock engines, it should work fine on the MSRBs (and ModularEngineClusters). After I fix up the GUI updating code, it should 'just work' from there.

Although... I am going to have to find a method to adjust ISP at some point. Would be very nice to be able to offer ISP variations through the Nozzle selections; e.g. for a vacuum optimized nozzle variant. Not relevant for the immediate concerns and near-future releases though.

GUI thrust and burn-time all fixed up in dev versions. Also fixed the incorrect thrust and burn-time displays that resulted from the >0 min thrust on the parts. Will push these updates to the private dev repo shortly, and you should be able to grab the updated SSTUTools.dll from there (I think gave you access, if not, I will shortly). ( https://github.com/shadowmage45/SSTULabsDev/blob/dev-ksp120/GameData/SSTU/Plugins/SSTUTools.dll )

(I really should turn that private repo into an assets-only repo...but that is a wholly separate discussion)

Been experimenting, but haven't gotten upgrades to apply properly yet. With some experimentation it seems like it's specific to the engine module - any upgrades I define on it don't show as having been applied (however, they do appear in the tech tree). Do you have any idea what might be preventing upgrades from applying to the engine module? I'll keep investigating, but just wanted to see if there was anything obvious.

I didn't see anything on the SRB module that should have messed with the ISP being updated. Have you tried it on the stock SRB's / some simpler parts?

I'm not sure on some of the finer details on how the part-upgrade system applies its upgrades. It could be that the ISP upgrades aren't triggering a re-calculation of the flow rate / other cached internal values on the engine module.

Don't think it has anything to do with atmosphereCurve specifically. If the upgrade was applied, its description would show when you click "Show upgraded stats" in the editor. My working theory is that somehow the upgrade nodes are being stripped off by something after the part is instantiated, but I'll have to do some investigation on this.

Hmm.. interesting. I have never tried upgrading ModuleEngines with the part-upgrade system, but AFAIK it should be doable as the PorkJet parts used PartUpgrades for a few things.

I'll see if I have time to play around with it this week, have been intending to sit down and figure out how that system works anyhow so that I could fix up the issues with upgrading ModularEngineCluster parts.

Figured it out. It is SSTU-specific, but also really the result of non-intuitive (nonsensical?) KSP behavior with regard to upgrades.

When you call Load it will wipe out any applied upgrades even if there is no UPGRADESAPPLIED node (which it looks for). This means that adjusting the engine thrust wipes out the applied upgrades.

I will report this to Squad. A temporary fix is also possible. Instead of calling module.Load(node) you could call module.Fields.Load(node); module.OnLoad(node). This might also bypass some other stuff that doesn't need to run such as looking for events and actions .

Looks like simply changing from using PartModule.Load() to PartModule.OnLoad() in my engine-thrust updating code should fix up the problem. (Might even have been a typo originally, as I don't think there is a reason to use Load over OnLoad)

ModuleEngines looks like it manually parses the min/max thrust values during OnLoad(), so for this case there is no need to manually update the fields / use the field.load() methods. Other modules might need the fields updated, but in this case I think the above solution should work.

Making the changes in SSTU code now, and should have it available in the dev repo later this evening.

Edit: Appears to be working as far as I've tested it, changing the thrust of the F-1 engine (as I'm unsure off the top of my head how to patch the ISP nodes to test those...).

(note the 3400 output from upgrade, vs 3174 base shown in editor part info window)

Should be available in the new dev branch of this repository a bit later this evening (have moved plugin stuff over to this repository, only modeling assets will remain in the private repo).

Interesting find, and thanks for the investigation.

That may well explain why the modular-engine clusters don't like their thrust to be upgraded either (even though I cache things in a way that -should- work fine).

I'll investigate that workaround a bit to see if it still does everything needed. Seems like it... should, as long as I update the fields beforehand.

This is an unrelated upgrade comment... seems like every tank upgrade should also give the decoupler upgrade, otherwise you end up with tanks you cannot use easily. (example is the 3.125 upgrade, that you cannot use until you upgrade the decoupler to 3.75)