Opened 11 years ago

Closed 10 years ago

Last modified 4 years ago

#2034 closed defect (fixed)

[PATCH] escort order

Reported by: mimo Owned by: mimo
Priority: Should Have Milestone: Alpha 15
Component: Core engine Keywords: patch
Cc: Patch:

Description

It would be nice to add an escort order to be able to protect siege units. Here is a patch which provides this. It is generic enough to allow units to either escort other units or guard buildings. In addition, escorting units may repair or heal their escorted.

I've used the Alt-Gr key for this order (we start to be short in keys); but from a previous patch, I think this key is not present on some mac, so you may have to change it. In addition, as I didn't wanted the escorting units to be selected by default when grabing units with the mouse, or double-clicking, there are removed by default but still can be selected by pressing F1 while double-clicking or grabing.

Attachments (7)

escort.diff (23.8 KB ) - added by mimo 11 years ago.
oups, i forgot the svn add ! here it is
escort-v2.diff (39.1 KB ) - added by mimo 10 years ago.
add-guard.png (5.0 KB ) - added by mimo 10 years ago.
remove-guard.png (5.4 KB ) - added by mimo 10 years ago.
escort-v2.2.diff (38.4 KB ) - added by mimo 10 years ago.
action-guard-disabled.png (1.5 KB ) - added by mimo 10 years ago.
action-remove-guard.png (1.9 KB ) - added by mimo 10 years ago.

Download all attachments as: .zip

Change History (28)

comment:1 by leper, 11 years ago

The patch seems to be missing the Escort component.

comment:2 by historic_bruno, 11 years ago

Milestone: BacklogAlpha 14

There was already an open ticket for this, btw: #25

Alt-Gr isn't available on US keyboards either.

Last edited 11 years ago by historic_bruno (previous) (diff)

by mimo, 11 years ago

Attachment: escort.diff added

oups, i forgot the svn add ! here it is

comment:3 by historic_bruno, 11 years ago

Same comment as for #1907: selections shouldn't rely on obscure botkeys. If you want to add a hotkey to make selections more exclusive, that would make sense, then it's up to the player to learn about and use it at their discretion.

I'm not sure that escort range should be specified by the target, shouldn't it be based on the escorter's vision range? That may not be as flexible but it doesn't seem like a place where we need that flexibility.

