Page MenuHomeWildfire Games

Fix the aiming code when performing an attack.
ClosedPublic

Authored by fatherbushido on Dec 29 2016, 11:30 AM.

Details

Summary

That's explained in #4276.
When performing an attack in PerfomAttack function of Attack.js, we compute an expected position of the target (aiming) taking account of its movement and of the spread.
But It could occur that the expected target pos is badly out of the attack range.

See ​http://trac.wildfiregames.com/attachment/ticket/4276/fireoutofrange.mkv

There is different way to fix it, I suggested to just cancel the attack.

Test Plan

It is easily shown with a catapult due to the high prepare/repeat time.
For example, with a catapult, and a fleeingn cav (fleeing radially), we can have that:

WARNING: selfPosition({x:617.5589752197266, y:20, z:606.6230926513672}) WARNING: targetPosition({x:550.9756622314453, y:19.999374389648438, z:676.3333740234375}) WARNING: previousTargetPosition({x:553.6228485107422, y:19.999374389648438, z:674.0438385009766}) WARNING: elevationAdaptedMaxRange80.03369140625 WARNING: rand[0.607085014306797, 0.6238512424861701] WARNING: realHorizDistance255.58346950339396

As the realHorizDistance is really bigger than the attack range of the catapult, we should not perform the attack.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

fatherbushido retitled this revision from to Don't perform attack when the aimed unit is out of range..
fatherbushido updated this object.
fatherbushido edited the test plan for this revision. (Show Details)
fatherbushido set the repository for this revision to rP 0 A.D. Public Repository.
Vulcan added a subscriber: Vulcan.Dec 29 2016, 12:13 PM

Build is green

Updating workspaces
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimation]’:
FCollada/FCDocument/FCDLibrary.cpp:149:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
  const T* cptr = ((const FCDLibrary<T>*)l1)->GetEntity(0);
           ^
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimationClip]’:
FCollada/FCDocument/FCDLibrary.cpp:150:34:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDCamera]’:
FCollada/FCDocument/FCDLibrary.cpp:151:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDController]’:
FCollada/FCDocument/FCDLibrary.cpp:152:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEffect]’:
FCollada/FCDocument/FCDLibrary.cpp:153:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEmitter]’:
FCollada/FCDocument/FCDLibrary.cpp:154:28:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDForceField]’:
FCollada/FCDocument/FCDLibrary.cpp:155:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDGeometry]’:
FCollada/FCDocument/FCDLibrary.cpp:156:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDImage]’:
FCollada/FCDocument/FCDLibrary.cpp:157:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDLight]’:
FCollada/FCDocument/FCDLibrary.cpp:158:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:159:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDSceneNode]’:
FCollada/FCDocument/FCDLibrary.cpp:160:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsModel]’:
FCollada/FCDocument/FCDLibrary.cpp:161:33:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:162:36:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsScene]’:
FCollada/FCDocument/FCDLibrary.cpp:163:33:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
In file included from FCollada/FUtils/FUSemaphore.cpp:10:0:
FCollada/FUtils/FUSemaphore.h:36:2: warning: #warning "FUSemaphore: Semaphore not implemented for non Windows" [-Wcpp]
 #warning "FUSemaphore: Semaphore not implemented for non Windows"
  ^
FCollada/FUtils/FUStringConversion.cpp: In function ‘void TrickLinkerFUStringConversion()’:
FCollada/FUtils/FUStringConversion.cpp:281:8: warning: variable ‘f’ set but not used [-Wunused-but-set-variable]
  float f = FUStringConversion::ToFloat(&c);
        ^
FCollada/FUtils/FUStringConversion.cpp:283:7: warning: variable ‘b’ set but not used [-Wunused-but-set-variable]
  bool b = FUStringConversion::ToBoolean(c);
       ^
FCollada/FUtils/FUStringConversion.cpp:285:8: warning: variable ‘i32’ set but not used [-Wunused-but-set-variable]
  int32 i32 = FUStringConversion::ToInt32(&c);
        ^
