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

[Bug]: It is not possible to transfer vehicles sharing orders between groups (using Ctrl+drag doesn't work) #9702

Closed
LC-Zorg opened this issue Nov 16, 2021 · 14 comments

Comments

@LC-Zorg
Copy link

@LC-Zorg LC-Zorg commented Nov 16, 2021

Version of OpenTTD

since 12.0, Win10

Expected result

You can use Ctrl+ drag to move groups of vehicles sharing orders between groups - as it was in 1.11.2 and earlier.

Actual result

Using Ctrl+ has no effect and it is not possible to move whole groups of vehicles between created groups in the vehicle list window.
To put 200 vehicles into the newly created group, you must move each vehicle separately.

Steps to reproduce

  1. Create a simple but long road connection between the two cities
  2. Buy dozens of vehicles to handle this connection
  3. Create a group in the vehicle list
  4. Try to put these several dozen vehicles in a new group
@LC-Zorg
Copy link
Author

@LC-Zorg LC-Zorg commented Nov 16, 2021

Seriously?! This is nonsense. Is it an accident at work or a deliberate act?

@LordAro
Copy link
Member

@LordAro LordAro commented Nov 16, 2021

No, this is a bug

@2TallTyler
Copy link
Member

@2TallTyler 2TallTyler commented Nov 16, 2021

When I try it, holding Ctrl before I click on a vehicle does nothing and the vehicle won't drag. If I click and drag, then hold Ctrl and release over the new group, it moves properly.

@LordAro
Copy link
Member

@LordAro LordAro commented Nov 16, 2021

Change was made in #9325
You are indeed able to drag vehicles sharing orders into a single group while holding ctrl if you press ctrl after you start dragging

OpenTTD/src/group_gui.cpp

Lines 748 to 762 in fc58ed9

if (v) {
if (_ctrl_pressed) {
if (this->grouping == GB_NONE) {
this->SelectGroup(v->group_id);
} else {
ShowOrdersWindow(v);
}
} else {
this->vehicle_sel = v->index;
SetObjectToPlaceWnd(SPR_CURSOR_MOUSE, PAL_NONE, HT_DRAG, this);
SetMouseCursorVehicle(v, EIT_IN_LIST);
_cursor.vehchain = true;
this->SetDirty();
}
}

However, the behaviour of having to press ctrl after starting dragging is a bit confusing, I wonder if we might be able to improve that... @btzy , any thoughts?

@frosch123
Copy link
Member

@frosch123 frosch123 commented Nov 16, 2021

The different behavior of ctrl+click depending on "group by" is weird to me.
How about something like

  • Ctrl+Singleclick: Select group + start dragging
  • Ctrl+Doubleclick: Show order window (independent of "group by")

On the other hand, ctrl+drag only makes sense if "group by" is set to "none". So it could also depend on the "group by" option:

  • "none": Ctrl+click starts dragging.
  • "shared orders": Ctrl+click shows orders.

@LC-Zorg
Copy link
Author

@LC-Zorg LC-Zorg commented Nov 17, 2021

Yes, I've already found it. This feature is definitely not supposed to work that way. Not only does it break players habits, but the new method is not consistent with anything else in the game. Wherever Ctrl + is used, the player presses Ctrl first and never the other way around. Changing this behavior in every other situation is not at all intuitive. Once this way, sometimes this way. No... I don't mean to be rude, sorry, but I have a very bad opinion about it.

I can see three methods to fix this:
1. Restore known behavior when grouping is disabled. For most players, everything will be the old way and just like everywhere else. When someone uses grouping by orders, then Ctrl + click will open the orders window as intended by that PR, and using Click + Ctrl will allow you to move a group of vehicles. For me that would be okay, but for others it can still be confusing.
2. Undo the change and instead add an icon that would open the orders window
Orders in vehicle list window

It can be done in such a way that this window shows some of the orders (first > second > maybe third), so that in many situations you will not even have to open the orders window.

3. Removing this change and forgetting about the problem - rather weak as the function is generally not bad.

Ctrl+Doubleclick:

I don't like double click ;) But maybe it won't be bad here. Certainly better than now.

@btzy
Copy link
Contributor

@btzy btzy commented Nov 17, 2021

I agree that this is a bug. However, it seems that before this change, users still need to be holding down Ctrl when releasing the mouse button - it's just that previously, the Ctrl key did nothing special when depressing the mouse button (except scrolling the UI to the current vehicle's group), so the same dragging mode starts whether or not the Ctrl key was already pressed when the mouse button is depressed. Arguably, the original behaviour was also awkward since it requires Ctrl to be pressed when releasing the mouse button, but not when depressing the mouse button.

