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

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.
@rubidium42 rubidium42 added the needs review: NewGRF Review requested from a NewGRF expert label Jul 22, 2021
Copy link
Member

@michicc michicc left a comment

Choose a reason for hiding this comment

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

Yes. For whatever it counts.

@rubidium42 rubidium42 merged commit d83647f into OpenTTD:master Aug 2, 2021
@rubidium42 rubidium42 deleted the pr9440 branch August 2, 2021 18:45
@rubidium42 rubidium42 added the backport requested This PR should be backport to current release (RC / stable) label Aug 2, 2021
@TrueBrain TrueBrain removed the backport requested This PR should be backport to current release (RC / stable) label Oct 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review: NewGRF Review requested from a NewGRF expert
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Inflation breaks negative cargo payments
3 participants