FCollada/FUtils/FUStringConversion.cpp:287:9: warning: variable ‘u32’ set but not used [-Wunused-but-set-variable]
  uint32 u32 = FUStringConversion::ToUInt32(&c);
         ^
In file included from FCollada/FUtils/FUThread.cpp:10:0:
FCollada/FUtils/FUThread.h:30:2: warning: #warning "Threads not yet implemented for non Windows." [-Wcpp]
 #warning "Threads not yet implemented for non Windows."
  ^
FCollada/FCDocument/FCDAnimationCurve.cpp: In member function ‘float FCDAnimationCurve::Evaluate(float) const’:
FCollada/FCDocument/FCDAnimationCurve.cpp:396:13: warning: ‘inTangent.FMVector2::<anonymous>.FMVector2::<anonymous union>::x’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   FMVector2 inTangent;
             ^
FCollada/FCDocument/FCDAnimationCurve.cpp:396:13: warning: ‘inTangent.FMVector2::<anonymous>.FMVector2::<anonymous union>::y’ may be used uninitialized in this function [-Wmaybe-uninitialized]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimation]’:
FCollada/FCDocument/FCDLibrary.cpp:149:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
  const T* cptr = ((const FCDLibrary<T>*)l1)->GetEntity(0);
           ^
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimationClip]’:
FCollada/FCDocument/FCDLibrary.cpp:150:34:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDCamera]’:
FCollada/FCDocument/FCDLibrary.cpp:151:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDController]’:
FCollada/FCDocument/FCDLibrary.cpp:152:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEffect]’:
FCollada/FCDocument/FCDLibrary.cpp:153:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEmitter]’:
FCollada/FCDocument/FCDLibrary.cpp:154:28:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDForceField]’:
FCollada/FCDocument/FCDLi

http://jw:8080/job/phabricator/40/ for more details.

I am not too fond of this change, honestly, it's just going to make it even harder to track a fleeing unit. Of course, catapults won't track much, so for them it's not an issue, but for archers this just means that all started attacks may fail even worse, which is annoying.

or something like if (realHorizDistance > 1.1 * elevationAdaptedMaxRange) ?

elexis added a subscriber: elexis.

Nice find. These ultralongrange catapult shots were infamous when they tried to hit ships. For catapults this can likely be removed though, as they now have a terrible chance of dealing damage anyway (unless hitting another unit in the actually struck location).

Arrows reach their target quickly, so archers are likely not too badly impacted.
Perhaps it is a more relevant change to skirmishers, as they have a short range and units often retreat out of attack range while the skirmishers are attacking them. This might nerf them in these situations. Testing this a bit with and without the patch would help judging the balancing impact (perhaps increasing the units attack range if so).

fatherbushido added a comment.EditedDec 30 2016, 9:26 AM

Thanks for looking at it.
As pointed out by wraitii, if the archers, javeliners is at max range when aiming (and during that time, the target fly away), the condition

realHorizDistance > elevationAdaptedMaxRange

is a bit too restrictive, adding a tolerance (10% ?) is perhaps neccesary. (Another option is just to fire in the target direction but at max range, it could still hit another unit perhaps).

I think we should tolerate up to Walkspeed * some amount of leeway, for javelineers particularly. A better fix would be to let the unit get a little closer before firing, but that's a non-trivial change to unitAI/Motion.
Also fine with just shooting at nothing anyway, I think it's better than straight aborting.

elexis edited edge metadata.Dec 30 2016, 1:42 PM
elexis added a subscriber: mimo.

quick r19085 replay showing that 8 cavalry that were dancing around near 2 skirmishers lost 20% HP in the unpatched code and 0% in the patched file, so the balancing effect should indeed be considered.

10% margin seems a bit arbitrary, definitely wouldn't increase the code complexity. Perhaps +10 meters is more useful than +10%. Wouldn't be bad to have such a const as a proto member, if not even in the templates.

Perhaps @mimo has a good advice.

fatherbushido updated this revision to Diff 107.EditedJan 2 2017, 1:57 PM
fatherbushido edited edge metadata.

