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 #9440: negative cargo payments not being handled right #9455

Merged
merged 1 commit into from Aug 2, 2021

Conversation

@rubidium42
Copy link
Contributor

@rubidium42 rubidium42 commented Jul 22, 2021

Motivation / Problem

Fixes #9440; negative cargo payments causing huge payments.

Description

Cargo payments were stored as unsigned integer, but cast to int64 during
application of inflation. However, then being multiplied with a uint64
making the result uint64. So in the end the payment that should have been
negative becomes hugely positive.

Limitations

First and major question is whether it is wanted that cargo payments can be negative. The specifications do not seem to be very clear about it, though the custom cargo calculation allowing negative factors might imply that the cargo payments themselves can be negative.

So, this makes it possible and seemingly work for the graphs and actual payment, but I'm far from certain it should be this way. So closing this PR and #9440 as not-a-bug would be a valid solution as well.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')
Cargo payments were stored as unsigned integer, but cast to int64 during
application of inflation. However, then being multiplied with a uint64
making the result uint64. So in the end the payment that should have been
negative becomes hugely positive.
michicc
michicc approved these changes Aug 1, 2021
Copy link
Member

@michicc michicc left a comment

Yes. For whatever it counts.

Loading

@rubidium42 rubidium42 merged commit d83647f into OpenTTD:master Aug 2, 2021
15 checks passed
Loading
@rubidium42 rubidium42 deleted the pr9440 branch Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants