Opened 13 years ago

Closed 12 years ago

Last modified 8 years ago

#745 closed enhancement (fixed)

[PATCH] Setting Rally Points on Resources/Buildings

Reported by: Mark Owned by: Jonathan Waller
Priority: Nice to Have Milestone: Alpha 8
Component: Core engine Keywords:
Cc: vts Patch:

Description

You should be able to set rally points on resources so that citizens will automatically gather that type of resource when they are created.

Things to think about:

[15:16] <@Philip`> (How does it work if e.g. the resource gets exhausted while it's the gather point?)

[15:16] <@Philip`> (or if e.g. it's an animal that has run away?)

[15:20] <mark__> I think in AoK that if the resource was exhausted that they just searched for a nearby matching resource. As if they had just used it up.

[15:20] <mark> Otherwise they stood there like a normal rally point.

Attachments (5)

rallypoint_on_entities.diff (10.9 KB ) - added by Jonathan Waller 12 years ago.
rallypoint_on_entities2011_12_5.diff (10.8 KB ) - added by Jonathan Waller 12 years ago.
RallyPointCommands.js (1.4 KB ) - added by Jonathan Waller 12 years ago.
rallypoint_on_entities_2011_12_10.patch (12.2 KB ) - added by Jonathan Waller 12 years ago.
rallypoint_on_entities2011_12_16_2.diff (12.7 KB ) - added by Jonathan Waller 12 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 by Jonathan Waller, 12 years ago

Owner: set to Jonathan Waller
Status: newassigned

by Jonathan Waller, 12 years ago

Attachment: rallypoint_on_entities.diff added

comment:2 by Jonathan Waller, 12 years ago

Keywords: review added
Milestone: BacklogAlpha 8
Summary: Setting Rally Points on Resources[PATCH] Setting Rally Points on Resources

Added a patch for this. A problem is that when setting the rally point on a large enough entity it gets hidden inside the entity so may be partially visible of completely hidden. The new line to rally point patch would mitigate this a bit but it should be done properly at some point.

This patch adds an extra command for the UnitAI's which is gatherNearPosition which reuses the code for units which are gathering a resource which runs out. This satisfies the conversation in the irc log.

This patch adds more functionality by handling all types of rally point commands, so if it is set on a building which needs constructing/repair, a garrisonable building or an enemy then the appropriate action will be taken. If the original target is destroyed then the rally point reverts to a normal walk to location.

comment:3 by Erik Johansson, 12 years ago

This sounds really nice. Pity I can't work out how to properly add a patch or I'd tested it just to experience it :)

comment:4 by historic_bruno, 12 years ago

OK it looks good so far, but here are some comments :)

  • The new rally behavior only seems to affect trained units, not ungarrisoned units, I think we will want both.
  • I can't set a rally point on attackable units if the building has an attack component, for example the civil centre can't rally at enemy units or buildings. I think it's best to keep attack as the higher priority behavior, so maybe we need a new set rally point button to override that? It could go next to the garrison buttons in the unit details pane.
  • How about also changing the cursor while setting the rally point, so it's clear that your units will gather, garrison, etc. instead of walking?
  • Is it possible to place the rally point flag on top of whatever the target is, so it's hopefully always visible? Or we could opt for a different visual indicator like highlighting the target's selection ring. That's probably an issue for further down the road if someone wants to fix it.

by Jonathan Waller, 12 years ago

by Jonathan Waller, 12 years ago

Attachment: RallyPointCommands.js added

comment:5 by Jonathan Waller, 12 years ago

Summary: [PATCH] Setting Rally Points on Resources[PATCH] Setting Rally Points on Resources/Buildings

I updated the patch. I moved some code into RallyPointCommands.js which should be placed in simulation/helpers (because I get confused by patch/diff software it is a separate attachment).

Now garrisoned units share the same code.

Setting rally points on attackable units just ends up with a walk command, this seems best because behavior with mobile enemies is ambiguous and might be confusing.

The cursor changes properly now.

vts might be able to get the rally point flag working better with his patch, it would be best ot keep it outside the scope of this one for simplicity.

by Jonathan Waller, 12 years ago

