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 #209: Cargo 'profit' callback applied a unit conversion, when there was no unit. #220

Merged
merged 1 commit into from Oct 10, 2021

Conversation

frosch123
Copy link
Member

@frosch123 frosch123 commented Jun 5, 2021

This is an alternative to #212 .

The documentation for the callback and the comments in NML are very technical.

    The 'profit' callback in NML was supposed to be used like this:
      cargo_income = callback_result * cargo_amount * price_factor
    But since NML defines 'price_factor' as income per 10 units over 20 tiles without time-penalty (=255/256), this definition results in a very weird meaning of 'callback_result'.
    NML applied some unit conversion to the callback result, which was supposed to revert this 20*10*255/256 scaling, but this only made it harder to use.

    By removing the unit conversion in the callback, the 'callback_result' can now be defined as relative scaling of the 'price_factor'.
    To do something similar as the default-payment, you can now use
      callback_result = distance * time_penalty
    with 'time_penalty' in range 0..1.

Closed #212

@frosch123
Copy link
Member Author

I checked all NewGRF hosted on devzone. None of them use this callback.

…hen there was no unit.

The 'profit' callback in NML was supposed to be used like this:
  cargo_income = callback_result * cargo_amount * price_factor
But since NML defines 'price_factor' as income per 10 units over 20 tiles without time-penalty (=255/256), this definition results in a very weird meaning of 'callback_result'.
NML applied some unit conversion to the callback result, which was supposed to revert this 20*10*255/256 scaling, but this only made it harder to use.

By removing the unit conversion in the callback, the 'callback_result' can now be defined as relative scaling of the 'price_factor'.
To do something similar as the default-payment, you can now use
  callback_result = distance * time_penalty
with 'time_penalty' in range 0..1.
@PeterN
Copy link
Member

PeterN commented Jun 5, 2021

Seems like the right thing to do to me.

@glx22 glx22 mentioned this pull request Jul 2, 2021
@andythenorth
Copy link
Contributor

Per #193 (comment) Tyler intends to use cargo_profit cb.

@2TallTyler
Copy link
Member

I'd like to start using the cargo_profit callback in Industries of the Caribbean. If I fork NML and apply this commit, will negative values still be calculated properly? (not requiring #212?)

Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

#YOLO

@glx22 glx22 merged commit 870a6d9 into OpenTTD:master Oct 10, 2021
@2TallTyler
Copy link
Member

\o/

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.

None yet

6 participants