2019-03-21 07:05 CET

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0006158Spring engineGeneralpublic2019-03-14 01:29
ReporterGoogle_Frog 
Assigned ToKloot 
PrioritynormalSeverityminorReproducibilityalways
StatusresolvedResolutionreopened 
Product Version104.0 +git 
Target VersionFixed in Version104.0 +git 
Summary0006158: 104.0.1-1060-g7f3541f maintenance Elevated hold position ballistic trajectory weapon target aquisition failure
DescriptionUnits with ballistic weapons that gain range with elevation do not automatically acquire targets beyond their usual range when set to hold position. I do not know if non-hold position movestates are affected.

The 'targetID == -1' interpretation of AllowWeaponTarget is taken into account in ZK, and so far no gadget uses it. I split it into AllowUnitTargetRange as follows: https://github.com/ZeroK-RTS/Zero-K/blob/master/LuaRules/gadgets.lua#L1374
TagsNo tags attached.
Checked infolog.txt for lua ErrorsYes
Attached Files

-Relationships
+Relationships

-Notes

~0019820

Kloot (developer)

https://github.com/spring/spring/blob/maintenance/rts/Sim/Units/CommandAI/Command.h#L109

https://github.com/spring/spring/blob/maintenance/rts/Sim/Units/CommandAI/MobileCAI.cpp#L1124 [extraRadius = 200.0f * owner->moveState * owner->moveState]

https://github.com/spring/spring/blob/maintenance/rts/Sim/Units/CommandAI/MobileCAI.cpp#L1130 [eventHandler.AllowWeaponTarget(owner->id, -1, -1, 0, &searchRadius)]

The automatic CAI target selection code has no concept of elevation differences, implementing that is up to you.

~0019826

Google_Frog (reporter)

Just to be clear, is it correct that all games are now expected to re-implement the formula for height-based range adjustment so that target acquisition can work correctly? This seems like a featured desired by all games that don't use strictly cylindrical target volumes, and replicating this behaviour in lua seems like an expensive waste of performance.

Are you aware that the engine used to acquire such targets correctly? Even if the engine was previously acquiring such targets with some sort of hack (I assume, as the feature was removed), it would still make sense for the engine to calculate the range modifier and send it to AllowWeaponTargettargetID-1.

~0019827

Kloot (developer)

Last edited: 2019-03-13 02:31

View 2 revisions

> Are you aware that the engine used to acquire such targets correctly?

Make a demo of a Crabe sitting on a terraform spire (while set to hold position) in a version which did, the AllowWeaponTarget CAI change is not supposed to be a regression.

If anything this looks like a retread of 0006076.

~0019828

Google_Frog (reporter)

I think the bug was introduce here: https://github.com/spring/spring/commit/198e315d4d7c736794223d3c8aece278909ac967#diff-2e7bae8df3201d119e7e9009f566e695L1140

Setting wTgt before calling w->AutoTarget() is a misuse of w->AutoTarget(). The purpose of w->AutoTarget() is not just to check that a target can be found, it is to find the target and modify w->GetCurrentTarget() as a side effect. This causes the later check (wTgt.type != Target_Unit || !IsValidTarget(wTgt.unit, w)) to use the wrong wTgt, and often fail.

The pertinent feature of w->AutoTarget() is that it searches for targets using the height based ranged modifier of the weapon itself. Here is what I mean: https://github.com/spring/spring/pull/435.

This change does not remove Luas control here, since Lua can modify the order and makeup of the GenerateWeaponTargets within AutoTarget. Also, Lua is given veto in AllowWeaponTarget a few lines later.

~0019834

Kloot (developer)

wTgt is an object reference (as returned by GetCurrentTarget()), so setting it before or after the call to w->AutoTarget() should make no difference at all. Did you test this?

~0019835

Kloot (developer)

Last edited: 2019-03-13 13:04

View 2 revisions

FYI I applied your patch locally and saw zero effect when playing back 20190312_230716_TitanDuel 2_104.0.1-1060-g7f3541f. If it worked the demo should have desynced.

~0019836

Google_Frog (reporter)

No, it is just my guess.
+Notes

-Issue History
Date Modified Username Field Change
2019-03-12 13:20 Google_Frog New Issue
2019-03-12 13:20 Google_Frog File Added: 20190312_230716_TitanDuel 2_104.0.1-1060-g7f3541f maintenance.sdfz
2019-03-12 13:21 Google_Frog File Added: holdPosAquireTarget.jpg
2019-03-12 13:57 Kloot Status new => closed
2019-03-12 13:57 Kloot Resolution open => no change required
2019-03-12 13:57 Kloot Note Added: 0019820
2019-03-13 02:07 Google_Frog Status closed => feedback
2019-03-13 02:07 Google_Frog Resolution no change required => reopened
2019-03-13 02:07 Google_Frog Note Added: 0019826
2019-03-13 02:07 Google_Frog File Added: oldEngineRange.jpg
2019-03-13 02:26 Kloot Note Added: 0019827
2019-03-13 02:31 Kloot Note Edited: 0019827 View Revisions
2019-03-13 02:55 Google_Frog Note Added: 0019828
2019-03-13 02:55 Google_Frog Status feedback => new
2019-03-13 02:58 Google_Frog File Added: 20190313_125620_TitanDuel 2_104.0.1-287-gf7b0fcc maintenance.sdfz
2019-03-13 12:52 Kloot Note Added: 0019834
2019-03-13 13:03 Kloot Note Added: 0019835
2019-03-13 13:04 Kloot Note Edited: 0019835 View Revisions
2019-03-13 13:28 Google_Frog Note Added: 0019836
2019-03-14 01:29 Kloot Assigned To => Kloot
2019-03-14 01:29 Kloot Status new => resolved
2019-03-14 01:29 Kloot Fixed in Version => 104.0 +git
+Issue History