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 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 label Dec 25, 2018
@nielsmh

This comment has been minimized.

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 nielsmh:fix-fund-industry branch from 1942e1f to db54cb2 Dec 27, 2018
@nielsmh nielsmh force-pushed the nielsmh:fix-fund-industry branch 2 times, most recently from 1ec4271 to 9800ac2 Jan 5, 2019
@nielsmh nielsmh added needs review and removed wip labels Jan 5, 2019
@frosch123

This comment has been minimized.

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
nielsmh added 2 commits Dec 25, 2018
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 force-pushed the nielsmh:fix-fund-industry branch from 9800ac2 to a615db9 Jan 6, 2019
@nielsmh nielsmh merged commit f37304f into OpenTTD:master Jan 19, 2019
1 check passed
1 check passed
OpenTTD CI Build #20190106.12 succeeded
Details
@nielsmh nielsmh deleted the nielsmh:fix-fund-industry branch Jan 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.