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: Show the cargoes the vehicles can carry in the list window. #8304

Merged
merged 1 commit into from Nov 24, 2022

Conversation

stormcone
Copy link
Contributor

This patch is based on @KeldorKatarn's commit:
KeldorKatarn/OpenTTD_PatchPack@0a57222a

cargo

@James103
Copy link
Contributor

Should there be a setting to toggle the effects of this PR? Not all players may want this change as it may increase the clutter for some.

@TrueBrain
Copy link
Member

A setting for this would be the right call, yes, but it can be on by default as far as I am concerned.

You did mangle two patches here, it seems, as I think part of "Feature: Show group name as part of the default vehicle name." is also in here :)

@TrueBrain TrueBrain added waiting on author candidate: yes This Pull Request is a candidate for being merged size: trivial This Pull Request is trivial preview This PR is receiving preview builds and removed waiting on author preview This PR is receiving preview builds labels Dec 14, 2020
@TrueBrain TrueBrain added the preview This PR is receiving preview builds label Dec 15, 2020
@DorpsGek DorpsGek temporarily deployed to preview-pr-8304 December 15, 2020 20:17 Inactive
@TrueBrain
Copy link
Member

You did mangle two patches here, it seems, as I think part of "Feature: Show group name as part of the default vehicle name." is also in here :)

I fixed my issue by merging your other patch. A rebase solves the problem now :D

src/vehicle_gui.cpp Outdated Show resolved Hide resolved
src/vehicle_gui.cpp Outdated Show resolved Hide resolved
@DorpsGek DorpsGek temporarily deployed to preview-pr-8304 December 15, 2020 20:28 Inactive
@frosch123
Copy link
Member

Is there somewhere an explanation what this info is used for?

  • Usually all vehicles in a group carry the same cargo, unless they use refit orders, right?
  • Cloning vehicles copies refitting, so it's also unlikely to introduce inconsistencies?

I would see the usefulness of prepending groups names with the cargos of all member vehicles, but I don't understand why someone would want to see it per vehicle.

@stormcone
Copy link
Contributor Author

stormcone commented Dec 17, 2020

Its depend on how someone organizes groups or whether using groups at all. The vehicles can be grouped by engines, by stations they visit, etc. But may be it is useful in the "all group" group. For example if you want to see which trains make the lowest profits, you can also whether the cargo makes them not profitable, so maybe better to carry it by other vehicle types.

But maybe sometimes you just would like to see which vehicle carry what when you just browsing the vehicle list. :)

I admit it may be the 'most' useful in the "all group" group. :)

Edit:
It also shows the cargoes in the station and waypoint lists:
openttd_cargo_names

@2TallTyler
Copy link
Member

This seems like a useful tool for waypoint and station management, especially with NewGRFs with lots of cargos carried in similar vehicles (looking at you, Iron Horse + FIRS Steeltown).

It needs some padding between the text and the vehicle sprite. Default engines abut the text with no space in between.

padding

Also needs a rebase (hopefully the last one, because I'd like to approve this 😃 )

@DorpsGek DorpsGek temporarily deployed to preview-pr-8304 October 24, 2022 23:21 Inactive
@stormcone
Copy link
Contributor Author

It needs some padding between the text and the vehicle sprite. Default engines abut the text with no space in between.

Its using the same string as the vehicles' names and also depends on the sprites' offsets. (Give a name to a vehicle, and check it out it in the vehicle list.)

vehicle_name

So I would like to leave this to someone else with much more experience with padding, margins, and scaling. :)

@DorpsGek DorpsGek temporarily deployed to preview-pr-8304 October 24, 2022 23:34 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-8304 October 24, 2022 23:37 Inactive
@2TallTyler 2TallTyler added this to the 13.0 milestone Oct 27, 2022
@2TallTyler
Copy link
Member

@PeterN Any thoughts on the padding issue described above? Apparently this is an existing problem. Will #10114 fix it? 🙂

(Once that's solved, I'd like to approve this.)

@Arastais
Copy link
Contributor

Maybe out of the scope of this PR, but I think it'd be really nice if you could put the cargo list dynamically in the name with a string key. For example if you put Train 1 for {CARGO} in the name editor box for a vehicle it would show up everywhere else as "Train 1 for [Iron, Wood]". or maybe don't include the brackets and just make Train 1 for ({CARGO}) show up as "Train 1 for (Iron, Wood)"

2TallTyler
2TallTyler previously approved these changes Nov 10, 2022
Copy link
Member

@2TallTyler 2TallTyler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, the text abutting the vehicle sprite is a problem inherent to OpenGFX graphics with the default font. It happens outside this PR. (I use original TTD baseset which doesn't have this problem). That's not your problem to fix, and it looks like #10114 solves it anyway.

OpenGFX graphics and default font in current nightly:
nightly

Adding a custom cargo flag for player-entered strings is definitely out of scope. 🙂

As I explained above, I think this is a useful feature for managing busy networks with lots of cargos. I'd use it. (And those who don't like the clutter can simply disable it.)

src/lang/english.txt Outdated Show resolved Hide resolved
src/table/settings/gui_settings.ini Outdated Show resolved Hide resolved
@DorpsGek DorpsGek temporarily deployed to preview-pr-8304 November 13, 2022 00:48 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-8304 November 13, 2022 01:00 Inactive
Copy link
Member

@2TallTyler 2TallTyler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a vehicle doesn't have its own name or a group name, the cargos listed alone begin with a space. I wonder if the space can be appended by code instead of being hardcoded in the language file, so it can only be included if needed to separate name and cargos.

space

@DorpsGek DorpsGek temporarily deployed to preview-pr-8304 November 13, 2022 18:44 Inactive
@LordAro LordAro merged commit e29547a into OpenTTD:master Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
candidate: yes This Pull Request is a candidate for being merged preview This PR is receiving preview builds size: trivial This Pull Request is trivial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants