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

Stations: add distant-join toggle to construction UI #5951

Closed
DorpsGek opened this issue Mar 22, 2014 · 8 comments
Closed

Stations: add distant-join toggle to construction UI #5951

DorpsGek opened this issue Mar 22, 2014 · 8 comments
Labels
component: interface This is an interface issue enhancement Issue would be a good enhancement; we accept Pull Requests! 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

CommanderZ opened the ticket and wrote:

This patch adds another toggle button to build station UI (of any vehicle type) under the "show coverage area" switch that allows player to distant join a station without using a keyboard. Holding CTRL behaves the same as it used to if the distant join mode is disabled, but it builds the station without distant join if the mode is enabled (so it effectively inverts the current mode). No other changes to distant join behavior are included.

The idea of this patch is to improve playability on touch screen devices and as a side effect improve new player experience (not everyone reads every tooltip or wiki).

Of course the main question here is whether there is desire to include UI changes/improvements like this in trunk OTTD - if there are, I will provide more small patches improving other aspects of the UI/exposing other functions hidden behind keyboard combos.

Attachments

Reported version: trunk
Operating system: All


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

CommanderZ wrote:

Oh, I attached a wrong patch :?

Here is the right one.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/5951#comment13164

@DorpsGek
Copy link
Member Author

aswn wrote:

good one


This comment was imported from FlySpray: https://bugs.openttd.org/task/5951#comment13165

@DorpsGek
Copy link
Member Author

planetmaker wrote:

The direction is a good one, I think, but a few issues remain. Currently I find the naming of "Join with adjacent stations" a bit confusing for what it actually does.

a) Conceptually I'd change the element added to the station GUI to mean and say "Join with distant station part" On/Off with the default being 'off', which is the current behaviour.
b) If there is no station in reach which can be joined to, do not even pop-up the station-join window, no matter the setting.
c) Consider the setup as in the attached screenshot. Assume I want to build a station on the road. That currently is not possible, nor when using this patch, without using Ctrl. In that case it must pop-up the station selection window in any case, no matter the setting.
d) Barring points b) and c), use of the ctrl key should invert the behaviour (as if the setting was changed).
e I'm afraid this task easily escalates, see #4185: http://bugs.openttd.org/task/4185 - thus do not consider competitor stations for joining.

f) From a coding style POV, please mind how tabs and white space is used: Do not use tabs except for the indentation of lines, but never for aligning comments. See also http://wiki.openttd.org/Coding_style


This comment was imported from FlySpray: https://bugs.openttd.org/task/5951#comment13166

@DorpsGek
Copy link
Member Author

planetmaker wrote:

The screenshot. Consider the road between Bonston Valley and Bardinghead South.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/5951#comment13167

@DorpsGek
Copy link
Member Author

CommanderZ wrote:

a) I'm not a native English speaker, I certainly don't expect my strings to be final.
b) Is this necessary? During my games, there is almost always some other station within the station spread range. I tried to keep the patch as simple as possible (so I didn't modify the actual distant join behavior at all).
c) I see, I didn't realize there were any checks before the actual decision whether join or not. I will look into this.
d) I agree, that is the intended behavior.
e) I don't understand. How does this concern this issue?
f) I will clean this up.


This comment was imported from FlySpray: https://bugs.openttd.org/task/5951#comment13168

@DorpsGek
Copy link
Member Author

planetmaker wrote:

b) and c) basically ensure that the UI gets easy to use and better no-matter-what. It makes sure that OpenTTD just does the right thing when there's no choice anyway. When making it more complicated (by this new setting) it's a good idea to polish the existing stuff first. We had a brief chat on IRC and these two edge cases seem to be concensus to be wanted when touching this code.

e) comes into the issue basically in the same or similar place as b) and c) come into play. In any case fixing that issue should not be hindered or complicated, thus better include it when touching this code :)


This comment was imported from FlySpray: https://bugs.openttd.org/task/5951#comment13169

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 2, 2017

andythenorth wrote:

This compiles on r27910

I didn't test it in depth. I don't like the specific UI layout, but the new behaviour is an improvement IMHO.


This comment was imported from FlySpray: https://bugs.openttd.org/task/5951#comment14729

@DorpsGek DorpsGek added component: interface This is an interface issue flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) enhancement labels Apr 7, 2018
@andythenorth
Copy link
Contributor

Let's pass on this one, as doing it well is complex. There are bigger fish to fry.

@andythenorth andythenorth added patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay enhancement Issue would be a good enhancement; we accept Pull Requests! and removed enhancement from FlySpray labels Apr 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: interface This is an interface issue enhancement Issue would be a good enhancement; we accept Pull Requests! 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

2 participants