Page MenuHomeWildfire Games

Check for cmpUnitAI when extracting formations.
ClosedPublic

Authored by Freagarach on Jan 12 2021, 9:13 AM.

Details

Summary

Fixes rP24480

The function had a comment about the entities needing to have UnitAI, but subsequently returned a subset having UnitAI, therefore I chose to just check for UnitAI and continue else.

Test Plan

Select two entities, one with UnitAI and one without. Order them to move. Observe the difference with and without this patch.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Freagarach created this revision.Jan 12 2021, 9:13 AM
Freagarach edited the summary of this revision. (Show Details)Jan 12 2021, 9:15 AM
Freagarach added inline comments.Jan 12 2021, 9:18 AM
binaries/data/mods/public/simulation/helpers/Commands.js
1455 ↗(On Diff #15160)

Notice we could also filter here, but that means another looping.

Build is green

builderr-debug-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine_dbg.a(precompiled.o) has no symbols
../../../source/graphics/ShaderProgram.cpp:86:15: warning: 'Reload' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
        virtual void Reload()
                     ^
../../../source/graphics/ShaderProgram.h:124:15: note: overridden virtual function is here
        virtual void Reload() = 0;
                     ^
../../../source/graphics/ShaderProgram.cpp:121:15: warning: 'Bind' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
        virtual void Bind()
                     ^
../../../source/graphics/ShaderProgram.h:135:15: note: overridden virtual function is here
        virtual void Bind() = 0;
                     ^
../../../source/graphics/ShaderProgram.cpp:131:15: warning: 'Unbind' overrides a membe

See https://jenkins.wildfiregames.com/job/macos-differential/2781/display/redirect for more details.

Freagarach requested review of this revision.Jan 12 2021, 9:24 AM

Yeah non-UnitAI stuff cannot receive orders, so it can be safely ignored here.

Non UnitAI _can_ receive orders ;) But can not have formations, so can safely be ignored there ;)

wraitii accepted this revision.Jan 12 2021, 9:35 AM

Fixes the reported issue.

This revision is now accepted and ready to land.Jan 12 2021, 9:35 AM

Rename members to formations for that is clearer.

Build is green

builderr-debug-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libnetwork_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgraphics_dbg.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui_dbg.a(precompiled.o) has no symbols
ld: warning: text-based stub file /System/Library/Frameworks//CoreAudio.framework/CoreAudio.tbd and library file /Sy

See https://jenkins.wildfiregames.com/job/macos-differential/2784/display/redirect for more details.

Stan added a subscriber: Stan.Jan 12 2021, 10:26 AM
Stan added inline comments.
binaries/data/mods/public/simulation/helpers/Commands.js
898–912 ↗(On Diff #15164)
Freagarach marked an inline comment as done.Jan 12 2021, 11:02 AM

Build is green

builderr-debug-macos.txt
ld: warning: text-based stub file /System/Library/Frameworks//CoreAudio.framework/CoreAudio.tbd and library file /System/Library/Frameworks//CoreAudio.framework/CoreAudio are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//AudioToolbox.framework/AudioToolbox.tbd and library file /System/Library/Frameworks//AudioToolbox.framework/AudioToolbox are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//ForceFeedback.framework/ForceFeedback.tbd and library file /System/Library/Frameworks//ForceFeedback.framework/ForceFeedback are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//CoreVideo.framework/CoreVideo.tbd and library file /System/Library/Frameworks//CoreVideo.framework/CoreVideo are out of sync. Falling back to library file for linking.
ld: warning: text-based stu

See https://jenkins.wildfiregames.com/job/macos-differential/2786/display/redirect for more details.

Stan added inline comments.Jan 12 2021, 11:26 AM
binaries/data/mods/public/simulation/helpers/Commands.js
893 ↗(On Diff #15166)

Have was correct ?

1430 ↗(On Diff #15166)

formation.formations seems a bit weird but subformations isn't better so dunno

Freagarach added inline comments.Jan 12 2021, 11:27 AM
binaries/data/mods/public/simulation/helpers/Commands.js
893 ↗(On Diff #15166)

I'll circumvent.

1430 ↗(On Diff #15166)

Yeah, it is a weird function.

Stan added inline comments.Jan 12 2021, 11:30 AM
binaries/data/mods/public/simulation/helpers/Commands.js
1430 ↗(On Diff #15166)

If we only store ids it could be formationIds?

Silier requested changes to this revision.Jan 12 2021, 11:31 AM
Silier added a subscriber: Silier.
Silier added inline comments.
binaries/data/mods/public/simulation/helpers/Commands.js
894 ↗(On Diff #15166)

this renaming is wrong, it holds formation members not formations

This revision now requires changes to proceed.Jan 12 2021, 11:31 AM
Freagarach marked 2 inline comments as done.Jan 12 2021, 11:32 AM
Freagarach added inline comments.
binaries/data/mods/public/simulation/helpers/Commands.js
894 ↗(On Diff #15166)

It holds formations _and_ their members.

1430 ↗(On Diff #15166)

I thought about refactoring indeed. But not in this diff ;)

Silier added inline comments.Jan 12 2021, 11:35 AM
binaries/data/mods/public/simulation/helpers/Commands.js
894 ↗(On Diff #15166)

formations are keys, members are values, one doesnt name map by keys but by values

Freagarach marked an inline comment as done.Jan 12 2021, 11:37 AM
Freagarach added inline comments.
binaries/data/mods/public/simulation/helpers/Commands.js
894 ↗(On Diff #15166)

Sure, I'll name it back then :)

Freagarach updated this revision to Diff 15172.Jan 12 2021, 1:28 PM
Freagarach marked 3 inline comments as done.

Renamed back.

Build is green

builderr-debug-macos.txt
ld: warning: text-based stub file /System/Library/Frameworks//CoreAudio.framework/CoreAudio.tbd and library file /System/Library/Frameworks//CoreAudio.framework/CoreAudio are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//AudioToolbox.framework/AudioToolbox.tbd and library file /System/Library/Frameworks//AudioToolbox.framework/AudioToolbox are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//ForceFeedback.framework/ForceFeedback.tbd and library file /System/Library/Frameworks//ForceFeedback.framework/ForceFeedback are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//CoreVideo.framework/CoreVideo.tbd and library file /System/Library/Frameworks//CoreVideo.framework/CoreVideo are out of sync. Falling back to library file for linking.
ld: warning: text-based stu

See https://jenkins.wildfiregames.com/job/macos-differential/2792/display/redirect for more details.

Silier resigned from this revision.Jan 12 2021, 3:20 PM
Silier removed a reviewer: Silier.
Freagarach removed a reviewer: Restricted Owners Package.Jan 12 2021, 3:23 PM
This revision is now accepted and ready to land.Jan 12 2021, 3:23 PM
This revision was automatically updated to reflect the committed changes.