search

KSP-KOS / KOS

Fully programmable autopilot mod for KSP. Originally By Nivekk
Other
689 stars 228 forks source link

Negative resource transfer creates resource from nowhere #2995

Open Ernesto-Alvarez opened 3 years ago

Ernesto-Alvarez commented 3 years ago

Attempting to create a resource transfer with a negative amount of resources ends up creating resources in the source part without taking them from the destination. For this to work, the parts MUST NOT be full.

Reproduction steps.

  1. Unpack the following data containing a craft and 2 .ks files. fuelbug.zip
  2. Load the ship into the VAB and then place it into the launch pad
  3. Press the resource button in the sidebar and tick the liquid fuel resource so that all tanks are visible
  4. Open the console
  5. CD into the directory where the .ks files are
  6. Execute bugdemo.ks
  7. Release focus to the console by clicking anywhere in the screen but the console

Tanks are denominated as A to D, from top to bottom. All tanks should be half full at the beginning of the test. The program attempts to transfer -50 units of fuel from tank A to tank D. Tank A should have an extra 50 units at the end, without draining any of the other tanks (tank D was the target tank).

The bug is not caused by excessive transfer, programming a lesser (not so) negative amount to be transferred will cause the same amount to appear at the source tank, even if we're transferring less than 45 units. The source tank may end up being "overfilled" due to the negative transfer if this negative transfer is too big.

Possible solutions could be to check the boundaries, preventing negative transfers or to transfer from destination to source if negative values are input (that would make me REALLY happy, as it would mean not having to deal with abs() and lots of conditionals)

Dunbaratu commented 3 years ago

This is one place where I get frustrated at how much of the KSP game's "universe rules" are implemented inside the game's GUI code instead of being in a deeper layer that the GUI code merely calls. That makes it a PITA for kOS to obey the universe's rules since it bypasses the GUI code.

kOS had to invent it's own resource transfer system since the stock one is inside the GUI code, and it looks like we didn't do a good enough job checking for negatives like this.

I agree that the best fix would be to make it so a negative amount isn't an error, but it just basically means you're just swapping source and destination around.