allista / ConfigurableContainers

This is a plugin for KSP mod development.
MIT License
8 stars 7 forks source link

Procedural Tanks No Longer Auto-Updates With Resources #23

Open Nerdy314159265 opened 6 years ago

Nerdy314159265 commented 6 years ago

Configurable Containers / Core: 1.4.1.2 KSP Version: 1.3.1.1891 Procedural Parts Version: 1.2.12

I recently updated my version of Configurable Containers to the latest (1.4.1.2) and attempted to build a rocket. When adding a Procedural Tank it adds just fine, I could fix this by editing the resource types to something else and back or re-adding them. However, the restored resources immediately disappear when the tank is modified.

I uninstalled Configurable Containers and manually installed 1.4.1 with CKAN over command-line and that issue goes away (some config issues appear but I am assuming it's due to the reinstall?).

I reinstalled CC and tested further while typing this. When the tanks is the first part it shows with resources no issue, only when added to an existing root does it show with no resources. Also the volumes of the resources inside the tank do update to the new tank sizes within CC but don't seem to get applied through to the actual tank.

allista commented 6 years ago

I'm afraid I have to remove PP support altogether :disappointed: The issue you've described is "caused" by the TankContentSwitcher module that PP uses to handle part resources and mass changes. Previously @Starwaster proposed an MM patch that solved a conflict between the TankContainerSwitcher and CC modules; but with the current version of PP this patch no longer works, because now without TANK_TYPE_OPTION nodes TankContainerSwitcher removes all resources on any shape/size changes. I've read through PP code in an attempt to find a way to solve the conflict, but found none aside from reimplementing PP's algorithm for mass change (along with corresponding module fields). And this is not a maintainable solution. Especially now, when amount of time I can spare on modding is barely enough to make some fixes in my own code.

RealKolago commented 6 years ago

Maybe the mod author of PP can implement a fix.

RealKolago commented 6 years ago

Open an issue: https://github.com/Swamp-Ig/ProceduralParts/issues/234

RealKolago commented 6 years ago

Open an issue: https://github.com/Starwaster/ProceduralParts/issues/26

allista commented 6 years ago

This would be awesome, but right now I'll have to release a KSP-1.4.1 compatibility version without the patch for PP, or the latter couldn't be installed alongside CC.

Edit: oh, and the current maintainer of PP seems to be: https://github.com/Tidal-Stream/ProceduralParts

Starwaster commented 6 years ago

In point of fact, the patch I provided always behaved that way; it couldn't have done otherwise because TankContentSwitcher has always rebuilt its resources from the ground up based on the resources specified in the selected tank type. (and the tank type in the patch didn't designate any resources)

At the time, I did inquire as to any issues with said patch and what sort of behavioral interaction was expected between the two mods. (Heck, I don't even know how Configurable Containers was supposed to behave by itself since I don't use it myself)

But I never got any sort of response back and assumed the matter was settled.

As I see it the solution is either to

I think Option 2 is the way to go, that way you can set the mass to whatever you want based on the part's maximum capacity.

allista commented 6 years ago

Option 3:

Thus, replacing TankContentSwitcher with another content manager (like CC modules) does not change the behaviour of the dry part when its shape/size changes.

Option 4:

Starwaster commented 6 years ago

TankContentSwitcher already implements IPartMassModifier and already manages mass changes due to tank type change.

But option 4 looks awesome.

allista commented 6 years ago

But not ProceduralPart, so if we remove TankContentSwitcher, shape changes do not affect part mass. That is, ProceduralPart cannot be used by itself without TankContentSwitcher onboard.

My proposal is to make PP and TCW independent of each other. The change is almost trivial and I could make a PR if you're not against the idea.