The International Simutrans Forum

Simutrans Extended => Simutrans-Extended development => Patches/pull requests for consideration => Topic started by: Ranran on July 16, 2022, 08:24:16 AM

Title: PR#561 - Overhaul of depot and replace dialog
Post by: Ranran on July 16, 2022, 08:24:16 AM
Happy rainy day guys. (´・ω・`)

Now you can  test the scalable new depot and replace dialogs.  :arrow:
https://github.com/Ranran-the-JuicyPork/simutrans-extended/tree/2207-depot-veh-sorting

This is a patch intended to be merged into the master branch, but a required patch for EX-15.

These UIs are much older and are causing more anomalies due to the accumulated diffs with the new drawing system.  ???  It is somthing like the same as the vehicle is in the obsolete state in simutrans-extended, so refactoring or updating required...
As mentioned before, it is difficult to proceed with adding new parameters in EX-15 without incorporating this. Because the depot dialog is already broken and difficult to repair, ad hoc deception and the task of adding something can be a lot of detours.
Overhauling them will bring them back to sanity and make these GUIs updatable in future Ex-15.
These are important dialogs in the game and should be carefully checked for any anomalies.

Layout changes are often either inseparable components as described before or efforts to prevent the dialog size from becoming too large.
For example, the top of the depot dialog is a depot frame that has the depot status. The bottom is the vehicle selector. It is a component that displays a list, and putting the state of the depot there is wasteful and laborious. In extended, the lower component (convoy assembler) is also used in the replace dialog, so it has such a configuration, which is the difference from standard and the reason why it cannot be the same layout as standard.

Simutrans's new UI automatically adjusts the size and enlarges the dialog if it's cramped. The extended depot dialog, which is overloaded with a lot of information, is very bloated, so we need to take measures against this.

The option below the vehicle list tends to push it off the screen and is inconsistent with other list dialogs. So I moved it up the vehicle list (tabs). The filters are collected in the upper right.

Basically, the function aims to maintain the status quo. Major design changes will be made in EX-15. Since more parameters will be added in EX-15, it will need to be changed. No changes have been made for that purpose at this time.

On the other hand, there are new features brought from Standard by updating these GUI. ;)
They are vehicle sorting and text search. (However, at the moment, the sort function is being adjusted and is a little broken.)
(This code should also be used for the consistent order.)
A new icon is added to the theme to indicate that the text box is a text search. Please update the theme before testing.

And finally the depot dialog and replace dialog now save the opened state.



Changes related to vehicle selector:
- The vehicle bar legend has moved down. This is because there was no space to display above. In the new UI system has the issue of expanding the upper space when placed it on above.

- A color bar legend has been added. It was added to take advantage of the large dead space created by the need to reserve space at the bottom for when the vehicle specs are displayed (otherwise the dialog would be enlarged later and destroy the layout).

- The vehicle's livery schemes display has been removed. It has no space to display. Check it out in convoy detail.


Changes related to depot dialog:
- You can now give your depot a unique name. The text input field has been placed at the top.
(https://i.imgur.com/QQj2nfa.png)
This was made possible with the help of James. Thank you very much. :hat:
The revision number increases for this change.

- The capacity of the convoy is now graphical and supports the display of 5 or more lines.
- The icon indicating that the depot is electrified has been restored. This has been lost for a long time in extended.


Changes related to replace dialog:
Fixed an issue where the order was broken on a reversed convoy. However, this fix may not work properly with EX-15. This is because, as already explained, there is a possibility that the reverse state of convoy and the reverse state of vehicle do not match.

- The replacement target has been changed from a checkbox to a combo box. This is intended to distinguish the button that means the target from the checkboxes that are mixed with, and the countermeasures for the bloatation of this dialog.

- The overlapping labels have been fixed.
- A reset button has been added. Previously, it had to be cleared, then closed and reopened.
- The assigned line display and access buttons to the schedule list and convoy detail have been added.


Changes in other dialogs:
- Added home depot sorting to the convoy list window.
- Added depot name sort to the depot list window.


The following translation terms are added.
hlptxt_vh_normal_state
need to connect vehicle
out of production
become obsolete
cannot select
lack of money
axle weight is excessive
appears only in upgrades
cannot upgrade
Only this convoy



I would appreciate any feedback on this. Thank you in advance. (´・ω・`)
Title: Re: Overhaul of depot and replace dialog
Post by: Ranran on July 16, 2022, 02:30:38 PM
Unfortunately this patch seems to have a big problem. But I don't know the solution.
An unusual load occurs when opening a different depot. If this anomaly occurs, the size of the depot dialog is vertically long.
However, for some reason, this anomaly cannot be reproduced when building with MSVC on my PC. You can reproduce this by testing what was generated by github action.
This anomaly does not seem to occur in the replace dialog.
I would be grateful if someone could help me in this regard.


