bugfix: Fix unreliability of historic bonus weapons #1727
Conversation
| HistoricWeaponDamageInfo(UnsignedInt f, const Coord3D& l) : | ||
| frame(f), location(l) | ||
| { | ||
| triggered = false; |
| } | ||
| } | ||
|
|
||
| if (count >= m_historicBonusCount - 1) // minus 1 since we include ourselves implicitly |
|
|
||
| while (it != m_historicDamage.end()) | ||
| { | ||
| UnsignedInt expirationDate = it->frame + m_historicBonusTime; |
| else | ||
| { | ||
| for (HistoricWeaponDamageList::iterator it = m_historicDamage.begin(); it != m_historicDamage.end(); ++it) | ||
| (*it).triggered = false; |
There was a problem hiding this comment.
I think we can get rid of this reset loop by having a
UnsignedInt HistoricWeaponDamageInfo::triggerFrame
which is then set to current frame when triggered, and HistoricWeaponDamageInfo removed when trigger count is confirmed.
Then have 2 functions:
trimExpiredHistoricDamage()
and
trimTriggeredHistoricDamage(triggerFrame)
Make trimExpiredHistoricDamage efficient by expecting chronological order and call it same place as trimOldHistoricDamage.
Call trimTriggeredHistoricDamage after the count test, to remove the triggered ones.
There was a problem hiding this comment.
Thinking about this again, triggerFrame would be not quite right if there is another sequence happening elsewhere in the same frame. The Coord3D would be a better identifier for the sequence. But that is a bit big (12 bytes). So instead I suggest creating a WeaponTemplate local counter that keeps incrementing on each call of processHistoricDamage. That will be unique and safe.
| // This one is close enough in time and distance, so count it. This is tracked by template since it applies | ||
| // across units, so don't try to clear historicDamage on success in here. | ||
| (*it).triggered = true; | ||
| ++count; |
There was a problem hiding this comment.
Is it correct that is just keeps counting even beyond m_historicBonusCount? Perhaps should stop at m_historicBonusCount - 1, to just take the required amount, not more?
2 participants
Fixes #962 from the patch repository
This change fixes historic bonus weapons not triggering reliably. The issue is that whenever a historic bonus weapon is triggered (e.g. a firestorm) it resets the historic bonus count for all weapons using the same template. This means that if a firestorm is triggered while another firestorm is half way to triggering, that other firestorm's progress will be reset.
In addition, historic damage instances now more accurately expire based on the weapon template's
HistoricBonusTimerather than the globalHistoricDamageLimit. Historic damage involved in triggering a historic weapon cannot be used in subsequent attempts.Example
Four MiGs must strike a target within a certain radius to trigger a firestorm. Each missile contributes +1 to the historic bonus count of
NapalmMissileWeapon, which must reach 8 to create the firestorm.Now imagine a scenario where there are two squadrons of four MiGs each, Squad A and Squad B. Both squads strike different targets at the roughly same time.
Squad A MiG 1 increases the historic bonus count to 2.
Squad A MiG 2 increases the historic bonus count to 4.
Squad B MiG 1 increases the historic bonus count to 6.
Squad B MiG 2 increases the historic bonus count to 8.
Squad A MiG 3 increases the historic bonus count to 10.
Squad A MiG 4 increases the historic bonus count to 12.
Of the 12 historic bonus instances, 8 are determined to be in the same location, and thus a firestorm is triggered for Squad A.
The triggering of Squad A's firestorm resets the historic bonus count to 0.
Squad B MiG 3 increases the historic bonus count to 2.
Squad B MiG 4 increases the historic bonus count to 4.
No firestorm is triggered for Squad B.
Demonstration of the bug
BAD_FIRESTORM.mp4
Note: When testing with Inferno Cannons, it is preferable to test without the
HeightDieUpdatemodules in theInfernoTankShellandInfernoTankShellUpgradedweapon objects for 100% accuracy. See #1564 from the patch repository for more info.