#1960 closed enhancement (fixed)
[PATCH] Elevation advantage for ranged units
Reported by: | sanderd17 | Owned by: | peter |
---|---|---|---|
Priority: | Nice to Have | Milestone: | Alpha 14 |
Component: | UI & Simulation | Keywords: | patch |
Cc: | Patch: |
Description
As talked about on the forum, elevation advantages for ranged units are nice to have.
http://www.wildfiregames.com/forum/index.php?showtopic=17358
Attachments (3)
Change History (16)
comment:1 by , 11 years ago
Keywords: | patch,review → patch review |
---|---|
Milestone: | Backlog → Alpha 14 |
Summary: | [PATCH]Elevation advantage for ranged units → [PATCH] Elevation advantage for ranged units |
comment:2 by , 11 years ago
comment:3 by , 11 years ago
Did some more modifications. I appended RangeManager to support parabolic queries. And I also introduced parabolic length comparison in the Vector3D file.
So now (with direct queries), the building range checking is a lot better.
I still have problems with the Unit range checking though. That code still contains sqrt-s, and is calculated completely on the JS side. Mainly because the MoveToTargetRange and CheckTargetRange take target outline into account. And I haven't found the way to change that.
Btw, looking at the UnitMovement code, that contains a lot of roots (with .Length() calculation), while some cases, it could also be compared with just length comparison, instead of length calculation. Which saves an sqrt.
The patches replace the old ones, and are split per directory.
comment:4 by , 11 years ago
Hi, in the future when you post patches, could you create only a single patch with all the related changes? It makes it easier to review and to track updates.
comment:6 by , 11 years ago
Cleaned up the code
- Replaced spaces with tabs where needed
- decided not to change FixedVector3D, and instead added a static method to RangeManager
- renamed some methods so they have more meaningful names
Next to that, range overlay in the developer mode now shows the correct ranges (with a precision of one tile to save CPU time)
For me, it's ready as I don't think I can improve it a lot further.
Getting rid of the SQRTs in UnitAI.js would be great, but requires deep changes in UnitMotion.cpp.
comment:7 by , 11 years ago
Uploaded a new file. This one includes gui changes (showing the average advantage when placing a foundation in a tooltip, and adapting the range value in the GUI to something more comprehensive).
It also contains an extension to the ranged attack XML schema that can be used. Including an example for the defense towers. If this is good, some other buildings also can get this tag (wall towers, outposts, ...).
by , 11 years ago
Attachment: | elevation_advantage_with_gui.2.diff added |
---|
comment:8 by , 11 years ago
The previous diff was wrong (cleaned it up a bit too much).
And I forgot to check the "overwrite" checkmark. But both are the same now.
comment:9 by , 11 years ago
As this patch contained code that split ranged attacks from melee attacks, I took the opportunity to also fix #1731.
comment:10 by , 11 years ago
I reverted my latest change for the following reasons:
- My original patch doesn't require re-balancing, while fixing #1731 requires rebalancing as relative ranges between units and buildings change (ballistas get in reach of fortresses and similar)
- It's better to only address one issue at a time, so that solution can be tested completely, without interference of other changes.
comment:13 by , 8 years ago
Keywords: | review removed |
---|
I noticed there are still some problems with it, at a sudden point in game, I got this error:
So that still needs to get fixed.