Closed Dunbaratu closed 10 years ago
Would you like to work on this?
It's clear I'm going to have to write a wrapper around the weird part attachment structure to get the real list of attached parts, because the attachNode lists don't seem to contain the radially mounted things.
In the testing of my fix to this I discovered another problem with stage amounts that is in the same exact ProspectForResources call, and was also there all along. See this screenshot:
It's counting the fuel in the two tiny tanks as if it was part of the same stage when it's not, and thus giving a value of 191 instead of 180 for stage:liquidfuel.
I saw this a few times before but had a hard time repeating it so I never reported it. Now that I've had my hands in the code I can see what causes it and it's repeatable.
The bug is triggered by having struts that connect a fuel tank of one stage to a fuel tank of a different stage. When ProspectForResources does its walk of the parts graph, it is traversing through that strut as if it was a crossfeed capable connection when it's not. The KSP API calls are treating it like it was crossfeed capable when its not, which is annoying.
I'll look into this as well and fix it in the same pull request. I may have to just hardcode a special case for struts, given that the KSP API is lying to us about their crossfeed capability.
good find, and blarg for KSP not giving us the right data.
Yeah I had a lot of problems with my staging logic before and I think this may have been the problem. The original bug makes it say the stage has no fuel when it does, and this new bug makes it say the stage has fuel when it doesn't. that was my main reason for switching to a check of flamed out engines to trigger staging - it was the only thing that was working reliably.
Oh, and on an unrelated note, on one of my tests I had a reported stage:liquidfuel of 0.02 and the engine was flamed out, so your idea of rounding to the hundredths place might not be enough to fix that rounding problem. Sometimes it's even less accurate than the hundredths place. Not sure what we can do about that.
seriously? does the UI show remained fuel?
Yup it was in the UI that way too. I also saw a thing that might be the actual cause of that problem. While liquidfuel was not quite zero, oxidizer was zero, so it wasn't possible for the engine to burn the rest of the liquidfuel. The implication is that the ratio of consumption must not have been exactly accurate, such that as the engine burned fuel along the way it was consuming fuel at something slightly different from the 11:9 ratio of oxidizer to liquidfuel that its supposed to.
Perhaps it was because I was using a very small engine on a low throttle setting, thus the consumption of fuel occurs over a large number of updates that each consume a small amount of fuel. I'd imagine the number of floating point roundoff errors getting accumulated is greater in that circumstance.
I now have the original problem surface-mounted engines fixed, and the second problem with struts fixed. But as long as I have my hands in the code I found out why the fuel lines aren't being taken into account and I can correct that problem too. (It's because, just to make our lives harder, the KSP API uses a different THIRD way to traverse fuel line links - they aren't attachNodes at all, and in fact are a separate data structure independant of the parts tree. There is no way to start from a Part and find links to the fuel lines attached to it. Instead you have to start from the vessel's list of all its fuel lines and they'll tell you which parts they're linked between.
More and more i realize we arent dealing with an api, just an overly promiscuous internal implementation :P
I'm not sure I've ever worked with an API that wasn't just an overly promiscuous internal implementation. Some API's just dress up to look prettier :p
To a large extent what makes this API a problem is that it's not documented. The strut problem is fine once you understand the API, but the only way to understand it is trial and error. The strut problem is that all the possible places where the interface says something is crossfeed capable have to be combined together in a boolean "and" relationship. Each one is a single small piece of the full expression, and this was never mentioned.
For a connection to truly be a crossfeed connection ALL the following must be true:
If any single one of them is false, the crossfeed is disallowed.
It's that third check that was missing before.
If parts are connected with struts, both the parts themselves will claim (for some reason) that they allow crossfeed through that attachnode. It's only the attachnode itself (which is the strut) that will block the feed by saying "I am not the type of attachment that you can crossfeed through."
Now, with this structure in place you'd think that the way fuel lines would be implemented is that they'd be a lot like struts, except they'd be attachnodes that provide no physics force but do have their crossfeed flags on. ... You'd THINK. But alas that's not how they work.
this should be closed - the pull request dealt with it.
The screenshot says it all.
I suspect the problem may be triggered by using radially-mounted engines instead of stack-mounted engines, but I haven't confirmed that yet.
(update) Yes I definitely think it's due to line 114 in Utils/Utils.cs saying this:
There are some engines for which being radially mounted is their correct mounting, and this bug occurs when I use one of them.. I'm not sure what the fix is, though, because for a stack engine if you try to radially mount it it won't see the fuel unless it has a yellow hose, so it is an important check that the attachment is on the "fuel feeding" attachment point, but I don't think this is the check for it.