Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#3962 closed enhancement (fixed)

[PATCH] Hotkey - force delete buildings with shift+delete

Reported by: elexis Owned by: elexis
Priority: Should Have Milestone: Alpha 21
Component: UI & Simulation Keywords: patch
Cc: Patch:

Description

Often a player wants to delete a building before it is captured. However clicking "Yes" on the confirmation dialog is timeconsuming.

A hotkey should be added so that the dialog is skipped if the hotkey is pressed.

See line 700 of unit_actions.js and openDeleteDialog in menu.js.

Attachments (3)

forcedelete.patch (2.0 KB ) - added by Sandarac 8 years ago.
forcedelete_v2.patch (1.9 KB ) - added by Sandarac 8 years ago.
forcedelete_v3.patch (3.3 KB ) - added by Sandarac 8 years ago.

Download all attachments as: .zip

Change History (11)

by Sandarac, 8 years ago

Attachment: forcedelete.patch added

comment:1 by Sandarac, 8 years ago

Keywords: review patch added
Milestone: BacklogAlpha 21
Summary: Hotkey - force delete buildings with shift+delete[PATCH] Hotkey - force delete buildings with shift+delete

comment:2 by fatherbushido, 8 years ago

Just a quick review. I m not really aware of gui stuff but :

  • massdelete doesn't seem to be a good name
  • i think deleteWithoutConfirmation should not be in menu.js. And imo it could be remove: just use !Engine.PostNetworkCommand({ "type": "delete-entities", "entities": selection }); after the if in unit_action

comment:3 by elexis, 8 years ago

  • Don't forget to add yourself to programming.json
  • That what fatherbushido said (the closing of open dialogs probably not needed/useful)
  • noConfirmation would be a better name (which is also already used for savegames, but don't reuse that key for deleting buildings, add a new entry to hotkey.session)

by Sandarac, 8 years ago

Attachment: forcedelete_v2.patch added

comment:4 by fatherbushido, 8 years ago

It sounds good now. (Looking to the namming of other stuff in this code, i don't know if it's better to use "noconfirmation" or "noConfirmation")

comment:5 by elexis, 8 years ago

  • Tested the patch, works, nice.
  • Good point fatherbushido, almost every default.cfg entry is written in lowercase. While at it, we should make this more consistent and replace noConfirmation with noconfirmation in default.cfg, replay_actions.js and load.xml.
  • There are spaces before Engine.PostNetworkCommand where a tab should be.
  • In r18224 the whitespace changed. You will be looking for line 1026 in unit_actions.js to add your code and you should add newlines to that object argument to conform to the format.
  • We could also make the deleteSelectedEntities from menu.js a global function in menu.js and reuse that in unit_actions.js, but I will leave that up to you, doesn't matter that much.

If you fix the whitespace and lowercase, I will commit it.

by Sandarac, 8 years ago

Attachment: forcedelete_v3.patch added

comment:6 by elexis, 8 years ago

Owner: set to elexis
Resolution: fixed
Status: newclosed

In 18238:

Implement hotkey to delete units and buildings without confirmation dialog. Patch by Sandarac, fixes #3962.

comment:7 by elexis, 8 years ago

Keywords: simple review removed
Type: defectenhancement

Thanks for the patch!

It would be preferable to have patches relative to the root directory of 0AD (one patch being relative to public and the other relative to binaries/data is a bit confusing).

comment:8 by elexis, 7 years ago

In 18996:

Implement noconfirmation hotkey for deleting savegames in the save dialog. Refs #3962.
Remove the deleteGame duplicate.

Note: See TracTickets for help on using tickets.