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: Correct display of industry requires/produces in Build Industry window #6990

Merged
merged 2 commits into from Jan 19, 2019

Conversation

nielsmh
Copy link
Contributor

@nielsmh nielsmh commented Dec 25, 2018

During development of the patch for 16 cargoes in/out I forgot to look at the Build Industry window and didn't notice it fails showing the required/produced cargoes correctly for 4+ in/3+ out.

This is an attempt at fixing this. It's not very idiomatic, in that it uses partial strings and concatenates in code. The alternative would be to have 16 strings for "Requires: list of cargoes" and 16 stings for "Produces: list of cargoes", one for each number possible.

I attempt to support correct pluralization of the Requires/Produces at the beginning, but it has the unfortunate side effect of the number of items also being printed. A solution is needed for this.

image

To do:

  • Fix pluralization number being printed
  • Allow Requires/Produces lines to break, to prevent overly large minimum window sizes
  • Remove old strings after checking they are now unused

@nielsmh nielsmh added the wip Work in progress. Feature branch that will require feedback during the development process label Dec 25, 2018
@nielsmh
Copy link
Contributor Author

nielsmh commented Dec 25, 2018

Another case that probably needs attention is that the minimum window size is measured to fit the longest cargo list line, in other words, neither of the Requires nor Produces lines can ever break. An industry with many ins/outs could make the entire window unreasonably large. It might be an idea to impose a max for the minimum window size, such that sufficiently long lines can be broken.

@nielsmh nielsmh force-pushed the fix-fund-industry branch 2 times, most recently from 1ec4271 to 9800ac2 Compare January 5, 2019 22:13
@nielsmh nielsmh added needs review and removed wip Work in progress. Feature branch that will require feedback during the development process labels Jan 5, 2019
@frosch123
Copy link
Member

frosch123 commented Jan 6, 2019

Are you aware of {CARGO_LIST}?
That makes it a lot easier.

Edit: Never mind, that does not deal with the cargo suffix.

src/industry_gui.cpp Outdated Show resolved Hide resolved
src/industry_gui.cpp Outdated Show resolved Hide resolved
Change the window to use a dynamically generated string of cargoes,
instead of one of a few fixed-length lists. With up to 16 cargoes
on each list, having 16 with the only difference how many are listed
seems like a bad maintenance idea.
@nielsmh nielsmh merged commit f37304f into OpenTTD:master Jan 19, 2019
@nielsmh nielsmh deleted the fix-fund-industry branch January 19, 2019 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants