blowfishpro / B9PartSwitch

A Kerbal Space Program plugin designed to implement switching of part meshes, resources, and nodes
GNU Lesser General Public License v3.0
50 stars 33 forks source link

Directly update tank type and volume for ModuleFuelTanks #183

Open bfrobin446 opened 4 years ago

bfrobin446 commented 4 years ago

If ModuleDataHandlerBasic was allowed to call Load() on a ModuleFuelTanks, it would clobber any resources that ModuleFuelTanks was managing, making it impossible to launch a rocket with fuel in the affected tanks. This code works to set the volume of a tank without resetting its contents, which should be enough to close issue #19 .

blowfishpro commented 4 years ago

I noticed that Procedural Parts does this with an event rather than reflection, maybe we could leverage that here too?

https://github.com/KSP-RO/ProceduralParts/blob/91286598583ba086a1801ed001346581b252a586/Source/ProceduralAbstractShape.cs#L260

https://github.com/NathanKell/ModularFuelSystem/blob/25caf921c9711d10f0d8b0d30532df445b253b41/Source/Tanks/ModuleFuelTanks.cs#L709

bfrobin446 commented 4 years ago

Setting tank volume with the event turned out to involve a little bit of extra math, but everything still works the same.

Bellabong commented 3 years ago

Hi there, is there an estimation as to when this pull request is implemented/works? I've been doing RF configs for BDB and a lot of its newer parts are reliant on tank volume switching.

blowfishpro commented 3 years ago

@bfrobin446 can you commit to fixing issues if they come up?

bfrobin446 commented 3 years ago

@bfrobin446 can you commit to fixing issues if they come up?

I haven't been playing as much KSP in the last couple of months as I was in March and April, so my response time will probably be three to four weeks for the foreseeable future. But if that's a time frame you can live with, I can commit that I won't let anything I'm tagged in go longer than four weeks.

DRVeyl commented 2 years ago

This looks like it might be effectively dead (no new comments since Aug 2020). I think the RO team is interested in this feature, however. Should we fork the source of this PR, make some of the changes as described, and re-PR? I'm not familiar with the B9PS side of things... is this technique still reasonable?

Speaking as a current ProcParts maintainer and also doing refactoring of RF, I'll confirm that sending an event like so is a good way to tell MFT to update the volume. We probably need a better way to change the type dynamically [if you want something responsive while in the VAB] via a similar event. And maybe make the event handler support volume or totalVolume so you don't need to go through these contortions of gathering the utilization setting.

        private void InitVolume(ConfigNode node)
        {
            // If totalVolume is specified, use that, otherwise scale up the provided volume.
            if (node.TryGetValue("totalVolume", ref totalVolume))
                ChangeTotalVolume(totalVolume);
            else if (node.TryGetValue("volume", ref volume))
                totalVolume = volume * 100d / utilization;
        }

is some current code in MFT. So the idea exists, it's just not easily exposed. (And I'm not sure this code "works," it won't go through the handler that updates resource quantities... tho that would be a 1-line fix.)