Topic: [patch] Correct cargo list redundancy

[Ranran]

Hi. Ranran made a small patch. (´・ω・｀)

The cargo list is currently cluttered by unnecessarily repeating the same word.
This patch reduces duplicate text, reduces the amount of text, and makes the list slightly sexy slimmer.

Examples:
The left side is after the change and the right side is before the change.

For example, everyone knows that everything grouped by passengers is passengers.





However, in some cases, it can subdivide passengineers further in Extended. In that case, it displays like this.






The freight goods display is as it is.


The repository for this patch is here.
https://github.com/Ranran-the-JuicyPork/simutrans-extended/tree/slim-the-cargo-list


What do you guys think about this? (´・ω・｀)

[Matthew]

This looks like two really good ideas! 

Firstly, saving 'screen estate' (space on the screen) by not using the word "passengers" unnecessarily.

Secondly, making it possible for players to distinguish Commuters from Visitors.

IMHO it would definitely be an improvement.

[wlindley]

Excellent!  While you are looking at those bits of code, I wonder whether you would agree that sorting by "Via (amount)" which currently gives results like:

Code: [Select]

30 via A
25 to B
20 to C
15 via B
5 via Cwould be more useful if it told us how many passengers wanted to travel either to or via, like this:

Code: [Select]

40 via B (25 to, 15 via)
30 via A
25 via C (20 to, 5 via)especially as that is far more useful in determining where to add service.  Listing the "to" and "via" separately seems quite confusing, don't you think?
I'm not sure whether that would be simple or complex to change.

[Ves]

Thank you, that looks really good! 

[ACarlotti]

One specific comment: In lines 577-584, you should replace the current construction (if (a&&!b) {} else if (a&&b) {} else {}) with nested ifs (i.e. if (a) { if (b) {} else {}} else {}).

Another issue I've noticed is that the code ignores that some languages have different pluralisation schemes. However, I suspect this is probably a widespread existing problem, and therefore beyond the scope of these changes.

Listing the "to" and "via" separately seems quite confusing, don't you think?
I'm not sure whether that would be simple or complex to change.

I think it would be relatively complicated to combine them while still showing the separate numbers, but relatively simple to combine them with just a single total.

[jamespetts]

Thank you for this - this is definitely an improvement.

One thing that I do notice when testing, however, is that, in "wealth (detail)" and "destination (detail)" modes, when passengers/mail/goods are going to a destination that is not in a town, an empty pair of brackets are displayed where the town name would usually go. Are you able to recode this so that the brackets show only when the town is non-null?

Thank you again.

[Ranran]

Thank you for your feedbacks. 

One specific comment: In lines 577-584, you should replace the current construction (if (a&&!b) {} else if (a&&b) {} else {}) with nested ifs (i.e. if (a) { if (b) {} else {}} else {}).

One thing that I do notice when testing, however, is that, in "wealth (detail)" and "destination (detail)" modes, when passengers/mail/goods are going to a destination that is not in a town, an empty pair of brackets are displayed where the town name would usually go.

Thank you for pointing it out. I think they were the same issue. I think now I fixed it.

I wonder whether you would agree that sorting by "Via (amount)" which currently gives results like:

Code: [Select]

30 via A
25 to B
20 to C
15 via B
5 via Cwould be more useful if it told us how many passengers wanted to travel either to or via, like this:

Code: [Select]

40 via B (25 to, 15 via)
30 via A
25 via C (20 to, 5 via)especially as that is far more useful in determining where to add service.  Listing the "to" and "via" separately seems quite confusing, don't you think?
I'm not sure whether that would be simple or complex to change.

I think it would be relatively complicated to combine them while still showing the separate numbers, but relatively simple to combine them with just a single total.

I agree with wlindley's idea. I have integrated these based on the second proposal from ACarlotti.
Please check the following comparison picture.



It is very useful to know where convoy passengers get off and where passengers waiting at the station are getting off.
IMO, I don't think it's necessary to divide this into via and to in this case. If player want to distinguish between "via" and "to", via (detail) will let you know, so I don't think there is any problem with integrating them in via (amount).

[jamespetts]

Thank you very much for this: this is good progress.

Two remaining things: first of all, in "Wealth (via"), we still have occasions when we get "> via", which is redundant. Relatedly, all the instances of "via" have been replaced with ">"; might it not be clearer to have "via" instead of ">" in the "via" modes? Then, we have ">" for "to", "<" for "from" and "via".

Thank you again for your work on this.

[Ranran]

I made some corrections. Is this adjustment in line with your requirements?

EDIT:
Currently, the wealth(via) is sorted by the name of the stop, but I thought it would be more convenient to order by the amount like this.


This change can be found in the branch below.
https://github.com/Ranran-the-JuicyPork/simutrans-extended/tree/slim-the-cargo-list2

[jamespetts]

Thank you very much for this. I do prefer the amount sorting for wealth (via), so I have included this on the cargo-list-improvements branch.

However, I am noticing some problems with via (amount): this seems to be showing the destination rather than the next transfer in at least some cases. I wonder whether you might look into this?

Thank you again for your work on this.

[Ranran]

I wonder whether you might look into this?

Thank you for pointing it out. I think I've fixed it now.

[jamespetts]

Excellent, thank you very much: I have now confirmed that this fix works.

This has now been incorporated into the master branch, so will be available from the next nightly build. Thank you very much for your work on this: it is much appreciated.