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

CTRL Click on units fails to check for existing orders before sharing. #5990

Closed
DorpsGek opened this issue Apr 19, 2014 · 7 comments
Closed
Labels
component: interface This is an interface issue flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)

Comments

@DorpsGek
Copy link
Member

Leszek opened the ticket and wrote:

This is a small bug in 1.4.0 release that I was able to duplicate on version 1.3.2 on a separate computer in seconds. It is an interface glitch and because of this the save game is irrelevant as any saved game can work.

I don't know the name of the feature so I will describe the expected behavior and the conditions that lead to the unexpected behavior.

Normal behavior:

  1. User can click on another unit to copy orders. This copy only happens if the unit has an empty orders list.
  2. User can CTRL - click on another unit to share orders.

Expected: The share of orders would only happen if the unit has an empty orders list. But it doesn't check, it just deletes the existing orders and starts to share. This error comes up often and silently if the user is in the habit of ctrl - Clicking depots to set "Maintain at XXX" orders.

Reported version: 1.4.0
Operating system: All


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

3298 wrote:

Working as intended. However, the code comment explaining this behavior has a TODO in it:
/* v is vehicle getting orders. Only copy/clone orders if vehicle doesn't have any orders yet.
* We disallow copying orders of other vehicles if we already have at least one order entry
* ourself as it easily copies orders of vehicles within a station when we mean the station.
* Obviously if you press CTRL on a non-empty orders vehicle you know what you are doing
* TODO: give a warning message */
(from order_gui.cpp, in the function OnVehicleSelect)

In my opinion this behavior should be changed though, because (as you mentioned) there are some Ctrl+Click shortcuts for creating orders which should be protected as well.


This comment was imported from FlySpray: https://bugs.openttd.org/task/5990#comment13242

@DorpsGek
Copy link
Member Author

Leszek wrote:

Also it is misleading, if copy is disabled with one order in, it seems natural that share would likewise be disabled. From the user perspective clicking on the depot and then clicking on the "maintain at" is the same as ctrl clicking on the depot. There is not a natural reason why it should not be so.

Though this might be a personal issue with the way I do things, it can cause headaches. I have a large complicated group of 70 some odd ships each doing its own thing. When this happens there is no message or other indication that you have now shared orders and if you don't catch it right away not only does the ship you are working on have messed up orders but an effectively random other ship that you now have to try to sort out. If you happen to click on a member of a group that already has shared orders and perhaps a timetable, it can take several game years to set things straight again.

Also I think that auto deleting an orders list is something to be avoided in general. It should take explicit user action to delete what is already setup wherever reasonable. At least IMHO.


This comment was imported from FlySpray: https://bugs.openttd.org/task/5990#comment13248

@DorpsGek
Copy link
Member Author

DorpsGek commented May 4, 2014

Alberth wrote:

Since it works as advertised, it's not a bug but a feature request


This comment was imported from FlySpray: https://bugs.openttd.org/task/5990#comment13291

@DorpsGek
Copy link
Member Author

alex6 wrote:

If it is not already done,
I tried to make a warning message only for shared orders when there are already orders for the vehicle.
Is there someone who can test it ?
Here is the patch file (the 4.1KiB one).

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/5990#comment13560

@DorpsGek
Copy link
Member Author

planetmaker wrote:

You should test the patch yourself, too ;) The 4.1KiB version disallows to create new shared orders for a new vehicle (which has no order yet) by ctrl+click on a vehicle which has some orders.

Remove dead code (line with //). Provide full doxygen for all functions you introduce (parameters need a brief explanation).
Translations are usually done via our web translator, patches only provide the English (BE) one.


This comment was imported from FlySpray: https://bugs.openttd.org/task/5990#comment13561

@DorpsGek
Copy link
Member Author

alex6 wrote:

Oh yes : unforgivable :)
I juste corrected it and the test is now ok.

I just removed the old line and added doxygen explanations.
I also saw that a variable name was not in a good naming style.
The french translation is now removed.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/5990#comment13563

@DorpsGek
Copy link
Member Author

Alberth closed the ticket.

Reason for closing: Won't implement

It breaks easily moving vehicles from one set of orders to another (ie you get an annoying new confirmation window with every vehicle).
I agree current solution is not optimal, but this is a bit too invasive.


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

@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
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 flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)
Projects
None yet
Development

No branches or pull requests

1 participant