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

Feature: Industry production graph #7575

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

@kiwitreekor
Copy link
Contributor

@kiwitreekor kiwitreekor commented May 7, 2019

  • This patch adds a window showing industry production history in last 24 months. (which already exists in Chris Sawyer's Locomotion)
    production_history
src/graph_gui.cpp Outdated Show resolved Hide resolved
src/industry_cmd.cpp Outdated Show resolved Hide resolved
src/widgets/graph_widget.h Show resolved Hide resolved
@nielsmh
Copy link
Contributor

@nielsmh nielsmh commented May 7, 2019

Commit checker found some whitespace problems:

*** b/src/graph_gui.cpp:1667: Trailing whitespace: '					'
*** b/src/graph_gui.cpp:1750: Trailing whitespace: '			'
*** b/src/graph_gui.cpp:1752: Trailing whitespace: '			'

@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch 2 times, most recently from 889a104 to af3332c May 8, 2019
@kiwitreekor
Copy link
Contributor Author

@kiwitreekor kiwitreekor commented May 8, 2019

Commit checker found some whitespace problems:

*** b/src/graph_gui.cpp:1667: Trailing whitespace: '					'
*** b/src/graph_gui.cpp:1750: Trailing whitespace: '			'
*** b/src/graph_gui.cpp:1752: Trailing whitespace: '			'

Should be fixed in 889a104.

@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch from af3332c to 966b7cd May 8, 2019
src/industry_gui.cpp Outdated Show resolved Hide resolved
src/saveload/saveload.h Outdated Show resolved Hide resolved
@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch from 966b7cd to ee7478d May 9, 2019
src/saveload/saveload.h Outdated Show resolved Hide resolved
@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch from ee7478d to a1f6475 May 9, 2019
@LordAro LordAro dismissed PeterN’s stale review May 9, 2019

resolved

@nielsmh
Copy link
Contributor

@nielsmh nielsmh commented May 11, 2019

This needs to be updated as the savegame version as been bumped.

@PeterN
Copy link
Member

@PeterN PeterN commented May 11, 2019

Currently display units for weight and volume are ignored, which is not ideal. This will also affect how the graph scales, and I'm not sure how it should handle oil rigs which have passengers tens and oil in the tens of thousands of litres.

@PeterN
Copy link
Member

@PeterN PeterN commented May 11, 2019

Another issue, graphs are not being redrawn properly on new a month, leading to visual glitches.

@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch from a1f6475 to d7eea7d May 11, 2019
@kiwitreekor
Copy link
Contributor Author

@kiwitreekor kiwitreekor commented May 11, 2019

This needs to be updated as the savegame version as been bumped.

Now rebased to e7f6f07.

@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch from d7eea7d to 754eb30 May 11, 2019
@kiwitreekor
Copy link
Contributor Author

@kiwitreekor kiwitreekor commented May 11, 2019

Another issue, graphs are not being redrawn properly on new a month, leading to visual glitches.

Now graphs are set dirty in UpdateStatisitics. This may resolve the issue.

@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch from 754eb30 to 400135d May 11, 2019
@kiwitreekor
Copy link
Contributor Author

@kiwitreekor kiwitreekor commented May 12, 2019

Currently display units for weight and volume are ignored, which is not ideal. This will also affect how the graph scales, and I'm not sure how it should handle oil rigs which have passengers tens and oil in the tens of thousands of litres.

hmm... maybe should I limit the number of showing cargoes to one at once?

@PeterN
Copy link
Member

@PeterN PeterN commented May 12, 2019

I don't think that's useful. The player can do that themselves with the filter anyway.

@kiwitreekor
Copy link
Contributor Author

@kiwitreekor kiwitreekor commented May 30, 2019

bump?

src/graph_gui.cpp Outdated Show resolved Hide resolved
src/graph_gui.cpp Outdated Show resolved Hide resolved
@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch from 400135d to bb5d93f Jun 16, 2019
@kiwitreekor
Copy link
Contributor Author

@kiwitreekor kiwitreekor commented Jun 16, 2019

Sorry for late reply...

@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch from bb5d93f to 69a95a5 Jun 16, 2019
@kiwitreekor
Copy link
Contributor Author

@kiwitreekor kiwitreekor commented Jul 23, 2019

bump

@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch 2 times, most recently from 30299e5 to d470426 Sep 9, 2019
@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch 3 times, most recently from 15238d6 to 8d210a9 Dec 19, 2019
@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch from 8d210a9 to fa63c95 Jan 12, 2020
src/lang/english.txt Outdated Show resolved Hide resolved
src/widgets/graph_widget.h Outdated Show resolved Hide resolved
src/graph_gui.cpp Outdated Show resolved Hide resolved
src/graph_gui.cpp Outdated Show resolved Hide resolved
src/graph_gui.cpp Outdated Show resolved Hide resolved
src/graph_gui.cpp Outdated Show resolved Hide resolved
@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch from fa63c95 to e5a2112 Jan 21, 2020
@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch from e5a2112 to 02a0327 Apr 25, 2020
@kiwitreekor kiwitreekor changed the title Feature: Add industry production graph Feature: ㅑndustry production graph Apr 25, 2020
@kiwitreekor kiwitreekor changed the title Feature: ㅑndustry production graph Feature: Industry production graph Apr 25, 2020
@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch from 02a0327 to c18c755 Apr 25, 2020
@kiwitreekor kiwitreekor requested a review from LordAro Apr 25, 2020
@LordAro
Copy link
Member

@LordAro LordAro commented Sep 24, 2020

I'm not sure about the concept of adding this. The graph ultimately serves no purpose, as you've had to add the history to the game in the first place. I guess it might make more sense in Locomotion if industry production does somehow depend on past months, but that's not the case here.

@kiwitreekor
Copy link
Contributor Author

@kiwitreekor kiwitreekor commented Sep 26, 2020

@LordAro I think this will be useful when using industry NewGRF such as FIRS. It will help to check that production is being carried out properly at industries, where production depends on the amount of engineering supplies shipped.

@andythenorth
Copy link
Contributor

@andythenorth andythenorth commented Dec 25, 2020

If there are no performance concerns about this, then I am +1 to it.

Seems like a straightforward addition. The UI is fine. Feature doesn't seem to limit any future possibilities.

@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch from c18c755 to dce455f Dec 29, 2020
@kiwitreekor
Copy link
Contributor Author

@kiwitreekor kiwitreekor commented Dec 29, 2020

I rebased this PR to latest master.

Copy link
Member

@LordAro LordAro left a comment

Needs a rebase, but in principle I have no issues with this

@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch 2 times, most recently from 211e96f to ba30c93 Feb 27, 2021
@kiwitreekor
Copy link
Contributor Author

@kiwitreekor kiwitreekor commented Feb 27, 2021

rebased to latest master

@PeterN
Copy link
Member

@PeterN PeterN commented Apr 26, 2021

OpenTTD graph windows have changed background colour since this was created, I guess it would make sense to do the same here.

@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch from ba30c93 to b0c5929 Apr 27, 2021
@kiwitreekor kiwitreekor force-pushed the industry-production-graph branch from b0c5929 to 4dd7f62 Apr 27, 2021
@kiwitreekor
Copy link
Contributor Author

@kiwitreekor kiwitreekor commented Apr 27, 2021

Thanks for mentioning it.

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

8 participants