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 recent NewGRF spec addition to make it less surprising/arbitrary #8590

Merged
merged 1 commit into from Jan 18, 2021

Conversation

frosch123
Copy link
Member

Motivation / Problem

While adding recent NewGRF features to NML, I came across #8079.
To translate NewGRF jargon into English, it adds this footnote to the API:

When smooth economy is enabled in OpenTTD and the NewGRF provides no custom production-change mechanics, there is a special case:

  • If the industry is built on water, then the second output cargo is clamped to produce at most 16 units per production cycle.
  • This only applies to water-based industries, and only to the second output cargo.
  • If this behaviour is unwanted, NewGRFs have to disable it explicitly.

As a NewGRF author I would not expect the "second output cargo" to behave any different from other output cargos, just because my industry happens to be built on water.

From a NML perspective this flag could be considered as "enable work-around for OpenTTD bug", so NML could enable it silently by default.
However, it's both easier and likely better to give the flag some intention, and implement it with less arbitrary conditions.

Description

The original intention of the special case in OpenTTD seems to be the original oil rig.
The oil rig features passengers as "support cargo", which are not supposed to be produced en-mass.

This PR changes the behavior of the flag added in #8079:

  • The condition "second output cargo" is replaced with "passenger production".
  • The condition "water-based industry" is removed.

This behavior is easier to explain, and likely also fits better what a NewGRF industry may want to achieve.

The "passenger" condition only triggers for real passenger, not for "tourists" or other cargos with "passenger" cargo class.
Though industry sets that feature "worker mechanics" always have custom production mechanics, so that smooth economy and this flag does not apply anyway.

In summary:

  • The flag does no longer trigger for arbitrary NewGRF industries, that happen to be built on water.
  • The flag does not trigger for NewGRF with distinct worker or tourist mechanics.
  • The flag only triggers for NewGRF industries that behave similar to the original oil rig: when producing passengers "just like that", with no further production mechanics involved.

Limitations

N/A

Checklist for review

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

…ly passenger production, instead of the second cargo of any water-based industry.

This behavior is less surprising to NewGRF authors, and may even be intentional behavior for some industries.
@frosch123 frosch123 added the needs review: NewGRF Review requested from a NewGRF expert label Jan 18, 2021
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.

I'll be honest, I've not examined it in detail, but I tend to trust your judgement in these matters :)

@frosch123 frosch123 merged commit b3d048d into OpenTTD:master Jan 18, 2021
frosch123 added a commit to frosch123/nml that referenced this pull request Jan 30, 2021
FLHerne pushed a commit to OpenTTD/nml that referenced this pull request Jan 30, 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.

None yet

2 participants