I don't understand the purpose of the class list in the Escort component, it seems like any unit capable of escorting/guarding should be able to escort/guard any other of the player's entities (and allies' as well)? To keep it simple, we could say if a unit has attack capability, it can escort/guard, then do an ownership check.

Last edited 11 years ago by historic_bruno (previous) (diff)

in reply to:  3 comment:4 by mimo, 11 years ago

Let's reply here to both comments to #1907 and this ticket which are on the same subject

Replying to historic_bruno on #1907:

Replying to mimo:

Corraling use the Ctrl key, and corraled animals, while still selectable by clicking them, can't be selected by grabing or double-click (to avoid unwanted un-corraling). To force them to be selected by dragging or double-click, you have to also press the F1 key.

Is that really necessary? I think a similar issue exists for other units and orders, for example infantry, some could be attacking and some idle or gathering, but we don't have special keys to select them by order type. With hotkeys there is a risk of adding confusion and this one doesn't seem intuitive :(

I think the problem here is a bit different because we are discussing cases where we would nearly never want to select these units. When a unit is escorting, we want to quicky select the escorted and give it orders without selecting the escort (otherwise that would remove its escort order, while we only want it to follow and protect its escorted). In the other ticket about corraled animals, the same thing happen : we nearly never want to select them, which would make us loose our bonus. In addition, in both cases, guarding units or corraled animals are immobile and difficult to distinguish from an idle unit, so we may be tempted to select them to affect them tasks. That's why I've implemented it this way : not selectable by default (except when directly clicking on them) and adding this key for the very rare occasion where we would volontarily want to select them.

Replying to historic_bruno:

Same comment as for #1907: selections shouldn't rely on obscure botkeys. If you want to add a hotkey to make selections more exclusive, that would make sense, then it's up to the player to learn about and use it at their discretion.

Yes, that would be a possibility, but as I said we nearly never want to select escort units, so in a battle with some units escorting our siege units, we would constantly press this exclusive-hotkey. So a better option could be to take as default what you propose (standard=inclusive, key=exclusive), but have a way to allow the user to redefine its selection preference as I proposed (standard=exclusive, key=inclusive) in full knowledge of the consequences.

in reply to:  3 ; comment:5 by mimo, 11 years ago

Replying to historic_bruno:

I'm not sure that escort range should be specified by the target, shouldn't it be based on the escorter's vision range? That may not be as flexible but it doesn't seem like a place where we need that flexibility.

This range is not the range of action, which is indeed based on vision range. It's only the distance at which the escort stand with respect to its escorted. It should not be too near to not interfere with its movements, but not too far to really protect it quicly enough.

I don't understand the purpose of the class list in the Escort component, it seems like any unit capable of escorting/guarding should be able to escort/guard any other of the player's entities (and allies' as well)? To keep it simple, we could say if a unit has attack capability, it can escort/guard, then do an ownership check.

But we want also have only ship which can escort ship, that ships can guard harbour but no other buildings, healer which can escort any units, only infantry to guard buildings, ... This class defines which units can escort the escorted.

in reply to:  5 ; comment:6 by historic_bruno, 11 years ago

Replying to mimo:

I think the problem here is a bit different because we are discussing cases where we would nearly never want to select these units.

You don't think people would want to move animals once they are corralled? What about moving them to a different corral or moving them out of danger. As for indicating which animals are corralled, I think interfering with selections is a terrible way to do it. People will simply wonder why some units can't be selected and think it's a bug with the game (I'd tend to agree). According to Michael, AoE3 had livestock pens and animals just stood around it (see link in #1907), I assume they were still selectable but I'm not sure.

Guarding is the same way, I never had a problem in AoK remembering which units were guarding and which were not, plus you can easily separate units by function/order using selection groups. And if you make a mistake, it's not hard to fix by re-tasking the unit to guard. That's far simpler than out-thinking the player IMO.

Replying to mimo:

This range is not the range of action, which is indeed based on vision range. It's only the distance at which the escort stand with respect to its escorted. It should not be too near to not interfere with its movements, but not too far to really protect it quicly enough.

I don't see the difference in practice. If the escort range were, say, the guard's visionRange/2, that would also be a fixed distance to which the guard would approach the target while guarding them. And basing it on the guard's vision range seems good, because that's the range they'll use to detect enemies (I haven't yet looked a the implementation closely).

Replying to mimo:

But we want also have only ship which can escort ship, that ships can guard harbour but no other buildings, healer which can escort any units, only infantry to guard buildings, ... This class defines which units can escort the escorted.

Hmm, why can't a land unit guard a ship? :) Ships can be attacked by land units, so it makes sense to let land units guard them. Of course if they can't get in range, the order should fail, if the player tried ordering a land unit to guard a distant ship in a naval battle.

Similarly, why can't a ship guard a wall or tower or any other building from other ships? Why can't cavalry guard a building? The restrictions don't make sense to me, was that discussed somewhere?

in reply to:  6 comment:7 by mimo, 11 years ago

Replying to historic_bruno:

Replying to mimo:

I think the problem here is a bit different because we are discussing cases where we would nearly never want to select these units.

You don't think people would want to move animals once they are corralled? What about moving them to a different corral or moving them out of danger. As for indicating which animals are corralled, I think interfering with selections is a terrible way to do it. People will simply wonder why some units can't be selected and think it's a bug with the game (I'd tend to agree). According to Michael, AoE3 had livestock pens and animals just stood around it (see link in #1907), I assume they were still selectable but I'm not sure.

Guarding is the same way, I never had a problem in AoK remembering which units were guarding and which were not, plus you can easily separate units by function/order using selection groups. And if you make a mistake, it's not hard to fix by re-tasking the unit to guard. That's far simpler than out-thinking the player IMO.

But they are selectable. The only thing is that you can't select them by group (double-clicking or grabbing) to avoid mistakes, but single-click still works on them. When testing this patch, I was annoyed by always having to reaffect my guarding units because I accidentally selected them. But ok, this part is just a few lines in the gui which do not interfere with the behaviour of the patch, so fill free to remove them if you don't want them.

Replying to mimo:

This range is not the range of action, which is indeed based on vision range. It's only the distance at which the escort stand with respect to its escorted. It should not be too near to not interfere with its movements, but not too far to really protect it quicly enough.

I don't see the difference in practice. If the escort range were, say, the guard's visionRange/2, that would also be a fixed distance to which the guard would approach the target while guarding them. And basing it on the guard's vision range seems good, because that's the range they'll use to detect enemies (I haven't yet looked a the implementation closely).

Why linking these two distances which are independent ? The choice of this guarding distance is governed by aesthetics and usability : having the guard too far is not nice looking and we quickly do not know anymore who is guarding when they are a lot of units. On the other side, especially for siege units and ships, it should not be too near to not disturb its movements. So the best distance depend on the unit to be escorted, not the escort. Also, it looks pretty weird to me that if one would like to increase vision distance, one would have to care about its implications for escort behaviour.

Replying to mimo:

But we want also have only ship which can escort ship, that ships can guard harbour but no other buildings, healer which can escort any units, only infantry to guard buildings, ... This class defines which units can escort the escorted.

Hmm, why can't a land unit guard a ship? :) Ships can be attacked by land units, so it makes sense to let land units guard them. Of course if they can't get in range, the order should fail, if the player tried ordering a land unit to guard a distant ship in a naval battle.

Similarly, why can't a ship guard a wall or tower or any other building from other ships? Why can't cavalry guard a building? The restrictions don't make sense to me, was that discussed somewhere?

Yes, the possible restrictions are certainly a subject for discussions (and can be easily changed by just modifying the xml), but the important point is if we want to be able to put some. If we do not put any, we have to foresee a lot of possibilities in the UnitAI code which are not done presently. For example, if a ship is able to guard a building, when will we say that the building is out of reach in order to cancel the order ? or may-be the user only wanted the ship to guard the coastline which lead to this building whatever the building distance and we should not abort it ? same situation in case of land unit escorting a ship. All these questions can be let for a future update of the patch, and when worked on, removing a restriction is easy.

comment:8 by Pureon, 11 years ago

#1907 and #2034 are both very useful/functional tickets and it's good to see patches for them. I don't think the F1 key should be used for selecting corraled animals as this won't be an obvious selection key for users who don't read the manual. The Atlas editor uses the Alt key to select objects that are on the map but aren't entities - I'm not sure what the Alt key does right now in-game (I'd need to check when I get home), but i do think it would make sense to align these in some way.

We should also get some of the other guys to look at these tickets, both are important enough to warrant a wider team decision. I'll mention it in saturday's team meeting - hopefully we can agree on something then.

comment:9 by historic_bruno, 11 years ago

Owner: set to mimo

comment:10 by Kieran P, 11 years ago

Milestone: Alpha 14Alpha 15

comment:11 by sanderd17, 10 years ago

Keywords: reviewed added; review removed

IMO, selecting should be handled the same way as with buildings. You can't drag select buildings as long as there are units in your bounding box. But you don't have a problem with double-click selecting or drag selecting when there are no units in the box.

That way, you could omit the special selection key.

There are also some other functionality differences I'd like.

  • When you click on a guarded unit, you should be able to see who is guarding them (a bit like the rally-point flags, or maybe highlighted actors). Maybe also the other way around, show which unit is being guarded by the current unit.
  • It would also be nice if there is a "guard" button, instead of just working via hotkeys. New players need buttons with hotkey-explaining tooltips (similar to garrisoning)
  • Next to that, some unguard command would also be nice instead of commanding a unit to ungaurd (a separate button, and perhaps unguard when pressing the guard button and clicking on some empty spot). It would also make acidentaly selecting and and moving some units less bad, as they keep guarding a unit, and the next time the guarded unit moves, they'll follow him.
  • I also think you're making it extra difficult with those allowed classes. IMO everyone should be able to garrison everyone.
  • You should think a bit about formations. Maybe, when guarding a unit, default to a box formation around the unit? Moving them loosely causes too many collisions, which is bad for performance and looks bad.
  • I understand your reasoning for the escort range, but if you try to align all units in some sort of box formation, that would replace the range (they always try to be in the same relative position). Of course, when guarding a building or big unit, the size of that box center needs to be adapted probably. But that's something we can modify in the formations

I can't comment on the code, as the above requests will change it enormously. But I trust you can write good code.

comment:12 by leper, 10 years ago

Keywords: reviewed removed

by mimo, 10 years ago

Attachment: escort-v2.diff added

by mimo, 10 years ago

Attachment: add-guard.png added

by mimo, 10 years ago

Attachment: remove-guard.png added

comment:13 by mimo, 10 years ago

Here is a new version of the patch, using some suggestions from previous comment by sanderd17.

The special selection key which nobody liked is now removed, and the logic has changed a bit. To remove a guard order, we have to use the command button. So we now may reassign a guarding unit to any other task, and if the guarded unit is attacked, the guarding unit will stop its current order (if it is not a forced order) to rescue the guarded unit. And to stop this guard behaviour, we must use the remove-guard button.

The allowed classes are also removed.

I've implemented the PageUp and PageDown keys, with PageUp (when pressed) highlights the units/buildings guarded by the selection, and PageDown highlights the units guarding the selection.

comment:14 by sanderd17, 10 years ago

Looks a lot better to me. Some small problems though.

The cursor should highlight the units you're hovering when wanting to guard a unit. Currently, it's hard to see which unit you're selecting. This is fine when using the "G" key though, it only happens when you use the button in the GUI.

When a unit is guarding another unit, and moving towards the guarded unit. You can press to "unguard" on the guard, but the guard won't stop moving. He'll just stop the walking animation and slide on.

And, it's possible to make a chain of guarding units in one direction, but not in the other. I think such a chain should never be possible, and the cursor should also be updated to show you can't guard a unit because it's a guard itself.

It would also be nice if you can press "G", and lick an empty space to unguard.

Apart from those issues, it looks like this patch is nicely taking shape.

by mimo, 10 years ago

Attachment: escort-v2.2.diff added

by mimo, 10 years ago

Attachment: action-guard-disabled.png added

by mimo, 10 years ago

Attachment: action-remove-guard.png added

in reply to:  14 ; comment:15 by mimo, 10 years ago

Thanks for the comments, I've fixed all the issues you've noticed in the new patch attached.

Replying to sanderd17:

Looks a lot better to me. Some small problems though.

The cursor should highlight the units you're hovering when wanting to guard a unit. Currently, it's hard to see which unit you're selecting. This is fine when using the "G" key though, it only happens when you use the button in the GUI.

Yes, I've also added it for the other preSelection actions (i.e. garrison and repair buttons also)

When a unit is guarding another unit, and moving towards the guarded unit. You can press to "unguard" on the guard, but the guard won't stop moving. He'll just stop the walking animation and slide on.

Thanks for noticing it. That's now fixed.

And, it's possible to make a chain of guarding units in one direction, but not in the other. I think such a chain should never be possible, and the cursor should also be updated to show you can't guard a unit because it's a guard itself.

You are right, chains were only protected in one direction. Both ways are now prevented. And the cursor not updating was because I uploaded a wrong version of the png. A new one is now available.

It would also be nice if you can press "G", and lick an empty space to unguard.

Nice idea, but may-be clicking would be enough :) That's now implemented

