SSTU diameter part upgrades

[assassinacc]

Tested and works. Related to #1625

[assassinacc]

That fixes the issue with the changed part upgrade method, covering all RO supported upgradeable SSTU parts. Related to #1625

[assassinacc]

Hold on, something is not working right.

[assassinacc]

@pap1723 and I did some more testing and the patches seems to apply correctly (according to the mm config cache), but somehow the partupgrade nodes get not applied to the techtree or gets somehow overwriten. We suspect something (SSTU plugin?) messing with it after the patches have loaded. The mm config cache looks right.

[assassinacc]

@shadowmage45 any idea?

[shadowmage45]

Not really; the stock part-upgrades seem to work fine in my uses, at least as far as the tanks and fairings go (engines/srbs have some issues that are being fixed up in dev versions).

I'm not seeing anything in the patches that sticks out at me as incorrect

Do the patches work in the absence of RO?
Does direct editing of the configs work?

Mind posting the MM-cache output for one or two of those parts so that I could take a look at it?

[pap1723]

Hey @shadowmage45
Thank you for your response. I have actually decided to test modifying the PARTUPGRADE tech unlock nodes in a stock install with only SSTU.

The PARTUPGRADES still do not move to the correct nodes. There is something that is happening with the processing inside of SSTU that is taking place after Module Manager modifies the files.

If I edit the information in the SSTU folder, all works fine, but any exterior MM configs do not work.

Thanks,
Pap

[shadowmage45]

Hmm.. interesting. I'll try to take some time to do some testing on patch-based upgrade editing. Do you have a patch file handy that I could try out (that would work in an otherwise stock/sstu install)?

Sadly, if there is anything in the plugin that needs fixed, even if I fix it immediately, it won't be available until KSP 1.3+ =\ -- in the middle of a major systems overhaul for the modular parts, and it still has at least a few weeks of work left on it.

[shadowmage45]

One other bit that came to mind -- if you are worrying about the part-upgrade tech-tree node locations -- you need to start a new game between tests.

The part-upgrade node locations are stored in the game persistence file (as is their purchased/unpurchased status).

[pap1723]

Alright @shadowmage45 @assassinacc @stratochief66 this cannot be fixed.

I have done a bunch of tests and it has to do with how part upgrades are treated by KSP. I have tested on SSTU stock and I have created my own part, given it an upgrade, and then tested changing the tech and it does not work.

This must have something to do with when PARTUPGRADE is read by KSP. @NathanKell can you shed any light on this?

[shadowmage45]

It seems like it should be workable in one fashion or another, as I've used patches to add part-upgrades to other SSTU parts and have had them work successfully.

Haven't tried it on the fuel tanks themselves, but it doesn't seem like 'MM-patches are incompatible with the upgrade system in general', which would point to there being a problem with this specific implementation.

As I stated earlier regarding tech-node placements -- you must create a new game after changing/patching the tech-tree node locations for the upgrades, as those are stored in the game-persistence file, which does not get patches applied to it.

[pap1723]

@shadowmage45 I have been doing exactly that each time, creating a new career.

However, the issue is not patching of the UPGRADES in the parts. The MM configs that @assassinacc has made actually work with SSTU.

The issue is with the PARTUPGRADES nodes. They somehow are getting called by KSP before the MM configs overwrite the ones in the SSTU folder. In looking through your source code for SSTU, I see that you do search for Upgrades in two files. According to the KSP log, SSTUTools.dll does run before MM?

`Source\WIPModule\SSTUUpgradeTest.cs (13 hits)
	Line 9:     public class SSTUUpgradeTest : PartModule
	Line 29:         //public override void ApplyUpgradeNode(ConfigNode node, bool doLoad)
	Line 31:         //    base.ApplyUpgradeNode(node, doLoad);
	Line 32:         //    MonoBehaviour.print("ApplyUpgradeNode: \n"+node);
	Line 35:         public override bool ApplyUpgrades(StartState state)
	Line 37:             MonoBehaviour.print("ApplyUpgrades: " + state);
	Line 38:             return base.ApplyUpgrades(state);
	Line 41:         public override void LoadUpgrades(ConfigNode node)
	Line 43:             base.LoadUpgrades(node);
	Line 44:             MonoBehaviour.print("LoadUpgrades: \n" + node);
	Line 47:         public override bool FindUpgrades(bool fillApplied)
	Line 49:             MonoBehaviour.print("FindUpgrades: " + fillApplied);
	Line 50:             return base.FindUpgrades(fillApplied);`

and also here:

