| View Issue Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||||
| ID | Project | Category | View Status | Date Submitted | Last Update | ||||
|---|---|---|---|---|---|---|---|---|---|
| 0004641 | Spring engine | General | public | 2015-01-03 03:43 | 2015-01-08 10:14 | ||||
| Reporter | Google_Frog | ||||||||
| Assigned To | cleanrock | ||||||||
| Priority | high | Severity | feature | Reproducibility | N/A | ||||
| Status | closed | Resolution | fixed | ||||||
| Product Version | 98.0.1+git | ||||||||
| Target Version | Fixed in Version | 98.0.1+git | |||||||
| Summary | 0004641: Feature: Configurable weapon aim sanity check | ||||||||
| Description | The number added here needs to be configurable. https://github.com/spring/spring/commit/226dba6089d1471f5bd61e47f5e21771dfa2d0bc 20 degrees is too small for units which have high turnrate and are intended to be good at aiming. Raiders in ZK are affected by this and AimWeapon is called too slowly for it to be taken into account script-side. | ||||||||
| Tags | No tags attached. | ||||||||
| Checked infolog.txt for Errors | |||||||||
| Attached Files |
| ||||||||
 Relationships |
|
 Relationships |
| Notes |
|
|
Google_Frog (reporter) 2015-01-03 03:45 |
I forgot to say that this should be a weapondef thing, not a modrule. |
|
cleanrock (reporter) 2015-01-06 09:39 |
I tested to increase AimWeapon call rate by changing CWeapon::UpdateTargeting: if (gs->frameNum >= (lastRequest+2 /*+ (GAME_SPEED >> 1)*/)) Anything lower than +2 will make units not fire. Currently it will call AimWeapon twice per second (+15) which seem like a very low rate. I did not see noticable much higher LUA CPU usage. GoogleFrog, I am far from sure this is a good fix (let's see what jk says) but perhaps you can test this change. |
|
Google_Frog (reporter) 2015-01-06 12:56 Last edited: 2015-01-06 12:56 |
That sounds dangerous and possibly slow. Rapid fire units would halt a lot of the time while they WaitForTurn. Presumably this is why doing AimWeapon every frame caused them to not fire. I prefer the weapon tag for sanity angle because that would introduce the least change. It should be added regardless of aim timing changes. |
|
silentwings (reporter) 2015-01-07 10:36 |
I agree with both of GFs points. I don't think https://github.com/spring/spring/commit/226dba6089d1471f5bd61e47f5e21771dfa2d0bc is a good idea, I can't imagine a single value working for all units. Kloot seems not even convinced that was a bug in 0003367. |
|
Google_Frog (reporter) 2015-01-07 12:04 |
0003367 refers to the fact that each weapon seems to remember whether AimWeapon returned true and if it did the weapon is able to fire. This can be exploited by a unit which is firing at something (so AimWeapon must have returned true) and changes target before AimWeapon is next called. This can give units half a second of firing backwards which is noticeable if the unit changes target or rotates rapidly at the right time. As far as I can tell cleanrock's change was to add an extra firing check which will stop a weapon from firing if the fire direction is too far off the last direction it was told to aim towards. This stops the unit from firing when it changes target or turns around rapidly. The problem is that 20 degrees is too small for some units. I do not think we need faster AimWeapon. Perhaps the tag I want (and fix that cleanrock implemented) already existed. Does anyone know what tolerance does for turret=true weapons? MaxAngleDifis definitely not relevant to this problem and I think that is what Kloot meant by fire arc. For a turreted weapon maxAngleDif defines the rotation range for the turret. The shooting backwards issue still exists with low maxAngleDif. |
|
Google_Frog (reporter) 2015-01-07 12:22 |
It seems that tolerance is only relevant during aiming. So it is not like cleanrock's change at all. |
|
cleanrock (reporter) 2015-01-07 13:12 |
I have been testing a bit and my +2 hack do not work with BA units (+4 seem ok), perhaps because of the sleep 100 in BAs AimWeapon .. not sure really. After comparing firing behaviour before and after my fix (91 vs develop with different +#) i think we should remove my hack fix due to raider units not firing often enough, this needs a proper fix instead of configurable hack constant. @silentwings, Look at smoths video in 0003367, that is engine and most probably not a ba bug. |
|
Google_Frog (reporter) 2015-01-07 14:03 |
But it works for large noticeable units. I think we should leave it and add configuration. Then people can decide how strictly their units should aim on a per-unit basis. |
|
Google_Frog (reporter) 2015-01-07 14:45 |
PR here https://github.com/spring/spring/pull/152 |
|
silentwings (reporter) 2015-01-07 17:45 Last edited: 2015-01-07 17:56 |
> Look at smoths video in 0003367, that is engine and most probably not a ba bug. As far as I can see, kloots explanation given on 0003367 for the behaviuor in the video is correct, it's essentially intended behaviour in BA. But afaics its also true that the engine does not have the feature of limiting the tolerance for firing (instead of just for aiming) - GFs pull req looks like a good way to add this feature to me. |
|
Google_Frog (reporter) 2015-01-08 03:19 Last edited: 2015-01-08 03:23 |
It is intended behaviour that you can skip the aim animation if you change targets at just the right time? Ok I have changed the default to match the old behaviour. |
|
cleanrock (reporter) 2015-01-08 10:14 |
Merged PR: https://github.com/spring/spring/commit/868b2a0 |
| Notes |
| Issue History |
|||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2015-01-03 03:43 | Google_Frog | New Issue | |
| 2015-01-03 03:45 | Google_Frog | Note Added: 0013883 | |
| 2015-01-06 09:39 | cleanrock | Note Added: 0013888 | |
| 2015-01-06 12:56 | Google_Frog | Note Added: 0013889 | |
| 2015-01-06 12:56 | Google_Frog | Note Edited: 0013889 | View Revisions |
| 2015-01-07 10:36 | silentwings | Note Added: 0013891 | |
| 2015-01-07 12:04 | Google_Frog | Note Added: 0013893 | |
| 2015-01-07 12:22 | Google_Frog | Note Added: 0013894 | |
| 2015-01-07 13:12 | cleanrock | Note Added: 0013895 | |
| 2015-01-07 14:03 | Google_Frog | Note Added: 0013896 | |
| 2015-01-07 14:45 | Google_Frog | Note Added: 0013897 | |
| 2015-01-07 17:45 | silentwings | Note Added: 0013898 | |
| 2015-01-07 17:46 | silentwings | Note Edited: 0013898 | View Revisions |
| 2015-01-07 17:50 | silentwings | Note Edited: 0013898 | View Revisions |
| 2015-01-07 17:56 | silentwings | Note Edited: 0013898 | View Revisions |
| 2015-01-08 03:19 | Google_Frog | Note Added: 0013899 | |
| 2015-01-08 03:23 | Google_Frog | Note Edited: 0013899 | View Revisions |
| 2015-01-08 10:14 | cleanrock | Note Added: 0013900 | |
| 2015-01-08 10:14 | cleanrock | Status | new => closed |
| 2015-01-08 10:14 | cleanrock | Assigned To | => cleanrock |
| 2015-01-08 10:14 | cleanrock | Resolution | open => fixed |
| 2015-01-08 10:14 | cleanrock | Fixed in Version | => 98.0.1+git |
| Issue History |