SSTU - Shadow Space Technologies Unlimited

SSTU - Shadow Space Technologies Unlimited

98.5k Downloads

SSTUModularEngineCluster - Incorrect mass for multi-engine clusters when ModuleEngineConfigs is used (RO)

shadowmage45 opened this issue ยท 14 comments

commented

Likely need to add a toggle to the MEC module that will leave the mass unadjusted, a simple 'adjust mass' flag or similar

commented

I don't think RF should add any mass for a given config unless a mass offset is specified in the config.

@NathanKell any thoughts?

commented

Pricing also isn't affected when using multi-engine clusters, meaning 4 engine clusters cost just as much as single engines. That might be more complicated tho since the price is set by RP-0.

commented

Hmm thought it only did that for fuel tanks, but I guess it does it for engines as well.

If your plugin is taking care of all mass, including that of the individual configs, then you can just set originMass and origCost to -1.

commented

I only take care of the base engine mass and cost.
I -dont care- about the engine configs; and dear god how I wish that entire module would just die already, it causes me nothing but grief.

So, if it were up to me there wouldn't be any problems.. as I'm -already- setting the mass properly. But ModuleEngineConfigs likes to be an ever-persistent pain-in-my-ass and so, of course, it has to manipulate mass as well (and thrust...but that is a different argument). And of course if I were to try to set my engines up in RO to -not- use ModuleEngineConfigs people would bitch about that as well (already tried that route...didn't go so well).

Which brings me back to the point of... I'm about half a step away from dropping all RO support because of these kinds of issues. It takes up so much of my time to fix problems, and make invasive code changes, for a mod that I don't use... and really just ends up making me bitter and angry towards dealing with modding... which is -very quickly- going to result in me no longer doing modding at all.

commented

The ModuleEngineConfigs are just too usefull in all that glorious mess that is Realism Overhaul. Otherwise manipulating a single engine forces you to update like 20 configs.

I'd say, if it's too hard to make it work, or a too arduous fix, then leave them like they are.

commented

I can understand that. If you don't play RO it is annoying to try and maintain it. Flip side is I don't play stock, and refuse to support it in my mods, so I completely understand.

Would you consider allowing RO to fork your code and maintain an RO compatible version as a separate thing so you don't have to?

commented

In a perfect world, yes, forking and making a derivative would be the best answer. Sadly I cannot see this working out well; users are already terrible at keeping the RO issues in the RO thread, I can just imagine if there were a separate fork how well they would manage that.

Its not that this particular 'fix' is difficult to implement; its the fact that I even have to make code-side changes to support an external mod; one that I do not use and have no idea what half of the functions are even supposed to be used for. Really, from my point of view... any mod that alters the parts for other mods' needs to ensure that it is compatible with those other mods' functions, not the other way around.

Either way, I have a tentative fix in-place. If the SSTUModularEngineCluster finds a ModuleEngineConfigs module present on the part it will disable its own mass adjustment, leaving all mass adjustment up to the ModuleEngineConfigs module.

This is done during the Start() method, any queries to GetModuleMass(float defaultMass) prior to that will return indeterminate results.

'Fix' will be available with the next SSTU update; likely either tonight or tomorrow sometime.

commented

The mass for RF is likely related to my use of the 'scale' feature/function (I use that to scale the thrust output/etc for multi-engine clusters).

Indeed:
https://github.com/NathanKell/ModularFuelSystem/blob/master/Source/Engines/ModuleEngineConfigs.cs#L991-L1004

Also, looking at that... ModuleEngineConfigs is still setting part.mass directly...

commented

@shadowmage45 thank you for finding that issue in RF, I will fix ASAP.

In general RF does need to be able to modify mass because engine CONFIGs can vary in mass (for example early vs late LR91s). What would be the best thing I could expose for you? Does a single 'scale' number work in this case?

commented

@NathanKell - The existing RF setup regarding the use of 'scale' works properly after I disable the mass-updating for the SSTU module (the 'fix' I mentioned earlier). So it appears that no action should be necessary on your end for this particular case. I'll likely even take it a step further and have it a config-level toggle so it can be used more generically rather than have it specifically check for ModuleEngineConfigs.

The current 'scale' value system seems to work fairly well for most uses that I've come across; there were just caveats where it would scale more things than I had planned for/intended :)

I'm honestly not sure what would be the best way to approach the setup for ModuleEngineConfigs. I'm unaware of any other mod that dynamically changes the # of engines in a part, so any solution to this on the part of RF would be a bit of a single-use setup.

Off the top of my head, some things that would have helped/could help:

  • Some way to decouple the engine base-mass adjustments from the engine-variant mass adjustments
    • moduleMass = variantMass * scale
    • moduleMass = (variantMass + baseMass) * scale
  • Okay, so really that was all I could come up with; I had a few others that would have been of more limited use and more invasive/painful to implement. The one above really would solve the situation in this instance.

And thanks either way ;) I understand how busy you are with so many projects (and people) competing for your time and attention.

commented

Better supporting mods like SSTU is I think important and shouldn't be considered one off stuff. I've wanted to do similar parts for the longest time, heck I want completely dynamic stages, and I know NK has thought about it in the past as well. Mainly its KSP reasons it hasn't been done yet, so I really applaud and enjoy your work.

Your system of dynamically changing the number of engines I think should be a more open two way street. Things like engine configs (MEC) should be able to be configured independently of the cluster size, but the cluster size should still have an effect on things outside of SSTU if they so desire. For example TestFlight would love to be able to know that a cluster has X engines on it so that the values could be scaled appropriately and function identical to if the player has individual engines. And that is just one example.

tl;dr is I really love what you are doing and want to see if become more of a standard not a niche thing, so don't think of yourself like that :)

commented

No, you should not have to. This should either be just a matter of getting the config right, or a fix on RFs side. You tend to move very fast which is certainly a good thing, but in this case means @NathanKell hasn't had a chance to respond.

commented

Yeah, sorry for the delay here. Reading through.

commented

I agree with @jwvanderbeck that that kind of system is something we'd love to support better / invest more into, so it's far from a oneoff. :)

The mass case is an interesting one, because there are more cases than just clusters; if the engine is part of a stage then really the extra mass should not be multiplied by the multiplier...