SSTULabs-master\Source\Util\ModelData.cs (12 hits)
	Line 64:         public readonly string upgradeUnlockName;
	Line 94:             upgradeUnlockName = node.GetStringValue("upgradeUnlock", upgradeUnlockName);
	Line 94:             upgradeUnlockName = node.GetStringValue("upgradeUnlock", upgradeUnlockName);
	Line 94:             upgradeUnlockName = node.GetStringValue("upgradeUnlock", upgradeUnlockName);
	Line 147:         public bool isAvailable(List<String> partUpgrades)
	Line 149:             return String.IsNullOrEmpty(upgradeUnlockName) || partUpgrades.Contains(upgradeUnlockName);
	Line 149:             return String.IsNullOrEmpty(upgradeUnlockName) || partUpgrades.Contains(upgradeUnlockName);
	Line 149:             return String.IsNullOrEmpty(upgradeUnlockName) || partUpgrades.Contains(upgradeUnlockName);
	Line 495:         public bool isAvailable(List<String> partUpgrades)
	Line 497:             return modelDefinition.isAvailable(partUpgrades);
	Line 578:         /// Get array of model data names, taking into consideration upgrade-unlocks available on the part module
	Line 589:                 if (data[i].isAvailable(module.upgradesApplied))

[PhineasFreak]

@pap1723 I believe that MM was never tested against PARTUPGRADE nodes (apart from this PR) and MM was never updated to handle them specifically (like it was done with the PHYSICSGLOBALS node for example).

In this case it would be a good idea to ping sarbian and ask him:

• If MM can currently handle PARTUPGRADE nodes (if so then there is a problem preventing that).
• If MM can be updated to handle PARTUPGRADE nodes.

[shadowmage45]

Those bits of code don't change anything regarding part-upgrades -- there are no write operations going on there, only reads. Thus they cannot (logically) be causing any kind of conflicts.

That 'SSTUUpgradeTest' module is not used anywhere by any parts, and was simply created for debugging and getting an understanding of how/where the upgrade system hooked into PartModule code. The other bit of code you posted simply iterates over the already applied upgrades and checks vs. a config value in the modular-part-model data constructs.

And no, I don't touch anything regarding config files until after ModuleManager has fully processed. As far as I'm aware the first time that any SSTU code gets run is during the ModuleManagerPostLoad callback, and that is mostly reading of config files. (For reference, all mod-added .dll's get loaded at the very start of the game, prior to any sort of asset or config processing)

At no point to I touch the configs in the database, or do anything regarding part-upgrades that could logically cause any kind of interference (not all stock code is logical though...).

---

I'll see about taking the patches from this PR and running them in a clean install, absent of any other mods that might interfere. That should at least inform us if it is a problem with SSTU code, module-manager/stock code, or an external mod interfering with things.

-Might- get a chance to play with this later this evening, but chances are it will be a bit later over the weekend.

---

Just to make sure I'm understanding the problem properly -- the upgrades are being applied to the part/module properly, but the problems you are running into are regarding the tech-tree placement of the 'upgrade parts' themselves?

[pap1723]

@PhineasFreak there have been 5 questions about it on the MM thread dating back a while (I was one of them a while ago) and it has not been figured out from what I can tell.

@shadowmage45 I gotcha. I didn't know what I was really looking at in the code, it was the only thing I found that referenced any type of upgrades.

And yes, you understand the situation correctly.

[shadowmage45]

Thanks for the confirmation, I'll do some research and get to the bottom of this, as it sounds like it might be a problem more wide-spread than just RO use, and I plan to make more extensive use of the PartUpgrade stuff in the future. Would be good to know that it is all functional and fully patchable.

[shadowmage45]

From a bit of... erm... poking... at the KSP-API, it looks like the PartUpgrades are fully processed and loaded prior to MM running its code.

The stock loading code goes (in order):

• load models, textures, sounds
• load asset bundles
• load config files
• load resources (from the pre-patch loaded config files)
• load experience system (from the pre-patch loaded config files)
• load part-upgrades (from the pre-patch loaded config files)
• Finished.

Module Manager then runs the patches against the config files that exist in the GameDatabase, and reloads the resource system (and experience systems?). At this point the PartUpgrades have already been populated/loaded from the un-patched files, and thus have stale tech-definitions.

This might be simple to fix in ModuleManager by reloading the upgrades. Might. I have no idea how amenable the upgrade system is to reloading...

Definitely a problem that needs to be addressed on at least the ModuleManager level, possibly even at the stock-code level.

---

At least the mystery of what is going on is solved. Now just how to address the actual problem?

This also explains why my attempts to patch upgrades into parts did not run into errors; while I was patching the UPGRADES into the part-modules, the UPGRADEPART nodes were done as standard (non-patched) configs.

[shadowmage45]

Further investigation reveals that the solution is indeed very simple, and is something that will need to be implemented by ModuleManager.

PartUpgradeManager.Handler.FillUpgrades();//force reload PartUpgrade parts -- needs to be called after patches are applied

I will do some further testing to make sure that it works properly without side effects, and submit a PR to MM repo shortly with the fix if it all works out.

---

Fix checks out. No apparent side effects.

This patch now produces the following result:

@PARTUPGRADE[SSTU-DC-D1]:NEEDS[SSTU]
{
	@techRequired = start
}

[assassinacc]

Awesome. Thanks to you all <3

[assassinacc]

tested. works as intended with mm 2.7.6

[assassinacc]

Btw. also applies to already existent save games. So no need to start a new save.

[assassinacc]

tested for over a week.
no bugs found related to the changes made.
can be safely merged :)

[pap1723]

I agree with @assassinacc I have been testing extensively as well. Good on my end!