View Issue Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||||
ID | Project | Category | View Status | Date Submitted | Last Update | ||||
---|---|---|---|---|---|---|---|---|---|
0006158 | Spring engine | General | public | 2019-03-12 13:20 | 2019-03-14 01:29 | ||||
Reporter | Google_Frog | ||||||||
Assigned To | Kloot | ||||||||
Priority | normal | Severity | minor | Reproducibility | always | ||||
Status | resolved | Resolution | reopened | ||||||
Product Version | 104.0 +git | ||||||||
Target Version | Fixed in Version | 104.0 +git | |||||||
Summary | 0006158: 104.0.1-1060-g7f3541f maintenance Elevated hold position ballistic trajectory weapon target aquisition failure | ||||||||
Description | Units 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 | ||||||||
Tags | No tags attached. | ||||||||
Checked infolog.txt for Errors | Yes | ||||||||
Attached Files |
|
![]() |
|
Kloot (developer) 2019-03-12 13:57 |
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. |
Google_Frog (reporter) 2019-03-13 02:07 |
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. |
Kloot (developer) 2019-03-13 02:26 Last edited: 2019-03-13 02:31 |
> 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. |
Google_Frog (reporter) 2019-03-13 02:55 |
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. |
Kloot (developer) 2019-03-13 12:52 |
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? |
Kloot (developer) 2019-03-13 13:03 Last edited: 2019-03-13 13:04 |
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. |
Google_Frog (reporter) 2019-03-13 13:28 |
No, it is just my guess. |
![]() |
|||
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 |