When we try to aim out of range, rescale to fire at max range.

EDIT: I think we have in this one the expected behavior. UnitAI tries to go at / checks range. Then attack tries to aim and if the target is too far we fire as far as we can in that direction.

Vulcan added a comment.Jan 2 2017, 3:00 PM

Build is green

Updating workspaces.
Build (release)...
../../../source/gui/CChart.cpp:38:41: warning: unused parameter ‘Message’ [-Wunused-parameter]
 void CChart::HandleMessage(SGUIMessage& Message)
                                         ^
../../../source/lib/tex/tex_png.cpp: In member function ‘virtual Status TexCodecPng::encode(Tex*, DynArray*) const’:
../../../source/lib/tex/tex_png.cpp:309:9: warning: variable ‘ret’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Wclobbered]
  Status ret = ERR::FAIL;
         ^
Build (debug)...
../../../source/gui/CChart.cpp:38:41: warning: unused parameter ‘Message’ [-Wunused-parameter]
 void CChart::HandleMessage(SGUIMessage& Message)
                                         ^
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/72/ for more details.

elexis added a comment.Jan 4 2017, 6:15 AM

So it shoots at max distance if the unit got out of attack range? Doesn't that mean that the arrow still misses? I guess it has a good chance of hitting other units that retreated. To be tested and shouldn't shy away from compensating balancing values if needed.

fatherbushido added a comment.EditedJan 4 2017, 11:10 AM

It can indeed hurt other units (or even hit that one (as attack only try to aim at an expected location))

