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 #7631: 16 out cargo support for industry directory #7809

Merged
merged 2 commits into from Nov 13, 2019

Conversation

@glx22
Copy link
Contributor

glx22 commented Oct 28, 2019

industry_directory

It's just a basic change. The window may need to be rewritten.
English.txt needs to be cleaned up

@James103
Copy link
Contributor

James103 commented Oct 29, 2019

Would it be possible to implement one of the following systems instead of just going to cargo sprites and hiding a bunch of information from players? This will retain more information in the industry directory window.

Plan A:
1-4 cargoes: Display the full cargo produced and transported ("100 tons of coal (50% transported)")
5-9 cargoes: Display only the cargo produced and the amount ("100 tons of coal, 200 tons of iron")
10+ cargoes: Display only the name of the cargo produced ("Coal, Iron, Gold, and Diamonds")

Plan B:
Cargoes 1-4: Display the full info regarding cargo produced and transported
Cargoes 5+: Collapse with a "...", "etc.", or "and more". Open industry window to view all cargoes there.
Example: "100 tons of coal, 200 tons of iron, 300 tons of gold, 400 tons of diamonds, and more"

@ldpl
Copy link
Contributor

ldpl commented Oct 29, 2019

IMO this change makes industry window completely pointless as amount of produced cargo was the only useful info there. Also default cargo icons are terrible in this context. They're too small and not very recognizable.

@LordAro
Copy link
Member

LordAro commented Oct 29, 2019

Why? Because currently the window hard crashes the game if your industry has more than 7 cargos produced/accepted.

I do agree that this isn't the correct solution though...

@andythenorth
Copy link
Contributor

andythenorth commented Nov 2, 2019

Could we do this:

  • sort cargos by amount produced, highest > lowest
  • take the first n (up to the limit of string params)
  • append 'and x more...'

??

The rationale is that it's high production amounts that are looked for in this window.

@glx22
Copy link
Contributor Author

glx22 commented Nov 3, 2019

So something like Unnamed, 15th Jan 1950 would be preferred.

@andythenorth
Copy link
Contributor

andythenorth commented Nov 3, 2019

Given that we have a hard-crash currently, I think that looks like a pretty good fix 👍

Long-term, maybe we should change string handling so we can show all the cargos, but that also raises UI questions (horizontal overflow). Let's fix the immediate crash.

@glx22 glx22 force-pushed the glx22:industry_directory branch from d42620d to 2deb22d Nov 3, 2019
@glx22 glx22 changed the title Fix #7631: replace produced&transported with cargo sprites in industry directory Fix #7631: 16 out cargo support for industry directory Nov 3, 2019
@glx22 glx22 marked this pull request as ready for review Nov 3, 2019
src/industry_gui.cpp Outdated Show resolved Hide resolved
src/industry_gui.cpp Outdated Show resolved Hide resolved
@glx22 glx22 force-pushed the glx22:industry_directory branch from 2deb22d to 198c8f9 Nov 4, 2019
@glx22
Copy link
Contributor Author

glx22 commented Nov 4, 2019

Oups I should have checked with mingw before pushing

@glx22 glx22 force-pushed the glx22:industry_directory branch from 198c8f9 to b0cabae Nov 4, 2019
@LordAro LordAro merged commit 9fc6329 into OpenTTD:master Nov 13, 2019
8 checks passed
8 checks passed
OpenTTD CI Build #20191104.3 succeeded
Details
OpenTTD CI (Linux commit-checker) Linux commit-checker succeeded
Details
OpenTTD CI (Linux linux-amd64-clang-3.9) Linux linux-amd64-clang-3.9 succeeded
Details
OpenTTD CI (Linux linux-amd64-gcc-6) Linux linux-amd64-gcc-6 succeeded
Details
OpenTTD CI (Linux linux-i386-gcc-6) Linux linux-i386-gcc-6 succeeded
Details
OpenTTD CI (MacOS) MacOS succeeded
Details
OpenTTD CI (Windows Win32) Windows Win32 succeeded
Details
OpenTTD CI (Windows Win64) Windows Win64 succeeded
Details
@glx22 glx22 deleted the glx22:industry_directory branch Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.