comment:6 by Jonathan Waller, 12 years ago

I have attached a new version of the patch, this merges the code with vts's changes and also reorders the command checks in input.js so that a hunting (food technically) rally point is set rather than the building attacking them (or at least trying since buildings don't actually attack animals when you tell them to).

Known issues: *Rally points cannot be unset *Flag renders inside of large entities. Possible solutions are making the flag ignore the z buffer (my preference), placing the flag where the rally point path meets the entity, placing the flags above the entity (looks silly with trees) *Vts's rally point path drawing code disagrees with the rally point being set at the centre of the rally points owner, and does not update the path.

in reply to:  6 comment:7 by historic_bruno, 12 years ago

Cc: vts added

Replying to quantumstate:

Vts's rally point path drawing code disagrees with the rally point being set at the centre of the rally points owner, and does not update the path.

Hmm this is annoying. Vts gave the impression in IRC that this would be easy for him to fix by clearing the rally line data every time it fails to generate a new one. Is that reasonable? I don't know if your patch can do anything to solve this.

Some additional comments:

The cursor changes properly now.

It would be great if the art team creates some new cursors. Perhaps one for setting a rally point (instead of the default arrow cursor) and then add variations of that for each possible action. That would be a separate ticket, if it's something we want.

a hunting (food technically) rally point is set rather than the building attacking them

Which is confusing because if I set a rally point on a deer and then train some females, they will walk up to the spot where the deer was, which will run away, but the females give up and walk to the nearest food source (might be a berry bush or farm). The rally point was explicitly set on a deer which indicates they should prefer hunting, at least to me. I guess the reason this could matter is if hunting provides advantages over other forms of gathering food (faster gather rate?).

Related is setting rally points on attackable enemies. I'm sure we could figure out a solution which is not ambiguous, maybe a concept similar to the "gather near" action, there could be an "attack if still in LOS" action, where the unit would arrive at the rally point and then test if the target is still a.) exists, b.) is visible to the player and c.) attackable. If all are true the unit would try to attack the target as in a normal attack, or else do nothing.

These are not big problems but worth leaving as TODOs.

Rally points cannot be unset

I still like the idea of a modifier hotkey for this, but we can see how it works in practice, if many people complain we can implement it :)

comment:8 by Kieran P, 12 years ago

Milestone: Alpha 8Alpha 9

comment:9 by historic_bruno, 12 years ago

Two other problems with autogarrisoning:

  • I don't like how it spawns the units and then issues a separate garrison order, it should be direct-to-garrison, but...
  • This behavior should only affect the training queue, so it's still possible to ungarrison units. At least that's how AOK implemented it, because there's no sense to regarrison in the same building. In other words, using ungarrison should always spawn the units outside the building, no matter where they go next.

Because the rally line bug is also triggered by autogarrisoning, would it be hard to leave that part out of your patch? I might have made the suggestion for adding autogarrisoning, but didn't really discuss or think about all these cases :/

by Jonathan Waller, 12 years ago

comment:10 by Jonathan Waller, 12 years ago

Thanks for your comments, they are helpful as always. These things are always more complicated than I expect. I have updated the patch again. I disabled setting the rally point on the building itself because this needs more advanced behavior as you said.

I changed the behavior so that it uses the specific resource type rather than the generic, I had thought that using the generic would be better for some reason, but specific is less confusing and matches the behavior for when a resource is depleted so it is more consistent as well.

The still seems to be a bug in vtsj's code, now when the rally point is unset (by trying to set it in the building itself) the marker and line vanish as they should, however when you deselect the building and select it again the marker reappears. I was able to reproduce this bug with a clean svn so my new patch is orthogonal to this problem.

comment:11 by Jonathan Waller, 12 years ago

Resolution: fixed
Status: assignedclosed

(In [10744]) Allows setting rally points on resources and buildings. closes #745

comment:12 by Kieran P, 12 years ago

Milestone: Alpha 9Alpha 8

comment:13 by Kieran P, 12 years ago

Type: defectenhancement

comment:14 by sanderd17, 8 years ago

Keywords: review removed
Note: See TracTickets for help on using tickets.