Opened 19 months ago

Closed 18 months ago

Last modified 12 months 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 18 months ago.
forcedelete_v2.patch (1.9 KB) - added by Sandarac 18 months ago.
forcedelete_v3.patch (3.3 KB) - added by Sandarac 18 months ago.

Download all attachments as: .zip

Change History (11)

Changed 18 months ago by Sandarac

Attachment: forcedelete.patch added

comment:1 Changed 18 months ago by Sandarac

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 Changed 18 months ago by fatherbushido

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 Changed 18 months ago by elexis

  • 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)

Changed 18 months ago by Sandarac

Attachment: forcedelete_v2.patch added

comment:4 Changed 18 months ago by fatherbushido

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 Changed 18 months ago by elexis

  • 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.

Changed 18 months ago by Sandarac

Attachment: forcedelete_v3.patch added

comment:6 Changed 18 months ago by elexis

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 Changed 18 months ago by elexis

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 Changed 12 months ago by elexis

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.