Opened 12 years ago

Closed 11 years ago

#1001 closed enhancement (fixed)

[PATCH] Attack Move

Reported by: Jonathan Waller Owned by:
Priority: Should Have Milestone: Alpha 13
Component: UI & Simulation Keywords:
Cc: Jonathan Waller Patch:

Description

The attack-move command should be implemented. This is a move command where the units attack any enemies that come into sight along the route, once the enemies are destroyed the units continue walking to the destination.

Attachments (3)

attack-move.diff (13.9 KB ) - added by Jonathan Waller 12 years ago.
attack-move-v2.diff (8.1 KB ) - added by mimo 11 years ago.
attack-move-v3.diff (10.2 KB ) - added by mimo 11 years ago.

Download all attachments as: .zip

Change History (17)

by Jonathan Waller, 12 years ago

Attachment: attack-move.diff added

comment:1 by Jonathan Waller, 12 years ago

Keywords: review added
Milestone: BacklogAlpha 8
Summary: Attack Move[PATCH] Attack Move

I have attached a patch which implements the attack move command, it is currently bound to Super+RightClick since this is unused. The main logic is in the additions to UnitAI.js.

The changes to Formation.js allow a formation to be 'suspended' which allows the units in the formation to act individually but still remain registered with the formation, this allows the units to attack individually and then return to a formation.

The other changes are just passing messages around.

Someone had mentioned binding it to Ctrl+RightClick, this would then clash with garrison so there would need to be some restructuring of the input code or forcing both garrison and attack-move to always share the same hotkey.

comment:2 by Jonathan Waller, 12 years ago

Keywords: review removed

I removed the review tag because after further thought I realized it needs some modification in functionality. Also in some testing earlier today it was definitely buggy so I will try and get that figured out. So if anyone wants to look at the code I wouldn't complain, but I consider the priority to be lower since I will definitely be making changes.

comment:3 by Kieran P, 12 years ago

Milestone: Alpha 8Alpha 9

comment:4 by Kieran P, 12 years ago

Milestone: Alpha 9Alpha 10

comment:5 by Jonathan Waller, 12 years ago

Owner: Jonathan Waller removed

comment:6 by Kieran P, 12 years ago

Cc: Jonathan Waller added
Keywords: ui ai added; unitAI removed
Owner: set to Jonathan Waller
Type: taskenhancement

comment:7 by Jonathan Waller, 12 years ago

Owner: Jonathan Waller removed
Summary: [PATCH] Attack MoveAttack Move

I am not going to be working on this in the near future. Formations are still incomplete so I think it is probably best to figure them out before attack move. My last uploaded patch had some fairly big bugs so it is left in case it is useful but would need lots of modification or a rewrite. I also have higher priorities that I am working on currently.

comment:8 by Kieran P, 12 years ago

Milestone: Alpha 10Backlog

comment:9 by mimo, 11 years ago

This enhancement would be really useful. Here is a new implementation of the concept.

by mimo, 11 years ago

Attachment: attack-move-v2.diff added

comment:10 by mimo, 11 years ago

Keywords: patch review added
Summary: Attack Move[PATCH] Attack Move

comment:11 by Jonathan Waller, 11 years ago

Sorry for the delay (again), please pester me if I take too long responding to further updates on this task. Most of the patch looks good, a few comments.

You currently have attack move using the garrison hotkey. It must have its own hotkey. The keep the use of ctrl you could remove the action-garrison-disabled cursor bit so that garrison works the same way that attack does in that bit of code.

Also when using attack move the units start walking before they find a new target. This looks horrible since they stutter and then resume attacking. Try it on the combat demo, it clear when ranged units are attacking.

It would be nice to randomize the attack targets a bit as well. This would prevent lots of focus fire and overkill (when 20 arrows hit a unit a lot of the damage is wasted since the first 10 arrows killed it). This is a nice enhancement but isn't critical, I would commit a patch without this.

in reply to:  11 comment:12 by mimo, 11 years ago

Replying to quantumstate:

Sorry for the delay (again), please pester me if I take too long responding to further updates on this task. Most of the patch looks good, a few comments.

You currently have attack move using the garrison hotkey. It must have its own hotkey. The keep the use of ctrl you could remove the action-garrison-disabled cursor bit so that garrison works the same way that attack does in that bit of code.

Yes, you are right. I was done just for quick tests. Now, its own hotkey is now used by this patch. But I see that CTRL is already used to force attack orders in binaries/data/config/default.cfg, so I'm now using AltGr which was not yet used.

Also when using attack move the units start walking before they find a new target. This looks horrible since they stutter and then resume attacking. Try it on the combat demo, it clear when ranged units are attacking.

Yes, this point is also fixed in my latest version of the patch.

It would be nice to randomize the attack targets a bit as well. This would prevent lots of focus fire and overkill (when 20 arrows hit a unit a lot of the damage is wasted since the first 10 arrows killed it). This is a nice enhancement but isn't critical, I would commit a patch without this.

That's a bit more complicated to implement, but you are right it would be better. I've to think a bit about it. In the meantime, I will provide a patch with the first two points fixed.

by mimo, 11 years ago

Attachment: attack-move-v3.diff added

comment:13 by Jonathan Waller, 11 years ago

In 13200:

Adds attack move bound to the Ctrl hotkey. Patch from mimo. Refs #1001.

comment:14 by Jonathan Waller, 11 years ago

Keywords: ui ai patch review removed
Milestone: BacklogAlpha 13
Resolution: fixed
Status: newclosed

Thanks for the patch. That one looks good.

I changed the hotkey stuff a bit. Now it shares Ctrl with garrison. Force attack got pushed over to AltGr because that is rarely used so we want the more useful stuff on the easier hotkey.

There are still some UI changes I would like and the suggestion I mentioned before. I have opened #1847 which I assigned to you since you said you were thinking about it. Hopefully this is ok, otherwise feel free to unassign yourself.

Note: See TracTickets for help on using tickets.