News:

Use the "Forum Search"
It may help you to find anything in the forum ;).

[patch] Correct cargo list redundancy

Started by Ranran(retired), May 09, 2019, 02:28:31 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

Ranran(retired)

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? (´・ω・`)
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

Matthew

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.
(Signature being tested) If you enjoy playing Simutrans, then you might also enjoy watching Japan Railway Journal
Available in English and simplified Chinese
如果您喜欢玩Simutrans的话,那么说不定就想看《日本铁路之旅》(英语也有简体中文字幕)。

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:
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:
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


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.

Quote from: wlindley on May 10, 2019, 05:15:04 PMListing 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.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ranran(retired)

Thank you for your feedbacks.  :)

Quote from: ACarlotti on May 11, 2019, 02:39:21 AMOne 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 {}).
Quote from: jamespetts on May 13, 2019, 11:17:08 PMOne 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.


Quote from: wlindley on May 10, 2019, 05:15:04 PMI wonder whether you would agree that sorting by "Via (amount)" which currently gives results like:

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:

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.
Quote from: ACarlotti on May 11, 2019, 02:39:21 AMI 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).
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

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.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ranran(retired)

#8
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
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

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.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ranran(retired)

Quote from: jamespetts on May 17, 2019, 10:35:25 PMI wonder whether you might look into this?
Thank you for pointing it out. I think I've fixed it now.
ひめしという日本人が開発者達の助言を無視して自分好みの機能をextendedに"強引に"実装し、
コードをぐちゃぐちゃにしてメンテナンスを困難にし(とりわけ道路と建物関連)、
挙句にバグを大量に埋め込み、それを知らんぷりして放置し(隠居するなどと言って)別のところに逃げ隠れて自分のフォーク(OTRP)は開発を続けている
その事実と彼の無責任さに日本人プレイヤーは目を向けるべき。らんらんはそれでやる気をなくした(´・ω・`)
他人の振り見て我が振り直せ。ひめしのようにならないために、らんらんが生み出したバグや問題は自分で修正しなくちゃね(´・ω・`)

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.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.