Apart from those issues, it looks like this patch is nicely taking shape.

in reply to:  15 comment:16 by sanderd17, 10 years ago

Replying to mimo:

Replying to sanderd17:

It would also be nice if you can press "G", and lick an empty space to unguard.

Nice idea, but may-be clicking would be enough :) That's now implemented

lol, then we're back to the original patch :D

I'll test the patch again.

comment:17 by sanderd17, 10 years ago

I like the patch behaviour, and the code is good too IMO. So for me, it's go to go in for A15.

comment:18 by leper, 10 years ago

Additional with one n.

The "enter" and "leave" states in "GUARD" aren't needed (and they are empty) so they should be removed. The comment in UnitAI line 4283 needs a 'unit' after 'guarded'. UnitAI.AddGuard() could use some empty lines to make it more readable.

Apart from that the code does look good, but I haven't tested the patch. (I trust you and sanderd17 on the fact that it is working and doesn't break anything)

comment:19 by mimo, 10 years ago

Resolution: fixed
Status: newclosed

In 14263:

allow units to guard/escort buildings or units, fixes #2034

comment:20 by Marcio, 10 years ago

Thank you for do it. Try check my list about game commands to know how can be the next.

comment:21 by Freagarach, 4 years ago

In 24033:

Allow guards to guard a guarding guard.

Explicitly disallowed in the past (see ticket), it now becomes possible to e.g. guard a catafalque with a hero and guard the hero with healers.

Refs #2034.

Differential Revision: D2732
Reviewed by: @Angen, @bb.

Note: See TracTickets for help on using tickets.