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

Patch : Speed limits tab in vehicle window #1199

Closed
DorpsGek opened this issue Sep 5, 2007 · 28 comments
Closed

Patch : Speed limits tab in vehicle window #1199

DorpsGek opened this issue Sep 5, 2007 · 28 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay

Comments

@DorpsGek
Copy link
Member

DorpsGek commented Sep 5, 2007

MagicBuzz opened the ticket and wrote:

Some trains sets use speed limit for wagons, and it's great. But when the vehicle is built, you know its global speed limit, but you can't figure out which wagons or engines are responsible of this limit.

So I just designed a very small patch in order to add a new tab in the vehicle info, to see the actual vehicle speed limit for each element of the train.

It remains just a small problem : The buttons are smaller now, so the labels should not fit with all languages.

r11042

Forum http://www.tt-forums.net/viewtopic.php?f=33&t=33941

Attachments

Reported version: trunk
Operating system: All


This issue was imported from FlySpray: https://bugs.openttd.org/task/1199
@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 5, 2007

skidd13 wrote:

I like the idea of the patch.
I noticed 2 things:
- in "src/lang/english.txt" are a few spaces&tabs too much
- TrainDetailsSpeedsTab() should have some comments for the documentation. See http://wiki.openttd.org/index.php/Coding_style# Functions_2


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2060

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 5, 2007

MagicBuzz wrote:

Hello,
I just updated with some comments.
About the english.txt file, I can't see which spaces/tabs you are talking about (I uses VS2005 and it uses tab size to 4. The whole file seems to be correctly lined up with these settings)

Update to r11044

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2061

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 5, 2007

MagicBuzz wrote:

Variant of the patch, with right aligned speeds.
Same revision.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2062

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 5, 2007

skidd13 wrote:

MagicBuzz: I can't see which spaces/tabs you are talking about
The problem IMO is you use tabs. The common way is to use spaces to align.
I just noticed it while reading the diff in Iceweasel.


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2063

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 5, 2007

Belugas wrote:

A picture is worth a thousand words. The red arrows are tabs. Unwanted. To be replaced by spaces, please.

Furthermore, I would strongly suggest that (while you are dealing with that gui) you should replace the widget index by enums. It will make the whole code less "magical"

I was wondering... instead of having the buttons been squeezed a little more, why not open up the gui by adding a new row of buttons?

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2064

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 5, 2007

MagicBuzz wrote:

Here are the corrections:
- Used space rather than tabs in english.txt
- Added a new button line in order to keep original size for the original buttons
- Used an enum TLW_* (for Train List Widgets) to name the widgets in the code instead of "magic numbers"

Still a minor bug:
- The original buttons are resized whan resizing the window. Mine doesn't, and I can't understand why...

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2065

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 5, 2007

skidd13 wrote:

I added a full enumification patch. This might be usefull for you.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2067

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 6, 2007

MagicBuzz wrote:

Well...
I was trying to update the patch with a complete enumeration (rien now I only updated the controls that were used in the code), but I just face to a problem I can't solve myself:

With the r11050, the vehicle detail window has been unified between all types of vehicles.
But the "Speed" tab will apply only for trains, then I don't know what to do...

I don't know how to hide/unhide some controls in a windows dynamically (for any vehicle but trains, the new line with the "speed" buttond shouldn't be displayed).
And I don't think that creating a new window definition especialy for the trains is a good idea.

Could someone help me ?


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2070

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 6, 2007

skidd13 wrote:

The quick Idea I've got would be to add the speed button and the spacer in the global window descritpion and hide them at WE_CREATE for vehicle type != train. But it needs a bit more thinking. I'll see what I can do for you, when I'm back home.
A GUI that does similar things is the order_gui IIRC. Maybe have a look at the solution in there.


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2071

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 6, 2007

MagicBuzz wrote:

Here is the update for r11050 (thank you and nycom for the tips!)

Thus now the button is correctly resized when window size changes.
I had to split my "end line" panel into 3 smaller pannels in order to make it works although. These 3 panels may be used for other new patches.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2073

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 6, 2007

skidd13 wrote:

Nice. :)
The 3 extra buttons could confuse beginners.
The GUI must defenetly care about beginners IMO.
A large panel should do a better job.


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2074

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 6, 2007

MagicBuzz wrote:

That's what I did, but I have a problem with the "ResizeButton".
If I use a "big" panel and activate the resizing on the button, the button take 1/2 of the window width. It seems the ResizeButton function use the same witdh for any control rather than keeping correct ratio.
I think having a so big button for a so small patch is quite strange.
Anyway, I can change if you really think it's a better solution (or remove the resizing function)


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2075

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 6, 2007

skidd13 wrote:

I'd do it this way:
- Vertical size of speed button and panel = 0
- Hide the size button and the panel allways.
- Resize and show them if a the window is for trains


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2076

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 7, 2007

MagicBuzz wrote:

I don't have any problem with showing/hiding these buttons.

