The International Simutrans Forum

 

Author Topic: [patch] Correct cargo list redundancy  (Read 598 times)

0 Members and 1 Guest are viewing this topic.

Offline Ranran jp

  • *
  • Posts: 412
  • Languages: ja
[patch] Correct cargo list redundancy
« on: May 09, 2019, 02:28:31 PM »
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? (´・ω・`)

Offline Matthew

  • *
  • Posts: 158
  • Languages: EN, some ZH, DE & SQ
Re: [patch] Correct cargo list redundancy
« Reply #1 on: May 09, 2019, 10:22:04 PM »
This looks like two really good ideas!  :D

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.

Offline wlindley us

  • Devotee
  • *
  • Posts: 962
    • Hacking for fun and profit since 1977
  • Languages: EN, DE
Re: [patch] Correct cargo list redundancy
« Reply #2 on: May 10, 2019, 05:15:04 PM »
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 C
would 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.

Offline Ves

  • Devotee
  • *
  • Posts: 1659
  • Languages: EN, SV, DK
Re: [patch] Correct cargo list redundancy
« Reply #3 on: May 10, 2019, 11:35:41 PM »
Thank you, that looks really good!  :D

Offline ACarlotti

  • *
  • Posts: 442
Re: [patch] Correct cargo list redundancy
« Reply #4 on: May 11, 2019, 02:39:21 AM »
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.

Offline jamespetts gb

  • Simutrans-Extended project coordinator
  • Moderator
  • *
  • Posts: 18587
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: [patch] Correct cargo list redundancy
« Reply #5 on: May 13, 2019, 11:17:08 PM »
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.

Offline Ranran jp

  • *
  • Posts: 412
  • Languages: ja
Re: [patch] Correct cargo list redundancy
« Reply #6 on: May 14, 2019, 12:57:29 PM »
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 C
would 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. :award: 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).

Offline jamespetts gb

  • Simutrans-Extended project coordinator
  • Moderator
  • *
  • Posts: 18587
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: [patch] Correct cargo list redundancy
« Reply #7 on: May 16, 2019, 10:24:11 PM »
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.

Offline Ranran jp

  • *
  • Posts: 412
  • Languages: ja
Re: [patch] Correct cargo list redundancy
« Reply #8 on: May 17, 2019, 04:18:28 PM »
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
« Last Edit: May 17, 2019, 04:59:34 PM by Ranran »

Offline jamespetts gb

  • Simutrans-Extended project coordinator
  • Moderator
  • *
  • Posts: 18587
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: [patch] Correct cargo list redundancy
« Reply #9 on: May 17, 2019, 10:35:25 PM »
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.

Offline Ranran jp

  • *
  • Posts: 412
  • Languages: ja
Re: [patch] Correct cargo list redundancy
« Reply #10 on: May 18, 2019, 03:57:26 AM »
I wonder whether you might look into this?
Thank you for pointing it out. I think I've fixed it now.

Offline jamespetts gb

  • Simutrans-Extended project coordinator
  • Moderator
  • *
  • Posts: 18587
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: [patch] Correct cargo list redundancy
« Reply #11 on: May 18, 2019, 09:25:01 AM »
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.