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

Fix stage:liquidfuel #1082

Merged
merged 1 commit into from
Jul 1, 2015
Merged

Conversation

hvacengi
Copy link
Member

@hvacengi hvacengi commented Jul 1, 2015

Fixes #513
StageValue.cs

  • Re-write the resource getter code to return liquid fuel and oxidizer
    (and any other STACK_PRIORITY_SEARCH resource) based on the active
    engines.
  • This will return the total amount available to all currently active
    engines. It will not return zero when an asparagus stage is depleted.
    It will also count any fuel available to an engine that was manually
    activated rather than staged.
  • All other resources are returned according to what the root part is
    able to access (in my testing this aligns with the value shown in the
    resource panel).

Fixes KSP-KOS#513
StageValue.cs
* Re-write the resource getter code to return liquid fuel and oxidizer
(and any other STACK_PRIORITY_SEARCH resource) based on the active
engines.
* This will return the total amount available to all currently active
engines.  It will not return zero when an asparagus stage is depleted.
It will also count any fuel available to an engine that was manually
activated rather than staged.
* All other resources are returned according to what the root part is
able to access (in my testing this aligns with the value shown in the
resource panel).

Fix stage:liquidfuel

Fixes KSP-KOS#513
StageValue.cs
* Re-write the resource
getter code to return liquid fuel and oxidizer
(and any other
STACK_PRIORITY_SEARCH resource) based on the active
engines.
* This will
return the total amount available to all currently active
engines.  It
will not return zero when an asparagus stage is depleted.
It will also
count any fuel available to an engine that was manually
activated rather
than staged.
* All other resources are returned according to what the
root part is
able to access (in my testing this aligns with the value
shown in the
resource panel).
@erendrake erendrake merged commit 9020506 into KSP-KOS:develop Jul 1, 2015
foreach (var resource in list)
{
available += resource.amount;
capacity += resource.maxAmount;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hvacengi were you hoping to use this for something? it doesnt look like it is being used after the sum. If not you could write

 double available = list.Sum(resource => resource.amount);
 return Math.Round(available, 2);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah... I only did that while I was working on it because I thought it might be useful in the future. There's no reason to be doing the extra math, and I'm almost always in favor of more LINQ. Want me to make the change?

@hvacengi
Copy link
Member Author

hvacengi commented Jan 8, 2016

@EhevuTov Have you tested this with KSP 1.0.5 kOS 0.18.2? If this is still an not working for you, please submit a new issue, and include a copy of the craft file that you use to observe the issue. The most current implementation simply finds all active engines, and then totals up the fuel tanks that the engines report as being available. The current logic should be precisely coupled to the engines' ability to fire.

@EhevuTov
Copy link

EhevuTov commented Jan 9, 2016

Everything is working fine, thank you.

@macksimk
Copy link

I want to dump a radially attached fuel tank when it is empty. I have set this up as a stage. kOS does not report the stage as empty when the fuel tank is empty, counting instead all the fuel that is available to the engine. This is not desirable.

@hvacengi
Copy link
Member Author

While it may not be desirable in your use case, it is unfortunately a limitation that I think you will need to live with @macksimk. There are 2 issues at play:

  1. Replicating Squad's fuel flow logic is complicated, and prone to failure. We had our own part walking method at one time. It was fragile and prone to failure for KSP updates and fringe cases (like this). So we migrated to using one of KSP's methods for checking and providing the resources. So far that has survived a couple of updates well enough.

  2. Your request is to essentially make the term "stage" refer to "parts which will be dropped on the next staging event". I'm not sure that this is really the desirable default behavior. To make this change, we should also exclude the other resources which are not included in that definition of "stage". That means that an engine and it's fuel tank(s) that is currently running, but won't be dropped for 3 more stages would not be reported as part of the current stage, even if though they are currently active. This presents an issue for those of us who have delta-v calculators that are based on resource counts.

I think you would be better off adding a tag to the drop tank, and then use ship:partstagged to reference the tank itself and check for empty resources there. This method gives you very granular control, and also has the benefit of being significantly more efficient in the underlying C# as well.

Ideally it may be good for kOS to provide multiple stage selection options, possibly as separate structures supporting actual stage numbering. But for the time being it looks like an issue for maintenance, and we offer an efficient work around.

@macksimk
Copy link

  1. My case is not a fringe case. Most everyone who plays ksp tries to build
    a Shuttle replica at one point or another.
  2. Instead of me adding tags by hand, which is not efficient as I build a
    dozen rockets any given week, you could fix your dV calculator, which you
    only have to do once.

Btw, you do realise that if you drop engines and tankage, dV changes? How
are you accounting for this in your calculator, without knowing what
exactly will be jettisoned, and when?

  1. Ideally kOS would provide a fuel flow map as a data structure, or at
    least a FUELTANKS list to match the ENGINES one. But that is probably too
    much to ask.
  2. I just expect "stage" in the VAB/SPH to mean the same thing in kOS. Not
    an unreasonable expectation, I think, especially since when I tell a rocket
    to
    STAGE.
    in kerboscript, it does what I built it to do...

Basically now STAGE means two very different things in kOS and this is
nowhere even documented.
On 23 May 2016 15:57, "Brad White" notifications@github.com wrote:

While it may not be desirable in your use case, it is unfortunately a
limitation that I think you will need to live with @macksimk
https://github.com/macksimk. There are 2 issues at play:

  1. Replicating Squad's fuel flow logic is complicated, and prone to
    failure. We had our own part walking method at one time. It was fragile and
    prone to failure for KSP updates and fringe cases (like this). So we
    migrated to using one of KSP's methods for checking and providing the
    resources. So far that has survived a couple of updates well enough.

  2. Your request is to essentially make the term "stage" refer to "parts
    which will be dropped on the next staging event". I'm not sure that this is
    really the desirable default behavior. To make this change, we should also
    exclude the other resources which are not included in that definition
    of "stage". That means that an engine and it's fuel tank(s) that is
    currently running, but won't be dropped for 3 more stages would not be
    reported as part of the current stage, even if though they are currently
    active. This presents an issue for those of us who have delta-v calculators
    that are based on resource counts.

I think you would be better off adding a tag to the drop tank, and then
use ship:partstagged to reference the tank itself and check for empty
resources there. This method gives you very granular control, and also has
the benefit of being significantly more efficient in the underlying C# as
well.

Ideally it may be good for kOS to provide multiple stage selection
options, possibly as separate structures supporting actual stage numbering.
But for the time being it looks like an issue for maintenance, and we offer
an efficient work around.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1082 (comment)

@hvacengi
Copy link
Member Author

I should have asked this in my previous reply, but could you please move the discussion to a new issue,as this has moved beyond the scope of a comment on an existing merged pull request?

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

Successfully merging this pull request may close these issues.

4 participants