KSP-RO / RealFuels

Modular fuel tanks and engines, with real fuels and realistic performance.
55 stars 66 forks source link

Use kerbalism's resource system when installed #246

Closed dgfl-gh closed 4 years ago

dgfl-gh commented 4 years ago

Uses kerbalism's API to produce boiloff product (when present) to avoid the "incoherent behaviour" message.

Needs more testing. I've found it to work fine, but no one outside of me has (to my knowledge tested it).

Starwaster commented 4 years ago

I’m not familiar with Kerbalism; can you point me at any created issues or discussions on the issue that this is meant to correct?

On Sep 7, 2020, at 4:17 PM, Standecco notifications@github.com wrote:

 Uses kerbalism's API to produce boiloff product (when present) to avoid the "incoherent behaviour" message.

Needs more testing. I've found it to work fine, but no one outside of me has (to my knowledge tested it).

You can view, comment on, or merge this pull request online at:

https://github.com/NathanKell/ModularFuelSystem/pull/246

Commit Summary

Use kerbalism's resource system when installed File Changes

M Source/Tanks/ModuleFuelTanksRF.cs (44) M Source/Utilities/Utilities.cs (30) Patch Links:

https://github.com/NathanKell/ModularFuelSystem/pull/246.patch https://github.com/NathanKell/ModularFuelSystem/pull/246.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

dgfl-gh commented 4 years ago

Some discussion here on discord; the main thing (code-wise) that kerbalism does is replacing the stock resource consumption system with something that can work reliably at high warp rates. Doing this allows for various cool things, like making things work in the background as if they were loaded, but also creates an incompatibilty every time a resource is generated using the stock system, which is what happens in RFhere.

Starwaster commented 4 years ago

A few things here are concerning to me. One of which is that it's resulting in three resource manipulations for each single resource manipulation (per boiloff product) that are done without Kerbalism

And when is the Kerbalism call made? Ideally it would be in the same frame as RF is producing boiloff product but it could be out of phase in a different frame since they are calling us.

Something else to think about is that when you are removing (it is removing, not re-adding it as the comment suggests) boiloff product, there's no guarantee it's going to be drawn from the same sources it was added to. There might be only a single part eligible to receive the boiloff (placed for that purpose) but when you remove it, it's going to be drawn from every part in that stave evenly. (it's like ALL_VESSEL but only for the current stage)

Is there no way to call a Kerbalism function directly to pass a resource request to? That way you would replace

https://github.com/NathanKell/ModularFuelSystem/blob/e594ea0898da36f57ee4da1f609d522c665b19c2/Source/Tanks/ModuleFuelTanksRF.cs#L421

and skip https://github.com/NathanKell/ModularFuelSystem/blob/e594ea0898da36f57ee4da1f609d522c665b19c2/Source/Tanks/ModuleFuelTanksRF.cs#L428

Entirely?

dgfl-gh commented 4 years ago

Whoops, the comment is indeed wrong.

And when is the Kerbalism call made?

It gets called at every FixedUpdate. For some info, check kerbalism's wiki

Something else to think about is that when you are removing (it is removing, not re-adding it as the comment suggests) boiloff product, there's no guarantee it's going to be drawn from the same sources it was added to

well, it shouldn't happen though? The method is called for the part, to which I have just added an amount of boiloff product. Removing that same exact amount starting from the same part will result in 0 change overall unless something has modified the resources in between the 2 calls, which I don't think (but I might be wrong) is possible.

About the 2 different calls, I choose this for the simple reason that kerbalism completely ignores crossfeed or any other resource flow modifier. This way I let KSP check if there really is space available for the resource in the stage, and then let kerbalism consume it. EDIT: to be clear, it wouldn't "overflow" resources of anything like that. But it would possibly place the oxygen from your upper stage tank into your crew service module, which probably shouldn't happen.

Is there no way to call a Kerbalism function directly to pass a resource request to?

Sadly no, I don't think that'd even be possible with how kerbalism handles resources.

Starwaster commented 4 years ago

well, it shouldn't happen though? The method is called for the part, to which I have just added an amount of boiloff product. Removing that same exact amount starting from the same part will result in 0 change overall unless something has modified the resources in between the 2 calls, which I don't think (but I might be wrong) is possible.

STAGE_PRIORITY_FLOW is applied evenly to all eligible parts in that stage. It's basically ALL_VESSEL but only within the current stage. So let's say it's O2 (and it probably will be) - every part containing O2 will contribute when when it is drained whereas before it might have been only a catch tank or a sump tank that was receiving it. What happens to it after that when Kerbalism is notified that there is a resource you want to add? I have no idea. Will it add it back to parts that had it removed. I don't know.

But there's definitely potential for some weird behavior with boiloff product especially with O2. Depends on Kerbalism and how each player designs their ship. I doubt it will be gamebreaking but you'll probably see weird questions popping up later on down the road regarding weird resource movement.

dgfl-gh commented 4 years ago

I see what you mean now. I've pushed a possible solution which involves KSP's own "simulate" bool parameter in RequestResource, which should remove the even O2 consumption issue.