The problem is that when you resize the windows (left/right) the tab buttons "cargo", "information", "capatity" and "total cargo" are resized also (ie when the window width is 400px, each button is 100px, when it's 600, the buttons are 150 :

1111222233334444 (400 px)
111111222222333333444444 (600 px)

This is done with the "ResizeButton" function. It resize the widget list you give in parameter. But the limitation of this function is that if a widget width is initialy 100px and the other one is 300px, after resizing the window, they do the same width : 200px each. The ResizeButton doesn't keep ratio.

1111222222222222 (initial size)
111111111111222222222222 (resized to 600px)

As a result, when I designed a small button followed by a big pannel (x3 place holder width), when resizing, by button was 2x button width and the panek was 2x button width.
That's why I created 3 small pannels rather than 1 big : button and pannels do the same size when resizing.

All that I could do is to stop using the ResizeButton function the the second line, but it won't be nice, as a resized window will loot at this :

===B1===|===B2===|===B3===|===B4===
=B5=|==============================

(B1, B2, B3, and B4 are the "cargo", "information", "capatity" and "total cargo" buttons and B5 is the "Speed" button).


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2083

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 7, 2007

skidd13 wrote:

Maybe this helps you:
http://wiki.openttd.org/index.php/OpenTTDDevBlackBook/Window/UseWindows# Flags_2


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2084

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 7, 2007

skidd13 wrote:

Another idea would be to use the event WE_RESIZE and sync values:
B5.right = B1.right;
Panel.left = B2.left;


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2085

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 7, 2007

MagicBuzz wrote:

Thank you.
The direct left and right properties update is not really "clean", but that was easy to do, so I used and it works perfectly.
Thanks a lot!

ResizeButtons(w, VLD_WIDGET_DETAILS1_CARGO_CARRIED, VLD_WIDGET_DETAILS1_TOTAL_CARGO);
ResizeButtons(w, VLD_WIDGET_DETAILS2_SPEEDS, VLD_WIDGET_DETAILS2_FREE3);

Becomes

ResizeButtons(w, VLD_WIDGET_DETAILS1_CARGO_CARRIED, VLD_WIDGET_DETAILS1_TOTAL_CARGO);
w->widget[VLD_WIDGET_DETAILS2_SPEEDS].right = w->widget[VLD_WIDGET_DETAILS1_CARGO_CARRIED].right;
w->widget[VLD_WIDGET_DETAILS2_ENDLINE].left = w->widget[VLD_WIDGET_DETAILS1_TRAIN_VEHICLES].left;

(VLD_WIDGET_DETAILS2_FREE1, 2 and 3 widgets gone, replaced by VLD_WIDGET_DETAILS2_ENDLINE).

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2086

@DorpsGek
Copy link
Member Author

skidd13 wrote:

Small update to keep the patch up2date

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2156

@DorpsGek
Copy link
Member Author

skidd13 wrote:

Update by me cause the author seems to be uninterested. Hope I fixed all style stuff.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2555

@DorpsGek
Copy link
Member Author

DorpsGek commented Nov 1, 2007

skidd13 wrote:

Fixed some typos.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2559

@DorpsGek
Copy link
Member Author

DorpsGek commented Nov 1, 2007

skidd13 wrote:

Sorry missed one mistake.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2560

@DorpsGek
Copy link
Member Author

DorpsGek commented Nov 3, 2007

TrueBrain wrote:

I found it rather ugly in-game. Yet an other button. Isn't it possible to group together some of those items? Like Cargo and Total Cargo? Or maybe make one general window? It isn't that much information...


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment2588

@DorpsGek
Copy link
Member Author

skidd13 wrote:

For the log:
the patch or the idea haven't died at all...
I've got the idea to merge the whole settings in a matrix window.
But since it is a lot of work and trunk is in feature freeze ATM it is deferred past 0.6.


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment3650

@DorpsGek
Copy link
Member Author

peter1138 wrote:

Why not simply show the speed limits in the 'Information' tab? There is a reasonable amount of space there...


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment3651

@DorpsGek
Copy link
Member Author

skidd13 wrote:

I thought over merging the existing tabs to:
- a global / general tab
-- list view
-- about the whole train
-- used by all vehicle types
- a weagon tab
-- matrix view
-- about each weagon / engine
-- this could maybe used by ARVs too
- a economy tab
-- list view
-- with the informations like running costs, service intervall, etc.
-- used by all vehicle types


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment3652

@DorpsGek
Copy link
Member Author

sbr wrote:

Show the max speed of the rail vehicle in the 'Information' tab of the details window.
The max speed isn't displayed for a rail wagon if 'wagon speed limit' is disabled or if it hasn't a speed limit.

Please find an attached patch against r19403.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment7726

@DorpsGek
Copy link
Member Author

Alberth wrote:

A nice and simple solution, perhaps a bit too simple. Please open the attached picture, and tell me in one glance (ie without actually reading each line) which parts of the train are the limiting factor.

I couldn't.

The reasons for that seem to be (I think):

  1. The current picture is a mess, it takes a while before you understand what numbers are important to answer the question. The unpatched version was already pushing it, this is just a bit too much.
  2. No highlighting. All numbers are in one colour, so I have to figure out what the lowest numbers are, and how many there are.

The fix for 1) may be to use columns. That would definitely help here (nowadays, the GUI code is much more flexible, it can take care of the sizing problems for you), but how that affects the other tabs is a question that needs some careful thought.
Perhaps other solutions are possible here, with less impact on the other tabs.

The fix for 2) would be to use a different colour either for the limiting number (but that would include the 160km/h in the top-panel too), or for the non-limiting numbers.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199#comment7774

@DorpsGek
Copy link
Member Author

andythenorth closed the ticket.

Reason for closing: Won't implement

Mass closure of patch tickets with no commentary for >5 years. Goal is to reduce patch queue as an experiment to see if it aids faster reviewing and rejection/acceptance (it may not). If this offends you and the patch is maintained and compiles with current trunk, discuss with andythenorth in irc. (andythenorth has no ability to review patches but can re-open tickets).


This comment was imported from FlySpray: https://bugs.openttd.org/task/1199

@DorpsGek DorpsGek added the flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) label Apr 6, 2018
@DorpsGek DorpsGek added Vehicles patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay labels Apr 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay
Projects
None yet
Development

No branches or pull requests

1 participant