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 for -10% and +10% buttons in station's lobby #5628

Merged
merged 1 commit into from
Sep 13, 2023

Conversation

Max5377
Copy link
Contributor

@Max5377 Max5377 commented Sep 13, 2023

Fixes #5585

The problem was with this line in refuelInternalTank in data/pigui/modules/station-view/01-lobby.lua:

station:AddCommodityStock(Commodities.hydrogen, -math.ceil(mass))

math.ceil is rounding number up, so if this number is negative(ex. -2.3) it will be rounded to -2 because it is nearest value that greater or equal to -2.3. So the solution is to change above mentioned line to:

local commodityChangeAmount = mass < 0 and math.floor(mass) or math.ceil(mass) station:AddCommodityStock(Commodities.hydrogen, commodityChangeAmount)

Accepted answer by Guffa(albeit it's for javascript, but works the same way)

@impaktor
Copy link
Member

Looking good

Just change

Fix for #5585

to

Fixes #5585

and it will auto-close the issue when we merge this code.

@Max5377
Copy link
Contributor Author

Max5377 commented Sep 13, 2023

@impaktor Done

@Web-eWorks Web-eWorks merged commit 9402808 into pioneerspacesim:master Sep 13, 2023
@Max5377
Copy link
Contributor Author

Max5377 commented Sep 14, 2023

Upon futher testing, I can say that my solution is not good either.
In my code, the station will receive more hydrogen than needed, because number is just rounded up, leaving fractional parts behind. Example:
Coronatrix fuel tank: 23t
10% from this amount: 2.3t

math.floor(-2.3) = -3, math.ceil(2.3) = 3

In the original code, it would be a little different:

math.ceil(-2.3) = -2, math.ceil(2.3) = 3

So, if there are no fractions in the goods, I don't think this can be correctly counted without them. That's what I think, at least.

Also, in this line:

station:AddCommodityStock(Commodities.hydrogen, commodityChangeAmount)

I forgot to add minus to commodityChangeAmount, because of this, demand and in stock decrease is mixed up (-10% decreases in stock and +10% decreases demand).

@impaktor
Copy link
Member

impaktor commented Sep 14, 2023

@Max5377 I suggest you start a new branch & PR fixing the minus, and any other relevant issue as you've described.

@Max5377
Copy link
Contributor Author

Max5377 commented Sep 14, 2023

@impaktor I will definitely fix the minus part. As for the other thing, I don't know how to fix that, without fractional parts in the goods.

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.

Incorrect substraction from in stock or demand when using +10% or -10% buttons from station's lobby
3 participants