EDIT:
Also, it seems that this anomaly can be avoided by some sort of trigger.
Once evaded, it will no longer occur, but reboot the game it will continue to occur again until unknown trigger changes. But in any case, it cannot be reproduced in my self-build version...

EDIT2:
The first way to avoid this anomaly may be to open two depot window at the same time.


EDIT3:
Fortunately, I think I was able to fix this problem.
Title: Re: Overhaul of depot and replace dialog
Post by: Spenk009 on July 17, 2022, 10:50:07 AM
I think this is going to make life a lot easier for new players and old players alike. The addition of reset and clear in the replace dialog is good and I like that you are keeping the original convoy visible to the player.

If you hover over any vehicle, at the bottom of the window you see its details. Could you also enable this for the original vehicle at the very top of the window? (see attachment)
Also, possibly unrelated but sorting for Price sorts the vehicles by name. The other sorting types work.


Thank you for the changes, they look really good and are really functional too.
Title: Re: PR#561 - Overhaul of depot and replace dialog
Post by: Ranran on July 18, 2022, 10:22:16 AM
The patch has been somewhat metamorphosed by several days of roting. In Japan, things rot easily due to humidity.

- A sort order button has been added.
- A range sort has been added.
- The sorting algorithm has been improved and is now sane.
The default sort now sorts in class order.



I think I've done a series of work, so I pulled this to the master branch. Please check pull request #561.


Quote from: Spenk009 on July 17, 2022, 10:50:07 AMsorting for Price sorts the vehicles by name. The other sorting types work.
Thank you for pointing it out. I believe I have fixed it.

QuoteIf you hover over any vehicle, at the bottom of the window you see its details. Could you also enable this for the original vehicle at the very top of the window?
Thank you for your feedback. Unfortunately it is difficult.
Firstly, the above image list is a list of image data, not a list of vehicle data. In other words, it only displays the picture and does not have the vehicle name or spec data.

This is different from the display in depot that it is not the image of the current convoy because it is sorted to the state regardless of whether the convoy is currently reversed. If it shows the current convoy order, we can easily get the data by check the car order, but it's not.

