Page MenuHomeWildfire Games

[art/actors] move siege tower and ram actors from structures to units
Needs ReviewPublic

Authored by Nescio on Mar 26 2020, 4:03 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

Currently artillery actors and Roman rams are located under art/actors/units/*/, but siege towers and other rams are located under art/actors/structures/*/, which is not only inconsistent, but also quite confusing.
This patch:

  • Moves the siege tower and ram actors to the units folder and updates all affected templates accordingly.
  • Introduces a separate actor for the Kushite siege tower (the only difference is the garrison flag).
  • Deletes the art/actors/structures/thebans/ folder, as it contains just one unused file, which is a duplicate of fireraiser already present in the units folder.
Test Plan

Check for mistakes, verify everything still works, agree this is an improvement.

Unit TestsFailed

TimeTest
0 msJenkins > cxxtest_debug.xml::[failed-to-read]
Failed to read test report file E:\Jenkins\workspace\vs2015-differential\cxxtest_debug.xml org.dom4j.DocumentException: Error on line 347 of document : Content is not allowed in trailing section. at org.dom4j.io.SAXReader.read(SAXReader.java:511)
0 msJenkins > TestAllocators::Debug Build & Tests / test_da
0 msJenkins > TestAllocators::Release Build & Tests / test_da
0 msJenkins > TestAllocators::test_da
0 msJenkins > TestAllocators::test_da
View Full Test Results (1 Failed · 1,697 Passed)

Event Timeline

Nescio created this revision.Mar 26 2020, 4:03 PM
Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Mar 26 2020, 4:03 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1915/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/1391/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

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

Stan added a comment.Mar 26 2020, 5:35 PM

Technically isn't a siege tower a "structure"?

Technically isn't a siege tower a "structure"?

It is an object constructed from several parts, so yes, that means it qualifies as structure, and it has a roof and walls, so it qualifies as a building too.
However, in 0 A.D. all siege entities are considered to be units (inherit from template_unit.xml, have the “Unit” class, and can move), not structures (inherit from template_structures.xml, have the “Structure” class, and can't move).
Moreover, having most siege actors in the units folder (e.g. Roman ram) but some in the structures folder (e.g. Persian ram) is both inconsistent and confusing.

Nescio retitled this revision from move siege tower and ram actors from structures to units to [art/actors] move siege tower and ram actors from structures to units.May 18 2020, 10:17 AM
Stan added a comment.Aug 5 2020, 7:45 PM

What about ships? Are they structures, or units?

Well, they do have UnitAI ;)

Nescio added a comment.Aug 5 2020, 7:54 PM

Their templates inherit from template_unit_ship.xml, therefore they are units. Also, they can move around, unlike structures.
(Moving ship actors probably deserves a separate patch, though, if that's what you meant.)

Nescio updated this revision to Diff 14071.Nov 19 2020, 3:28 PM
Nescio edited the summary of this revision. (Show Details)
  • rebased

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

builderr-debug-macos.txt
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:153:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CFixedVector2D(-halfSize.X, -halfSize.Y),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
../../../source/third_party/fmt/format.cpp:145:7: warning: '_POSIX_C_SOURCE' is not defined, evaluates to 0 [-Wundef]
#if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE) || (defined(__ANDROID__) && __ANDROID__)
      ^
../../../source/third_party/fmt/format.cpp:145:37: warning: '_XOPEN_SOURCE' is not defined, evaluates to 0 [-Wundef]
#if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE) || (defined(__ANDROID__) && __ANDROID__)
                                    ^
2 warnings 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/libnetwork.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libtinygettext.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libtinygettext.a(tinygettext.o) has no symbols
/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/libglooxwrapper.a(precompiled.o) has no symbols
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:153: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/libscriptinterface.a(precompiled.o) has no symbols
../../../source/third_party/fmt/format.cpp:145:7: warning: '_POSIX_C_SOURCE' is not defined, evaluates to 0 [-Wundef]
#if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE) || (defined(__ANDROID__) && __ANDROID__)
      ^
../../../source/third_party/fmt/format.cpp:145:37: warning: '_XOPEN_SOURCE' is not defined, evaluates to 0 [-Wundef]
#if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE) || (defined(__ANDROID__) && __ANDROID__)
                                    ^
2 warnings generated.
/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
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblowlevel.a(dbghelp.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblowlevel.a(file_stats.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblowlevel.a(vfs_path.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/2016/display/redirect

I'd actually say to keep the template part separate from the actor part. So siege and stuff as structures.

The majority of siege actors is already under units, though.