I think the simplest fix for now is to implement @frosch123's second suggestion - to restore the old behaviour when not grouping by shared orders. It should just be switching the order of the if statements around those few lines of code. I don't quite like double clicks either - such functionality is generally non-discoverable via normal gameplay, and this steepens the learning curve.

In the long run, though, having a separate button to open the order list makes a bit more sense, since using 'Ctrl' for two totally different actions is probably confusing. It's reasonable to want to open the order list even if not grouped by shared orders.

@btzy
Copy link
Contributor

@btzy btzy commented Nov 17, 2021

I now see why this bug arose. The behaviour that @LC-Zorg wants is, and has been, only ever triggered by holding down Ctrl on release of the mouse button. If you hold down Ctrl on pressing down the mouse button, it selects that vehicle's group in the group GUI. However, previously, whether or not Ctrl is pressed while pressing down the mouse button, the drag-and-drop function is initiated.

Selecting the vehicle's group in the group GUI doesn't make sense when the dropdown is set to shared orders, since two vehicles sharing the same orders can be from different groups. Hence, the group GUI selection behaviour was removed if the dropdown is at shared orders, but the (desirable) behaviour of continuing to initiate drag-and-drop was inadvertently removed too.

btzy added a commit to btzy/OpenTTD that referenced this issue Nov 17, 2021
@btzy
Copy link
Contributor

@btzy btzy commented Nov 17, 2021

Actually, @LC-Zorg, I noticed you have the group-by set to shared orders, so you should do the drag-and-drop without pressing Ctrl. This is because you're already dragging the entire shared order group when using the shared orders view. It is not possible to move individual vehicles when using the shared orders view.

btzy added a commit to btzy/OpenTTD that referenced this issue Nov 17, 2021
btzy added a commit to btzy/OpenTTD that referenced this issue Nov 17, 2021
btzy added a commit to btzy/OpenTTD that referenced this issue Nov 17, 2021
btzy added a commit to btzy/OpenTTD that referenced this issue Nov 17, 2021
@Rau0x75

This comment was marked as disruptive content.

@LC-Zorg
Copy link
Author

@LC-Zorg LC-Zorg commented Nov 17, 2021

Fix: Revert old behaviour in group GUI when not using shared orders #9704

Thanks! :)
If you were planning to add this button in the future, especially for both lists, then of my suggestions I think the "or" would be better.
Especially showing an excerpt of orders could be useful. The problem could be that the windows of planes and ships show otherwise. I leave it for thought. ;) Thanks.

Issue to close, I suppose

@btzy
Copy link
Contributor

@btzy btzy commented Nov 17, 2021

The problem could be that the windows of planes and ships show otherwise.

What do you mean? Planes and ships show a little order list on their panels, as well as a tiny black triangle to indicate the current order. Afaik there is no special onclick handler bound to that list right now.

@LC-Zorg
Copy link
Author

@LC-Zorg LC-Zorg commented Nov 17, 2021

What do you mean? Planes and ships show a little order list on their panels

This is a potential, not actual issue if you wanted to add a button to the orders window as I shown above at the bottom part. Then it would be good to standardize the way of showing orders for all types of vehicles. If you ever wanted to go that way, of course.

@btzy
Copy link
Contributor

@btzy btzy commented Nov 17, 2021

I see, yeah, it would need to be standardized if we do that. I'm not privy to the design decisions behind making planes and ships display their order list though; maybe someone else here knows.

TrueBrain pushed a commit to TrueBrain/OpenTTD that referenced this issue Jan 3, 2022
TrueBrain pushed a commit that referenced this issue Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants