Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Procedural Tanks No Longer Auto-Updates With Resources #23

Open
Nerdy314159265 opened this issue Jan 7, 2018 · 9 comments
Open

Procedural Tanks No Longer Auto-Updates With Resources #23

Nerdy314159265 opened this issue Jan 7, 2018 · 9 comments
Assignees
Labels

Comments

@Nerdy314159265
Copy link

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 allista self-assigned this Mar 26, 2018
@allista
Copy link
Owner

allista commented Mar 26, 2018

I'm afraid I have to remove PP support altogether 😞
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
Copy link

Maybe the mod author of PP can implement a fix.

@RealKolago
Copy link

Open an issue: KSP-RO/ProceduralParts#234

@RealKolago
Copy link

Open an issue: Starwaster/ProceduralParts#26

@allista
Copy link
Owner

allista commented Mar 27, 2018

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
Copy link

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

  • Have TankContentSwitcher be modified to ignore all resource contents and just handle the mass (using a modified version of the previously provided patch that signifies to the modified TCS that resources are handled elsewhere, i.e. CC)
  • allista goes back to the previous method whereby TankContentSwitcher was removed but then Configurable Containers takes responsibility for modifying the part's mass as befits a tank of that size.

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
Copy link
Owner

allista commented Mar 28, 2018

Option 3:

  • ProceduralPart implements IPartMassModifier interface and manages dry mass changes due to shape change.
  • TankContentSwitcher also implements IPartMassModifier and manages mass changes due to tank type change.

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:

  • leave everything as is, without any patches on CC side, so that both mods could make their thing alongside without interfering.

@Starwaster
Copy link

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

But option 4 looks awesome.

@allista
Copy link
Owner

allista commented Mar 28, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants