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

Don't automatically deactivate the vehicle clone tool after cloning a vehicle #6754

Merged

Conversation

alexanderweiss
Copy link
Contributor

@alexanderweiss alexanderweiss commented Apr 27, 2018

Changed the vehicle clone tool to remain active after cloning a vehicle instead of resetting the cursor (and disabling the clone tool).

This behaviour is more similar to the other tools (e.g. adding orders, building, etc.) and allows you to clone a vehicle multiple times without having to click the clone button again for each one.

@frosch123
Copy link
Member

frosch123 commented Apr 28, 2018

To summary various discussions from the past. There are two cloning tools, which currently behave like this:

  • Depot window with cloning tool:
    • Opens the window of the new vehicle.
    • Disable the tool.
  • Vehicle window clone button when vehicle is inside depot:
    • Open the window of the new vehicle when copy-cloning.
    • Do not open the window of the new vehicle when share-cloning.

The behaviour targets these use-cases:

  • When you copy-clone a vehicle, you use the source vehicle as a template, but want to adjust it. I.e. open the vehicle window.
  • When you share-clone a vehicle, you do not want to make any adjustments. I.e. do not clutter the screen with vehicle windows.

So with this in mind, at least in the copy-clone case the depot tool should be deactivated after usage.
The share-clone case is probably fine to change.

While the behaviour appears inconsistent, assuming the player knows about shared orders, it is the most convenient behaviour.

@frosch123
Copy link
Member

Suggested behaviour:

  • Depot clone tool stays active when share-cloning. Vehicle window is not opened.
  • Depot clone tool disables when copy-cloning. Vehicle window is opened.

Other opinions?

@ldpl
Copy link
Contributor

ldpl commented Apr 28, 2018

@frosch123 It's better as it is now imo. When you share-clone from depot you need to click vehicle in the veiwport which can be quite tricky if it's moving or there are other vehicles around. It's way easier to find it once and just spam clone in vehicle window than catching them in the viewport each time. I even have depot clone and vehicle clone binded to same hotkey(Ctrl-O) so if I need spam-clone some vehicles I just go Ctrl-O-click-O-O-O-O-O.

@ldpl
Copy link
Contributor

ldpl commented Apr 28, 2018

On a second thought I guess it won't be that hard to spam-click it in the viewport. While hotkey scheme I'm not even sure is possible in vanilla and it can be readjusted anyway. So it may be better overall since it will avoid opening redundant vehicle window.

@alexanderweiss
Copy link
Contributor Author

@frosch123 Deactivating it when copy-cloning kind of makes sense. At the same I don't think having the clone tool active when editing a vehicle is a problem, because I don't think it changes any of the behaviour of editing a vehicle (and it might be easier to just keep the clone tool behaviour the same)?
I'd be happy to change it though! (It's mainly useful for for share-cloning)

@frosch123
Copy link
Member

When changing the behaviour I would prefer to make the two clone buttons behave the same.
So, keep-active when share-cloning. Disable when copy-cloning.

Copy link
Member

@frosch123 frosch123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make the copy/share behaviour consistent with the clone-tool in the vehicle window.

@TrueBrain
Copy link
Member

Just a friendly poke to see if you are still up for this task? We would love to include it .. seems it is nearly there :D

@alexanderweiss
Copy link
Contributor Author

Whoops. Forgot about it. I'll finish it up.

@nielsmh
Copy link
Contributor

nielsmh commented Oct 30, 2018

I think this implements the changes @frosch123 suggested.

@alexanderweiss
Copy link
Contributor Author

Thanks @nielsmh!

@nielsmh nielsmh dismissed frosch123’s stale review October 30, 2018 19:58

Behaviour should now match between both clone tools.

@frosch123 frosch123 merged commit b3dc90a into OpenTTD:master Oct 31, 2018
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

Successfully merging this pull request may close these issues.

None yet

7 participants