Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#2190 closed defect (fixed)

[PATCH] Garrison loading range should depend on obstruction

Reported by: sanderd17 Owned by: wraitii
Priority: Should Have Milestone: Alpha 16
Component: UI & Simulation Keywords:
Cc: Patch:

Description

GarrisonHolder currently has a ridiculous "loadingRange" variable, while it shouldn't depend on the structure, but on the obstruction size of the unit. You'll see for example that the loading range of fortresses is a lot bigger, because it's expected to garrison large siege units.

The patch calculates a better range based on the unit size (so all units walk until they hit the wall).

This also fixes the Mauryan elephant that couldn't be garrisoned in a CC because it's too big.

Attachments (1)

betterGarrisonRange.patch (16.6 KB ) - added by sanderd17 10 years ago.

Download all attachments as: .zip

Change History (10)

by sanderd17, 10 years ago

Attachment: betterGarrisonRange.patch added

comment:1 by Yves, 10 years ago

I think the loading range is meant as a workaround for situations where it's sometimes difficult to really "touch" the target. For ships it could be desirable to allow garrisoning when units are a little bit further away than for a fortress. If the loading range depends on the garrison holder, this gives us more flexibility because we can apply that workaround for ships but leave it for a tower or a fortress. However, I'm not sure if it is really required for ships. Have you tested it?

Another approach would be taking the unit radius into consideration instead of calculating it as if it was a dot in CCmpUnitMotion::IsInTargetRange. So essentially doing what you did in addition to the loadingRange.

I'm wondering if the range problem for archers and buildings is fixed where it depended on the angle which one has the longer range. That looks like the same issue (not considering the building obstruction in this case).

Last edited 10 years ago by Yves (previous) (diff)

comment:2 by sanderd17, 10 years ago

because ships can sail up to a depth of 1m, and units can still walk in 2m deep water, there is always an overlap region (at least when the tiles are passable). So in any practical situation I tested, it worked without problems, and I couldn't think of a situation where it wouldn't work.

comment:3 by wraitii, 10 years ago

Does it work with a very abrupt cliff and a ship next to it? Units would have to go directly from land to ship.

comment:4 by sanderd17, 10 years ago

If the cliff is smooth enough to be passable, it works.

comment:5 by leper, 10 years ago

Milestone: Alpha 15Alpha 16

comment:6 by Marcio, 10 years ago

Ok if it's this the reason why some units like elephants can be garrison in some structures I'll closed the other ticket.

comment:7 by wraitii, 10 years ago

Owner: set to wraitii
Resolution: fixed
Status: newclosed

In 14428:

Take unit obstruction size into account when checking if in range for garrisoning. Reduce CC loading range in consequence. Fixes #2190 .

comment:8 by wraitii, 10 years ago

Keywords: patch review removed

Kept the loading range (I agree it can be useful), added sanderd's obstruction code.

comment:9 by Palaxin, 8 years ago

Summary: [Patch] Garrison loading range should depend on obstruction[PATCH] Garrison loading range should depend on obstruction
Note: See TracTickets for help on using tickets.