In the current master branch, trying to replace a convoy in the reversing state will generate such a broken vehicle order.
(https://i.imgur.com/F1HU5Yu.png)
However this patch avoids it and always presents a constant vehicle order.
Therefore, the current order of images and convoy may be inconsistent.

Secondly, the top vehicle images is located in the replace frame, while the images of other vehicles are located in another component called the convoy assembler. These are easy to work with, as specs display is also located in the convoy assembler.
It is not easy to resolve the contradiction caused by the simultaneous command to turn off the spec display when the mouse is released from the upper panel and to display the status when the cursor is placed on the lower panel...
Each one is monitoring the position of the mouse, and the replace frame commands the spec display to be turned off, but the convoy assembler commands the spec display to be displayed, and conflicts
The first problem can be solved by adding a vector that stores the order of the vehicles, but I don't have any idea to solve the second problem.

Title: Re: PR#561 - Overhaul of depot and replace dialog
Post by: Spenk009 on July 23, 2022, 05:54:55 PM
Thank you very much for the fixes and for explaining why we can't change the top. I'm really looking forward to this change, especially the sorting function!
Title: Re: PR#561 - Overhaul of depot and replace dialog
Post by: jamespetts on July 24, 2022, 12:06:39 AM
Excellent, thank you for this. Apologies for not having had the time to review this hitherto. I set out feedback in a numbered list below.



Thank you again very much for your work on this - this is very helpful.
Title: Re: PR#561 - Overhaul of depot and replace dialog
Post by: jamespetts on July 24, 2022, 11:23:49 AM
Incidentally, Matthew is currently having trouble accessing the forums. He has left extensive feedback on Github here (https://github.com/jamespetts/simutrans-extended/pull/561#issuecomment-1188809575).
Title: Re: PR#561 - Overhaul of depot and replace dialog
Post by: Ranran on August 06, 2022, 02:17:17 PM
QuoteI set out feedback in a numbered list below.
1,5,8,9,10,11,15,16,17,18 have been fixed.


Considering that there will be additions related to overhaul in the future, I think it would be good to be able to switch specs depending on the display mode. The livery scheme preview has been moved to a separate mode. Perhaps information about the overhaul will also be placed there.

Quote11. Having the traction type in blue also looks good.
Text colors should be managed by a theme. Because the background color of the UI is determined by the theme, and the readability of the text color depends on it. Therefore, I made it possible to manage it with a theme.
gui_color_livery_scheme
gui_color_mixload_prohibition
gui_color_overcrowded
gui_color_staff_shortage
gui_color_traction_type
gui_color_upgradeable
The version is incremented to 14.20 to clarify the occurrence of this change.


Quote2. The alignment for the figures next to max. speed and weight in the lower part of the dialogue seems to be slightly to the right of those of the other values.
This is because the digits are aligned to fix the position of km/h, etc. as much as possible. It would be easier to read that way when the mouse cursor is moved to display a series of vehicles. Performing alignment to the right brings it closer to the content on the right, and the UI system tends to enlarge the dialog so we could not do that.

Quote7. At the default size of the dialogue box, way constraint information can push some text off the bottom of the dialogue (try, e.g., the 1938 stock - requires 4th rail electrification; the small coastal ferry also has this issue)
Some changes have increased the base size of the dialog which may have eliminated this.


Quote13. I wonder whether we could make the dialogue a little wider when first opened?
I think it should be noted that only this dialog is too large compared to others.
Too much of this dialog alone may deprive the player of the opportunity to increase the font size.
It should also be noted that this is a dialog that is more sensitive to pak size than other dialogs.


Quote14. When creating a train (I tested with the SR Bulleid/Rawworth CC locomotive and the LNER corridor vestibule brake carriage), on adding the carriage after the locomotive, the relative position of the middle part of the dialogue moves downwards so that it is not possible to continue clicking in the same place to add more of the same vehicle. This is rather disruptive and can make using the dialogue harder than it needs to be.
I'm afraid I can't reproduce this. So I don't know what's going on and can't do anything about it. It would be helpful if you could explain in detail.


Quote19. In the replace dialogue, the goods type filter drop down menu seems to be corrupted if selecting it immediately after selecting the text filter.
As I reported here (https://forum.simutrans.com/index.php/topic,21776.msg201246/topicseen.html#msg201246) this is a bug from the standard. The patch by rafson doesn't seem to fix the underlying problem, and unfortunately the extended didn't fix this bug.



Quote20. In the replace dialogue, the selected livery scheme has no effect on the graphic displayed under "To be replaced by".
Try testing it on master branch. We cannot select livery in the replace dialog. Selection will be ignored.
However, it tricks players into thinking they can change the livery scheme. The livery scheme selector in replace flame has no meaning. I think I made a bug report for this a few years ago.
If we continue with this specification, I think it would be better to hide the livery selector in the replace dialog to reduce confusion for the player.

Quote22. The former use of checkboxes for replace all/replace all in line was easier to interact with and understand than the current drop down menu.
I didn't understand what this meant. Can you elaborate?

It should be noted that All is not all. I mean, it is limited to those have the same formation.
We may need to edit the text or port the description to the legend part.


QuoteIncidentally, Matthew is currently having trouble accessing the forums. He has left extensive feedback on Github here.
QuoteHowever, some locomotives can use all electric Track ways, in which case there is no permissive way constraint shown. I think the previous Depot window had MUST USE: and MAY USE: labels for prohibitive and permissive way contraints. I think it would be helpful to new players if these labels were restored to explain the way constraint information.
Correctly update the updated files in the branch before testing. I can see from your contradiction with James that you are not testing that branch correctly. From the image it's clear that you haven't updated the theme, but it was an earlier update in this branch. Secondly, James correctly updated and tested the translation, but you didn't. The new GUI doesn't allow line breaks in labels, but extended has unfortunately made such a choice before as a result of diverging from standard's development, so a change is needed. In extended, there is a translation that should not have a line break at the beginning of the sentence. So all labels in simutrans can be removed by putting a line break at the beginning of the translation. If you have a silly translation, the label won't show anything because it does a line break just before the MUST USE. It is no longer available only in the notepad stype component. Such relics of the past are now almost gone.


QuoteMystery Ship Vehicles
I recommend checking if the behavior is different from the current master branch (and standard).
It is a famous specification of simutrans that vehicles with empty images cannot be selected. This method is often used when pakset creators want to set two goods for one vehicle. So nothing has changed.


QuoteI would prefer Replace all in line as default, because I use this option much more. Perhaps default is just chosen by English alphabetical order here? But if you can select a default, that would be helpful please.
This should not be done so as not to form a trap for kicking out newbies.
It is very painful to undo the replace command issued to 1000 convoys of a line, but it is very easy to change the command issued to one convoy to all (same formation) convoy of the line. Also note that the line option is only present if it belongs to a line. Newbies will fall into traps more often if the default options are different for convoy and line.
Simutrans never ask for annoying pre-confirmation dialogs when performing risky actions.
Title: Re: PR#561 - Overhaul of depot and replace dialog
Post by: jamespetts on August 07, 2022, 12:37:26 PM
Excellent, thank you for that: I have confirmed the fixes, and I cannot at present reproduce the issue with the changing position of the dialogue box. The essential issues all having been remedied, I have now merged this into master. Thank you very much for your work on this: it is much appreciated.

I have updated the translation texts and at the same time clarified the texts relating to "replace all" in the depot dialogue as suggested.

Thank you again for your work on this - it is a great improvement and good progress in relation to work on the 15.x branch, too.