edit: I have also check, we naturally fall into that if in about 10% of the case wich is not nothing.
Also if the difference is smaller than the footprint, we have nothing changed relating to damage.
In regard of gameplay, finally in multiple units combat, there is nothing wrong. In single unit fight (mostly for example ranged units against women or idle units, we'll be really rarely in that case).

elexis added a comment.Jan 9 2017, 4:52 PM

Considering the replay, where 8 cavalry units are chased by 3 skirmishers units, with the most recent patch the 8 cavalry units had 1234/1268HP remaining, i.e. 2,7% HP lost.
Without the patch, it were 1061/1268HP remaining, i.e. 16,3% HP lost.
So I still predict lobby players accusing us of nerfing skirmishers.

(Wasn't hunting impaired by rP18828 / rP18828 as there wasn't damage dealt anymore on miss?).
Attaching a second replay

revealing the effects on hunting. One cavalry skirmisher receives an order to move away from the civic center, then attacking a deer, so that the deer runs towards the civic center and ideally dies close to it.
Without the patch the animal dies in proximity to the civic center after some seconds.
With the patch, the cavalry chases the animal beyond the civic center, reaching the map border, then chasing it around the map border for more than 3 minutes, almost reaching the enemy base before the replay ends.

I accept the logic change to be correct, but I do raise concern about getting it committed without working out and addressing the balancing impact before the next release.

elexis added a comment.Jan 9 2017, 4:54 PM

Also if one were funny, one could add a test to validate that a unit out of range wasn't hurt.

Thanks for the tests!
In bigger battle (in fact ranged units are more for that), the result would be better.
But for hunting it's annoying. It's mainly unitAI fault :p (wich try to be at max range iirc).

Given those results also confirm my fear, I'd refrain from committing this for now.

My current work on UnitMotion should help fixing it by making it easier for units to go closer to their target before starting to attack, and by potentially allowing attack while moving. At that point, we can add this back in.

Didn't we anticipate a general accuracy buff anyway? The result of this diff might look completely different if that was done.

In D20#1764, @elexis wrote:

Didn't we anticipate a general accuracy buff anyway? The result of this diff might look completely different if that was done.

it s more the fact that unitai is too clever by trying to put us at max range.

That's a really dumb move BTW. UnitAI should try to put us around mid-range.
But then you'd have to adapt the "should chase" logic to handle units being sufficiently fast that we'll never catch them.

And the case elexis tested is exactly one the worse (with that patch).

So if the issue is that hunting isn't possible without a greater range and if the unit had an effectively greater range before, we might want to increase the range slightly while keeping in mind that increasing the range also nerfs accuracy, which might be a good implicit antagonist.

elexis: increasing the range won't change anything :/ The matter is that unitAI place you at the max range. So the aimed unit wich is leaving will almost always be out of range (if the way it moves in one turn seems to place it out of range).

Fatherbushido is correct. Here's how the problem appears:
-UnitAI is ordered to attack an enemy
-UnitAI tells UnitMotion to get in range [min,max], but if we're outside that range initially UnitMotion will try to bring us to Max-Range.
-unit gets to max range and unitMotion tells unitAI it's done walking
-UnitAI sees it's indeed in range and starts firing.
-Enemy gets hit, starts to flee, is immediately out of range. By the time UnitAI's attack goes through, the enemy is already out of range, even if only by a tiny amount. This only gets worse as time goes on.
-UnitAI sees after a timer call it's out of range. Go back to step 2. Repeat.

To fix this, we need to make unitAI tell us to go to "somewhere between min range and max range". This would work OK for hunting, however an enemy player could still cheese it by using the time spent approaching to attack you once, then move away with his faster units before to get out of range again by the time the attack completes (in which case it's even a little worse since the unit will actually get attacked).

As far as I can tell, the proper fix here would be to make UnitAI much cleverer:
-Go to "range in between" from the unit.
-If the unit is not fleeing, attack ASAP to avoid the cheese I talked about above.
-if the unit is fleeing, wait to arrive (or if we're outrun, just stop)
(Or, alternatively, we need to allow attacking-while-walking.)

This requires changing the Combat.Approaching/Chasing behaviors to check ranges on a timer-base, on top of listening to MoveCompleted. This is something I'll be doing anyways for my unitMotion Rewrite, so this is probably not the time.

If we had what I describe above, then the problem of units not hitting their target would be reduced. But, with a unit with a faster attack rate, it could probably still be cheesed a bit, so I do suggest we allow 5/10% overthrow anyways.

elexis added a comment.EditedJan 12 2017, 2:09 PM

Recapitulation:

Problem: The code contradicts the design decision of the maximum range manifested in the templates.
Requirements:

  • UnitAI orders the unit to move to MaxRange before attacking. If the unit isn't allowed to shoot further than MaxRange and the target flees, there will be an infinite chase.
  • Therefore either the unit needs to shoot further than defined in the templates (as it was before),
  • or the unit moves closer than maxRange to the target before shooting

Alternative Proposals:
(1) Add a new field OvershootRange to the templates, solving the contradiction by letting the balancing designer decide.
(2) Add a new field TargetRange to the templates and change UnitAI to move to that range before shooting, instead of using MaxRange.

Er, or (3) make unitAI cleverer without tempering with the templates, we already have all the info we'd need.

In D20#1876, @wraitii wrote:

Er, or (3) make unitAI cleverer without tempering with the templates, we already have all the info we'd need.

(2) would be the range how far "in between" is. Hardcoding a number like 80% maxRange is less flexible.

I d suggest to suspend it a bit.

@elexis: but that's still hardcoded. My whole point is that unitAI can be told to pick a clever in-between point.
For example if we're attacking an enemy ranged unit, and our range is higher, it's a good idea to stay just out of range. If we're attacking a really fast unit, we'll want to get proportionally closer. It could also depend on our stance, and so on.

fatherbushido retitled this revision from Don't perform attack when the aimed unit is out of range. to Fix the aiming code when performing an attack..Feb 4 2017, 7:10 PM
fatherbushido added a comment.EditedFeb 4 2017, 7:19 PM

Well the current approximation is itself a bit wrong.
I will first write a "test" to show some weird cases.
In that diff I will mainly adress the bugs of the aiming part (relative to proj - target speed).
I will not adress the unitAI bugs (relative to the timers - target moving). One of those bugs exists also for melee attack.

nothing anymore to review here for the moment.

fatherbushido planned changes to this revision.Feb 19 2017, 5:55 PM
elexis changed the visibility from "All Users" to "Public (No Login Required)".Mar 30 2017, 6:08 PM
Self position, missile horizontal speed, target position, target velocity vectorTarget position at collision timeMissile position at collision time
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(0, 0, 0));idle target({x:20, y:0, z:0})({x:20, y:0, z:0})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(1, 0, 0));radial flee slow({x:26.666666666666668, y:0, z:0})({x:26.666666666666668, y:0, z:0})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(4, 0, 0));radial flee same speed({x:Infinity, y:NaN, z:NaN})({x:NaN, y:NaN, z:NaN})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(16, 0, 0));radial flee fast({x:-6.666666666666668, y:0, z:0})({x:46.66666666666667, y:0, z:0})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(-1, 0, 0));radial approach slow({x:16, y:0, z:0})({x:16, y:0, z:0})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(-4, 0, 0));radial approach same speed({x:10, y:0, z:0})({x:10, y:0, z:0})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(-16, 0, 0));radial approach fast({x:4, y:0, z:0})({x:4, y:0, z:0})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(0, 0, 1));tangential slow({x:20, y:0, z:5})({x:20, y:0, z:5.153882032022076})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(0, 0, 4));tangential same speed({x:20, y:0, z:20})({x:20, y:0, z:28.284271247461902})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(0, 0, 16));tangential fast({x:20, y:0, z:80})({x:20, y:0, z:329.84845004941286})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(1, 0, 1));45 deg flee({x:26.666666666666668, y:0, z:6.666666666666667})({x:26.871842709362767, y:0, z:6.871842709362768})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(2, 0, 2));45 deg flee({x:40, y:0, z:20})({x:42.3606797749979, y:0, z:22.360679774997898})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4 , new Vector3D(20, 0, 0), new Vector3D(8, 0, 8));45 deg flee({x:-20, y:0, z:-40})({x:109.44271909999159, y:0, z:89.44271909999159})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(-1, 0, 1));45 deg approach({x:16, y:0, z:4})({x:15.876894374382339, y:0, z:4.123105625617661})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(-2, 0, 2));45 deg approach({x:13.333333333333332, y:0, z:6.666666666666667})({x:12.546440075000701, y:0, z:7.453559924999299})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(-8, 0, 8));45 deg approach({x:6.666666666666666, y:0, z:13.333333333333334})({x:-9.814239699997195, y:0, z:29.814239699997195})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4, new Vector3D(20, 0, 0), new Vector3D(4, 0, 2));fleeing with same radial speed({x:Infinity, y:NaN, z:Infinity})({x:Infinity, y:NaN, z:Infinity})
testPredictTargetPosition(new Vector3D(0, 0, 0), 4 , new Vector3D(20, 0, 0), new Vector3D(-4, 0, 2));({x:10, y:0, z:5})({x:8.819660112501051, y:0, z:5.5901699437494745})

I posted above what currently the predicted target function do.

How does your speed vector translate to in-game unit speeds?

I have compute the exact solutions which I will submit in a patch

In D20#12632, @wraitii wrote:

How does your speed vector translate to in-game unit speeds?

the speed vector is just the ratio between the displacement between a turn and that turnlength. So it's not the template target speed but the actual one (assuming it will move in straight line).

In the cases where the aim has no solution, we should just fire the current target position (as the aim fail).

Fix the predict target position function

So I guess https://code.wildfiregames.com/D20#12630 shows how weird the current target function is wrong.
Now we compute the exact solution (a preliminary math study prevent to do useless computations/comparaisons) and return false when there is no solution. In the false case, we don't try to aim and just target the current target position.
That thing should be followed by a spread template value rework and perhaps a speed projectile cleaning too.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/785/ for more details.

Fix missing spaces

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/786/ for more details.

wraitii accepted this revision.Apr 16 2017, 5:31 PM
This revision is now accepted and ready to land.Apr 16 2017, 5:31 PM
This revision was automatically updated to reflect the committed changes.