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

Stage:Liquidfuel failing to detect fuel in stage. #174

Closed
Dunbaratu opened this issue Jun 29, 2014 · 12 comments
Closed

Stage:Liquidfuel failing to detect fuel in stage. #174

Dunbaratu opened this issue Jun 29, 2014 · 12 comments
Assignees

Comments

@Dunbaratu
Copy link
Member

The screenshot says it all.

screenshot of the problem

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:

                   && attachNode.nodeType == AttachNode.NodeType.Stack //and the attached part is stacked (rather than surface mounted)

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.

@erendrake
Copy link
Member

Would you like to work on this?

@Dunbaratu
Copy link
Member Author

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.

@Dunbaratu
Copy link
Member Author

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:

screenshot from KSP

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.

@erendrake
Copy link
Member

good find, and blarg for KSP not giving us the right data.

@Dunbaratu
Copy link
Member Author

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.

@erendrake
Copy link
Member

seriously? does the UI show remained fuel?

@Dunbaratu
Copy link
Member Author

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.

@Dunbaratu
Copy link
Member Author

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.

@erendrake
Copy link
Member

More and more i realize we arent dealing with an api, just an overly promiscuous internal implementation :P

@north1
Copy link

north1 commented Jun 30, 2014

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

@Dunbaratu
Copy link
Member Author

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:

  • The part object on one side of the connection must state "this attachment point on me is one I will allow crossfeed through".
  • The part object on the other side of the connection also must state "this attachment point on me is one I will allow crossfeed through".
  • the connection itself, an object of type AttachNode, also must state "I am the sort of attachment that will allow crossfeed through it.".

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.

Dunbaratu added a commit to Dunbaratu/KOS-1 that referenced this issue Jun 30, 2014
  1 - Side-mounted engines now can 'see' fuel through their surface mounts.
  2 - Now detects when an attachment can't carry fuel (attachment is strut)
  3 - Now traverses yellow fuel lines to find fuel.
@Dunbaratu
Copy link
Member Author

this should be closed - the pull request dealt with it.

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

No branches or pull requests

3 participants