The gaia attackers look for the closest organic unit to attack. Sometimes, that unit happens to be on the other side. And the gaia congregates on the bank not attacking anything.
Details
- Reviewers
elexis - Commits
- rP21767: Fix gaia attackers on the left riverside of Danubius trying to attack units on…
- Trac Tickets
- #4622
Gather some units on the bank in one side, and move all units on the other bank to the very edge of the map. Without the patch, the attackers spawned on the side with units moved away would target the units on the opposite bank. With the patch, the attackers would patrol their side instead of gathering on the bank.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 5850 Build 9790: Vulcan Build Jenkins Build 9789: arc lint + arc unit
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 14| 14| | 15| 15| const danubiusAttackerTemplates = deepfreeze({ | 16| 16| "ships": TriggerHelper.GetTemplateNamesByClasses("Warship", "gaul", undefined, undefined, true), | 17| |- "siege": TriggerHelper.GetTemplateNamesByClasses("Siege","gaul", undefined, undefined, true), | | 17|+ "siege": TriggerHelper.GetTemplateNamesByClasses("Siege", "gaul", undefined, undefined, true), | 18| 18| "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen","gaul", undefined, undefined, true), | 19| 19| "healers": TriggerHelper.GetTemplateNamesByClasses("Healer","gaul", undefined, undefined, true), | 20| 20| "champions": TriggerHelper.GetTemplateNamesByClasses("Champion", "gaul", undefined, undefined, true), | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 15| 15| const danubiusAttackerTemplates = deepfreeze({ | 16| 16| "ships": TriggerHelper.GetTemplateNamesByClasses("Warship", "gaul", undefined, undefined, true), | 17| 17| "siege": TriggerHelper.GetTemplateNamesByClasses("Siege","gaul", undefined, undefined, true), | 18| |- "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen","gaul", undefined, undefined, true), | | 18|+ "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen", "gaul", undefined, undefined, true), | 19| 19| "healers": TriggerHelper.GetTemplateNamesByClasses("Healer","gaul", undefined, undefined, true), | 20| 20| "champions": TriggerHelper.GetTemplateNamesByClasses("Champion", "gaul", undefined, undefined, true), | 21| 21| "champion_infantry": TriggerHelper.GetTemplateNamesByClasses("Champion+Infantry", "gaul", undefined, undefined, true), | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 16| 16| "ships": TriggerHelper.GetTemplateNamesByClasses("Warship", "gaul", undefined, undefined, true), | 17| 17| "siege": TriggerHelper.GetTemplateNamesByClasses("Siege","gaul", undefined, undefined, true), | 18| 18| "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen","gaul", undefined, undefined, true), | 19| |- "healers": TriggerHelper.GetTemplateNamesByClasses("Healer","gaul", undefined, undefined, true), | | 19|+ "healers": TriggerHelper.GetTemplateNamesByClasses("Healer", "gaul", undefined, undefined, true), | 20| 20| "champions": TriggerHelper.GetTemplateNamesByClasses("Champion", "gaul", undefined, undefined, true), | 21| 21| "champion_infantry": TriggerHelper.GetTemplateNamesByClasses("Champion+Infantry", "gaul", undefined, undefined, true), | 22| 22| "citizen_soldiers": TriggerHelper.GetTemplateNamesByClasses("CitizenSoldier", "gaul", undefined, "Basic", true), | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 290| 290| | 291| 291| let animations = ritualAnimations[ | 292| 292| cmpIdentity.HasClass("Healer") ? "healer" : | 293| |- cmpIdentity.HasClass("Female") ? "female" : "male"]; | | 293|+ cmpIdentity.HasClass("Female") ? "female" : "male"]; | 294| 294| | 295| 295| let cmpVisual = Engine.QueryInterface(ent, IID_Visual); | 296| 296| if (!cmpVisual)
Link to build: https://jenkins.wildfiregames.com/job/differential/381/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 14| 14| | 15| 15| const danubiusAttackerTemplates = deepfreeze({ | 16| 16| "ships": TriggerHelper.GetTemplateNamesByClasses("Warship", "gaul", undefined, undefined, true), | 17| |- "siege": TriggerHelper.GetTemplateNamesByClasses("Siege","gaul", undefined, undefined, true), | | 17|+ "siege": TriggerHelper.GetTemplateNamesByClasses("Siege", "gaul", undefined, undefined, true), | 18| 18| "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen","gaul", undefined, undefined, true), | 19| 19| "healers": TriggerHelper.GetTemplateNamesByClasses("Healer","gaul", undefined, undefined, true), | 20| 20| "champions": TriggerHelper.GetTemplateNamesByClasses("Champion", "gaul", undefined, undefined, true), | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 15| 15| const danubiusAttackerTemplates = deepfreeze({ | 16| 16| "ships": TriggerHelper.GetTemplateNamesByClasses("Warship", "gaul", undefined, undefined, true), | 17| 17| "siege": TriggerHelper.GetTemplateNamesByClasses("Siege","gaul", undefined, undefined, true), | 18| |- "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen","gaul", undefined, undefined, true), | | 18|+ "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen", "gaul", undefined, undefined, true), | 19| 19| "healers": TriggerHelper.GetTemplateNamesByClasses("Healer","gaul", undefined, undefined, true), | 20| 20| "champions": TriggerHelper.GetTemplateNamesByClasses("Champion", "gaul", undefined, undefined, true), | 21| 21| "champion_infantry": TriggerHelper.GetTemplateNamesByClasses("Champion+Infantry", "gaul", undefined, undefined, true), | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 16| 16| "ships": TriggerHelper.GetTemplateNamesByClasses("Warship", "gaul", undefined, undefined, true), | 17| 17| "siege": TriggerHelper.GetTemplateNamesByClasses("Siege","gaul", undefined, undefined, true), | 18| 18| "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen","gaul", undefined, undefined, true), | 19| |- "healers": TriggerHelper.GetTemplateNamesByClasses("Healer","gaul", undefined, undefined, true), | | 19|+ "healers": TriggerHelper.GetTemplateNamesByClasses("Healer", "gaul", undefined, undefined, true), | 20| 20| "champions": TriggerHelper.GetTemplateNamesByClasses("Champion", "gaul", undefined, undefined, true), | 21| 21| "champion_infantry": TriggerHelper.GetTemplateNamesByClasses("Champion+Infantry", "gaul", undefined, undefined, true), | 22| 22| "citizen_soldiers": TriggerHelper.GetTemplateNamesByClasses("CitizenSoldier", "gaul", undefined, "Basic", true), | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 290| 290| | 291| 291| let animations = ritualAnimations[ | 292| 292| cmpIdentity.HasClass("Healer") ? "healer" : | 293| |- cmpIdentity.HasClass("Female") ? "female" : "male"]; | | 293|+ cmpIdentity.HasClass("Female") ? "female" : "male"]; | 294| 294| | 295| 295| let cmpVisual = Engine.QueryInterface(ent, IID_Visual); | 296| 296| if (!cmpVisual)
Link to build: https://jenkins.wildfiregames.com/job/differential/382/display/redirect
Bug:
The filtering would have to be done prior to the slice, otherwise you might get 0 attack targets.
Performance:
You might already be familiar with "Spawn lag" on every gaia attacker map.
This would likely worsen it due to the vector computation twice per attacker per attack target.
In comparison, JebelBarkal_SpawnAttackerGroups only computes all jebelBarkal_pathTargetClasses = "CivCentre Wonder" once per call.
So maybe we can reconsider this and possibly unify the gaia control.
#4622 also mentions that chosing 3 targets is not always good for soldiers, since these targets may have retreated already (luring) and became unreachable.
JB only picks one target then random locations and makes units patrol there.
targetCount = 3 seems reasonable for ships, not bad but not needed for siege engines and counterproductive for soldiers.
So maybe setting it to 1 is ok. That makes crossing the sea easier - or we use three different variables.
If it were 1 for all times and all difficulties, then we only need to find the closest target instead of sorting all targets (which does 2n² vector operations instead of O(n)).
Dunno, mabe the performance isn'T such a big problem in this case, but I'm dubious. We have had like 3-5 different significant spawn performance improvements on polar sea.
I'm afraid that using this with the current method of gaia targeting would lead to a non-negligible performance cost. And considering how laggy a game can get after a while (30 mins or so) even without this patch, I think changing how the gaia targets is also needed along with this.
The lag relevant to this patch is only the spawn-lag. Would be great to have a replay profile graph to see the lagspikes.
I'd likely still take this if the filter is done prior to the sorting and slicing.
binaries/data/mods/public/maps/random/danubius_triggers.js | ||
---|---|---|
390 | This comment is wrong because the function is also used for the units unloaded. |
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 14| 14| | 15| 15| const danubiusAttackerTemplates = deepfreeze({ | 16| 16| "ships": TriggerHelper.GetTemplateNamesByClasses("Warship", "gaul", undefined, undefined, true), | 17| |- "siege": TriggerHelper.GetTemplateNamesByClasses("Siege","gaul", undefined, undefined, true), | | 17|+ "siege": TriggerHelper.GetTemplateNamesByClasses("Siege", "gaul", undefined, undefined, true), | 18| 18| "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen","gaul", undefined, undefined, true), | 19| 19| "healers": TriggerHelper.GetTemplateNamesByClasses("Healer","gaul", undefined, undefined, true), | 20| 20| "champions": TriggerHelper.GetTemplateNamesByClasses("Champion", "gaul", undefined, undefined, true), | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 15| 15| const danubiusAttackerTemplates = deepfreeze({ | 16| 16| "ships": TriggerHelper.GetTemplateNamesByClasses("Warship", "gaul", undefined, undefined, true), | 17| 17| "siege": TriggerHelper.GetTemplateNamesByClasses("Siege","gaul", undefined, undefined, true), | 18| |- "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen","gaul", undefined, undefined, true), | | 18|+ "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen", "gaul", undefined, undefined, true), | 19| 19| "healers": TriggerHelper.GetTemplateNamesByClasses("Healer","gaul", undefined, undefined, true), | 20| 20| "champions": TriggerHelper.GetTemplateNamesByClasses("Champion", "gaul", undefined, undefined, true), | 21| 21| "champion_infantry": TriggerHelper.GetTemplateNamesByClasses("Champion+Infantry", "gaul", undefined, undefined, true), | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 16| 16| "ships": TriggerHelper.GetTemplateNamesByClasses("Warship", "gaul", undefined, undefined, true), | 17| 17| "siege": TriggerHelper.GetTemplateNamesByClasses("Siege","gaul", undefined, undefined, true), | 18| 18| "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen","gaul", undefined, undefined, true), | 19| |- "healers": TriggerHelper.GetTemplateNamesByClasses("Healer","gaul", undefined, undefined, true), | | 19|+ "healers": TriggerHelper.GetTemplateNamesByClasses("Healer", "gaul", undefined, undefined, true), | 20| 20| "champions": TriggerHelper.GetTemplateNamesByClasses("Champion", "gaul", undefined, undefined, true), | 21| 21| "champion_infantry": TriggerHelper.GetTemplateNamesByClasses("Champion+Infantry", "gaul", undefined, undefined, true), | 22| 22| "citizen_soldiers": TriggerHelper.GetTemplateNamesByClasses("CitizenSoldier", "gaul", undefined, "Basic", true), | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 290| 290| | 291| 291| let animations = ritualAnimations[ | 292| 292| cmpIdentity.HasClass("Healer") ? "healer" : | 293| |- cmpIdentity.HasClass("Female") ? "female" : "male"]; | | 293|+ cmpIdentity.HasClass("Female") ? "female" : "male"]; | 294| 294| | 295| 295| let cmpVisual = Engine.QueryInterface(ent, IID_Visual); | 296| 296| if (!cmpVisual)
Link to build: https://jenkins.wildfiregames.com/job/differential/412/display/redirect
Still performs unneeded sorting. The patch might become a performance improvement rather than a worsening this way.
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 14| 14| | 15| 15| const danubiusAttackerTemplates = deepfreeze({ | 16| 16| "ships": TriggerHelper.GetTemplateNamesByClasses("Warship", "gaul", undefined, undefined, true), | 17| |- "siege": TriggerHelper.GetTemplateNamesByClasses("Siege","gaul", undefined, undefined, true), | | 17|+ "siege": TriggerHelper.GetTemplateNamesByClasses("Siege", "gaul", undefined, undefined, true), | 18| 18| "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen","gaul", undefined, undefined, true), | 19| 19| "healers": TriggerHelper.GetTemplateNamesByClasses("Healer","gaul", undefined, undefined, true), | 20| 20| "champions": TriggerHelper.GetTemplateNamesByClasses("Champion", "gaul", undefined, undefined, true), | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 15| 15| const danubiusAttackerTemplates = deepfreeze({ | 16| 16| "ships": TriggerHelper.GetTemplateNamesByClasses("Warship", "gaul", undefined, undefined, true), | 17| 17| "siege": TriggerHelper.GetTemplateNamesByClasses("Siege","gaul", undefined, undefined, true), | 18| |- "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen","gaul", undefined, undefined, true), | | 18|+ "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen", "gaul", undefined, undefined, true), | 19| 19| "healers": TriggerHelper.GetTemplateNamesByClasses("Healer","gaul", undefined, undefined, true), | 20| 20| "champions": TriggerHelper.GetTemplateNamesByClasses("Champion", "gaul", undefined, undefined, true), | 21| 21| "champion_infantry": TriggerHelper.GetTemplateNamesByClasses("Champion+Infantry", "gaul", undefined, undefined, true), | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 16| 16| "ships": TriggerHelper.GetTemplateNamesByClasses("Warship", "gaul", undefined, undefined, true), | 17| 17| "siege": TriggerHelper.GetTemplateNamesByClasses("Siege","gaul", undefined, undefined, true), | 18| 18| "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen","gaul", undefined, undefined, true), | 19| |- "healers": TriggerHelper.GetTemplateNamesByClasses("Healer","gaul", undefined, undefined, true), | | 19|+ "healers": TriggerHelper.GetTemplateNamesByClasses("Healer", "gaul", undefined, undefined, true), | 20| 20| "champions": TriggerHelper.GetTemplateNamesByClasses("Champion", "gaul", undefined, undefined, true), | 21| 21| "champion_infantry": TriggerHelper.GetTemplateNamesByClasses("Champion+Infantry", "gaul", undefined, undefined, true), | 22| 22| "citizen_soldiers": TriggerHelper.GetTemplateNamesByClasses("CitizenSoldier", "gaul", undefined, "Basic", true), | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 290| 290| | 291| 291| let animations = ritualAnimations[ | 292| 292| cmpIdentity.HasClass("Healer") ? "healer" : | 293| |- cmpIdentity.HasClass("Female") ? "female" : "male"]; | | 293|+ cmpIdentity.HasClass("Female") ? "female" : "male"]; | 294| 294| | 295| 295| let cmpVisual = Engine.QueryInterface(ent, IID_Visual); | 296| 296| if (!cmpVisual)
Link to build: https://jenkins.wildfiregames.com/job/differential/415/display/redirect
You asked in the lobby about the purpose of 3 attacker targets.
The use case is only to optimize the effectiveness of the unit.
If there is only one unit focused, then it will take random patrol routes and possibly miss targets for quite a while (contributing to lag and removing difficulty unintentionally).
For soldiers it seems quite unlikely that the closest 3 targets (soldiers) are spread out by far.
For ships it is quite likely that the 3 closest targets (ships) are spread out far. Most often there are only 0-3 clumps of player ships on the water. But once there is one clump of 3 or more ships, the targetCount = 3 becomes useless again.
For siege engines it is not unlikely but probably not sufficiently likely with regards to performance cost).
For soldiers, targetCount = 3 also thas the issue that the target can become blocked by surrounding soldiers, then the gaia units are slaughtered without attacking anything.
This isn't the case for ships being blocked by ships, because the ships (currently) still fire at surrounding ships while trying to chase.
So it seems we have two reasons to remove it and not a sufficient reason to keep it.
So up for nuking sort and only finding the closest unit using a fancy reduce call?
binaries/data/mods/public/maps/random/danubius_triggers.js | ||
---|---|---|
390 | Not units as it's also used for structures: targets | |
415 | We don't want to compute this.IsLeftRiverside(attackers[0]) about 100 times but only once, right? |
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 14| 14| | 15| 15| const danubiusAttackerTemplates = deepfreeze({ | 16| 16| "ships": TriggerHelper.GetTemplateNamesByClasses("Warship", "gaul", undefined, undefined, true), | 17| |- "siege": TriggerHelper.GetTemplateNamesByClasses("Siege","gaul", undefined, undefined, true), | | 17|+ "siege": TriggerHelper.GetTemplateNamesByClasses("Siege", "gaul", undefined, undefined, true), | 18| 18| "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen","gaul", undefined, undefined, true), | 19| 19| "healers": TriggerHelper.GetTemplateNamesByClasses("Healer","gaul", undefined, undefined, true), | 20| 20| "champions": TriggerHelper.GetTemplateNamesByClasses("Champion", "gaul", undefined, undefined, true), | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 15| 15| const danubiusAttackerTemplates = deepfreeze({ | 16| 16| "ships": TriggerHelper.GetTemplateNamesByClasses("Warship", "gaul", undefined, undefined, true), | 17| 17| "siege": TriggerHelper.GetTemplateNamesByClasses("Siege","gaul", undefined, undefined, true), | 18| |- "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen","gaul", undefined, undefined, true), | | 18|+ "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen", "gaul", undefined, undefined, true), | 19| 19| "healers": TriggerHelper.GetTemplateNamesByClasses("Healer","gaul", undefined, undefined, true), | 20| 20| "champions": TriggerHelper.GetTemplateNamesByClasses("Champion", "gaul", undefined, undefined, true), | 21| 21| "champion_infantry": TriggerHelper.GetTemplateNamesByClasses("Champion+Infantry", "gaul", undefined, undefined, true), | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 16| 16| "ships": TriggerHelper.GetTemplateNamesByClasses("Warship", "gaul", undefined, undefined, true), | 17| 17| "siege": TriggerHelper.GetTemplateNamesByClasses("Siege","gaul", undefined, undefined, true), | 18| 18| "females": TriggerHelper.GetTemplateNamesByClasses("FemaleCitizen","gaul", undefined, undefined, true), | 19| |- "healers": TriggerHelper.GetTemplateNamesByClasses("Healer","gaul", undefined, undefined, true), | | 19|+ "healers": TriggerHelper.GetTemplateNamesByClasses("Healer", "gaul", undefined, undefined, true), | 20| 20| "champions": TriggerHelper.GetTemplateNamesByClasses("Champion", "gaul", undefined, undefined, true), | 21| 21| "champion_infantry": TriggerHelper.GetTemplateNamesByClasses("Champion+Infantry", "gaul", undefined, undefined, true), | 22| 22| "citizen_soldiers": TriggerHelper.GetTemplateNamesByClasses("CitizenSoldier", "gaul", undefined, "Basic", true), | | [NORMAL] ESLintBear (indent): | | Expected indentation of 4 tabs but found 3. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 285| 285| | 286| 286| let animations = ritualAnimations[ | 287| 287| cmpIdentity.HasClass("Healer") ? "healer" : | 288| |- cmpIdentity.HasClass("Female") ? "female" : "male"]; | | 288|+ cmpIdentity.HasClass("Female") ? "female" : "male"]; | 289| 289| | 290| 290| let cmpVisual = Engine.QueryInterface(ent, IID_Visual); | 291| 291| if (!cmpVisual) | | [NORMAL] ESLintBear (no-trailing-spaces): | | Trailing spaces not allowed. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 410| 410| let targets = TriggerHelper.MatchEntitiesByClass(TriggerHelper.GetAllPlayersEntities(), targetClass); | 411| 411| let minDistance = Infinity; | 412| 412| let closestTarget; | 413| |- | | 413|+ | 414| 414| for (let target of targets) | 415| 415| { | 416| 416| if (this.IsLeftRiverside(target) != isLeft) | | [NORMAL] ESLintBear (no-trailing-spaces): | | Trailing spaces not allowed. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 415| 415| { | 416| 416| if (this.IsLeftRiverside(target) != isLeft) | 417| 417| continue; | 418| |- | | 418|+ | 419| 419| let targetDistance = DistanceBetweenEntities(attackers[0], target); | 420| 420| | 421| 421| if (targetDistance < minDistance) | | [NORMAL] ESLintBear (no-trailing-spaces): | | Trailing spaces not allowed. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 417| 417| continue; | 418| 418| | 419| 419| let targetDistance = DistanceBetweenEntities(attackers[0], target); | 420| |- | | 420|+ | 421| 421| if (targetDistance < minDistance) | 422| 422| { | 423| 423| closestTarget = target; | | [NORMAL] ESLintBear (semi): | | Missing semicolon. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 421| 421| if (targetDistance < minDistance) | 422| 422| { | 423| 423| closestTarget = target; | 424| |- minDistance = targetDistance | | 424|+ minDistance = targetDistance; | 425| 425| }; | 426| 426| }; | 427| 427| | | [NORMAL] ESLintBear (no-extra-semi): | | Unnecessary semicolon. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 422| 422| { | 423| 423| closestTarget = target; | 424| 424| minDistance = targetDistance | 425| |- }; | | 425|+ } | 426| 426| }; | 427| 427| | 428| 428| this.debugLog(debugName + " " + uneval(attackers) + " attack " + uneval(closestTarget)); | | [NORMAL] ESLintBear (no-extra-semi): | | Unnecessary semicolon. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/danubius_triggers.js | 423| 423| closestTarget = target; | 424| 424| minDistance = targetDistance | 425| 425| }; | 426| |- }; | | 426|+ } | 427| 427| | 428| 428| this.debugLog(debugName + " " + uneval(attackers) + " attack " + uneval(closestTarget)); | 429| 429| binaries/data/mods/public/maps/random/danubius_triggers.js | 424| » » » minDistance·=·targetDistance | | [NORMAL] JSHintBear: | | Missing semicolon. binaries/data/mods/public/maps/random/danubius_triggers.js | 425| » » }; | | [NORMAL] JSHintBear: | | Unnecessary semicolon. binaries/data/mods/public/maps/random/danubius_triggers.js | 426| » }; | | [NORMAL] JSHintBear: | | Unnecessary semicolon.
Link to build: https://jenkins.wildfiregames.com/job/differential/424/display/redirect
The bug is easy to reproduce:
Set shipUngarrisonInterval to 1, start with 2 players, one of them garrisons all units, then the gaia soldiers on the other side of the river cling to the shoreline.
While testing I got
ERROR: Error in timer on entity 1, IID93, function DoAction: TypeError: v1 is undefined Vector2D.sub@globalscripts/vector.js:199:2 Trigger.prototype.IsLeftRiverside@maps/random/danubius_triggers.js:484:35 Trigger.prototype.AttackAndPatrol@maps/random/danubius_triggers.js:416:7 Trigger.prototype.CheckShipRange@maps/random/danubius_triggers.js:573:3 Trigger.prototype.DoAction@simulation/components/Trigger.js:331:3 Timer.prototype.OnUpdate@simulation/components/Timer.js:120:4
This is because MatchEntitiesByClass includes garrisoned units, which don't have a Position2D.
This is handled by DistanceBetweenEntities but not in the new code.
This bug should occur in other places too, GetActiveRiversides for instance.
Since that checks for structures and structures can't be garrisoned in the public mod, we didn't encounter it yet (equally broken in a22).
Good that we found that, fixed in rP21766.
Likely needs things in the other triggerscript maps too (yay maintining all of these).
Thanks for the patch, smiley!
binaries/data/mods/public/maps/random/danubius_triggers.js | ||
---|---|---|
392 | Looks like this function should be moved to TriggerHelper and become used by these two nub maps. | |
425 | no whitespace in empty lines | |
429 | missing semicolon, hopefully reported by the bot above | |
431 | no semicolon here and in the line above |