Page MenuHomeWildfire Games
Differential D3005

Range safe buildingAI
AbandonedPublic
Actions

Authored by bb on Sep 7 2020, 6:18 PM.

Details

Reviewers
Silier
Summary

The buildingAI currently depend on the rangemanager for units to be in range. After D2759 this can be slightly problematic, hence we should check the proper ranges somewhere.

Test Plan

Dislike the fact we query the obstructionManager every round.
Wonder why the rangeManager doesn't use the obstructionManager function already (answer is in D2759)
Dislike the hardcoding of attackType in buidingAI
Dislike the parabolic range code
Dislike querying the positions of all the units once again

Yell more about buildingAI.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
SeverityLocationCodeMessage
Warningbinaries/data/mods/public/simulation/components/BuildingAI.js:142ESLintBear (indent)ESLintBear (indent)
Warningbinaries/data/mods/public/simulation/components/BuildingAI.js:143ESLintBear (indent)ESLintBear (indent)
Warningbinaries/data/mods/public/simulation/components/BuildingAI.js:171ESLintBear (indent)ESLintBear (indent)
Warningbinaries/data/mods/public/simulation/components/BuildingAI.js:172ESLintBear (indent)ESLintBear (indent)
Warningbinaries/data/mods/public/simulation/components/BuildingAI.js:324ESLintBear (consistent-return)ESLintBear (consistent-return)
Unit
No Unit Test Coverage
Build Status
Buildable 13228
Build 26388: Vulcan BuildJenkins
Build 26387: Vulcan Build (macOS)Jenkins
Build 26386: Vulcan Build (Windows)Jenkins
Build 26385: arc lint + arc unit

Event Timeline

bb created this revision.Sep 7 2020, 6:18 PM
Vulcan added a comment.Sep 7 2020, 6:25 PM

Successful build - Chance fights ever on the side of the prudent.

builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui.a(precompiled.o) has no symbols

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1521/display/redirect

Harbormaster completed remote builds in B13138: Diff 13447.Sep 7 2020, 6:32 PM
bb requested review of this revision.Sep 7 2020, 6:32 PM
Freagarach accepted this revision.EditedSep 10 2020, 8:45 PM
Freagarach added a subscriber: Freagarach.

Idea and code is correct, adds ~50-150 microseconds per fire arrows function call.

This revision is now accepted and ready to land.Sep 10 2020, 8:45 PM
wraitii added a subscriber: wraitii.Sep 11 2020, 9:26 AM
wraitii added inline comments.
binaries/data/mods/public/simulation/components/BuildingAI.js
346

You might want to add the filter here to avoid looping twice?

bb updated this revision to Diff 13480.Sep 11 2020, 7:22 PM

Move the check to a loop below. This should help performance since most entities returned by the rangemanager will be in range. Also nukes a loop

Vulcan added a comment.Sep 11 2020, 7:35 PM

Successful build - Chance fights ever on the side of the prudent.

builderr-debug-macos.txt
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:155:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CFixedVector2D(-halfSize.X, -halfSize.Y),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/graphics/tests/test_Camera.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/graphics/tests/test_Camera.h:168:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CVector3D(-101.0f, -101.0f, 101.0f),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/simulation2/tests/test_SerializeTemplates.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/simulation2/tests/test_SerializeTemplates.h:39:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        3, 0, 1, 4, 1, 5
                        ^~~~~~~~~~~~~~~~
                        {               }
1 warning generated.
builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby.a(precompiled.o) has no symbols
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:155:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CFixedVector2D(-halfSize.X, -halfSize.Y),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libsimulation2.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgraphics.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui.a(precompiled.o) has no symbols
In file included from ../../../source/graphics/tests/test_Camera.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/graphics/tests/test_Camera.h:168:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CVector3D(-101.0f, -101.0f, 101.0f),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/simulation2/tests/test_SerializeTemplates.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/simulation2/tests/test_SerializeTemplates.h:39:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        3, 0, 1, 4, 1, 5
                        ^~~~~~~~~~~~~~~~
                        {               }
1 warning generated.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1542/display/redirect

Harbormaster completed remote builds in B13171: Diff 13480.Sep 11 2020, 7:37 PM
Silier requested changes to this revision.Sep 11 2020, 8:40 PM
Silier added a subscriber: Silier.
Silier added inline comments.
binaries/data/mods/public/simulation/components/BuildingAI.js
354

This is wrong.
cmpAttack returns raw range data and ranged attacks use parabolic attack ranges.
So it looks like comment about targets not being in range does not take that fact into account.

This revision now requires changes to proceed.Sep 11 2020, 8:40 PM
Freagarach resigned from this revision.Sep 16 2020, 5:57 PM
Freagarach removed a reviewer: Freagarach.
bb updated this revision to Diff 13528.Sep 23 2020, 10:09 PM
bb edited the test plan for this revision. (Show Details)

Duplicate the parabolic code from unitai

Harbormaster completed remote builds in B13227: Diff 13528.Sep 23 2020, 10:14 PM
Stan added a subscriber: Stan.Sep 23 2020, 10:17 PM
Stan added inline comments.
binaries/data/mods/public/simulation/components/BuildingAI.js
357

I assume we have no way to check whether it's in SquareRange to avoid the squareroot below? Also, already checked for selectedtarget.

bb added inline comments.Sep 23 2020, 10:21 PM
binaries/data/mods/public/simulation/components/BuildingAI.js
357

selectedTarget: fruits of rererewritting code

The way to avoid sqrts would be to learn the squared checks to cpp, but iirc I ran into overflow issues when I tried it ~ 3 years ago

bb updated this revision to Diff 13529.Sep 23 2020, 10:21 PM
Harbormaster completed remote builds in B13228: Diff 13529.Sep 23 2020, 10:26 PM
Silier added a comment.Sep 26 2020, 11:41 AM

One could take look into performQuery function in parabolic range branch and see this checks looks like redundant
https://code.wildfiregames.com/source/0ad/browse/ps/trunk/source/simulation2/components/CCmpRangeManager.cpp$1210

bb abandoned this revision.Dec 8 2020, 7:52 PM
bb mentioned this in D3198: Stop misleading counting arrows in buildingAI,.

Absorbed in rP24217 and D3198.

Revision Contents
Changeset List

PathSize
binaries/
data/
mods/
public/
simulation/
components/
42 lines

Diff 13529

View Options

binaries/data/mods/public/simulation/components/BuildingAI.js

Loading...
Wildfire Games · Phabricator