Page MenuHomeWildfire Games

clean up structure templates build restriction categories, sounds etc
ClosedPublic

Authored by Nescio on Fri, Sep 25, 11:11 AM.

Details

Reviewers
Freagarach
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP24104: Clean up structure templates.
Summary

This patch cleans up some template_structure* lines.
Split off from D2855:

  • Defines attack sounds only in those structure templates that define an <Attack>.

Split off from D2993:

  • Defines <BuildingAI> only in those structure templates that define an <Attack>.

Split off from D3000:

  • Removes <Sound> from template_structure_special.xml, since most of its children have different values.
  • Removes <Health/Max> from template_structure_special.xml, since that's already present in most its children.
  • Removes <TerritoryInfluence> from template_structure_special.xml, since that belongs in most of its children too.
  • Purges unused <BuildRestrictions/Category> lines.
Test Plan

Check for completeness and correctness.

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

Nescio created this revision.Fri, Sep 25, 11:11 AM
Owners added a subscriber: Restricted Owners Package.Fri, Sep 25, 11:11 AM
Nescio edited the summary of this revision. (Show Details)Fri, Sep 25, 11:12 AM
Nescio requested review of this revision.Fri, Sep 25, 11:19 AM
Freagarach added inline comments.Fri, Sep 25, 11:31 AM
binaries/data/mods/public/simulation/components/BuildRestrictions.js
9 ↗(On Diff #13534)

Why this?

Nescio added inline comments.Fri, Sep 25, 11:37 AM
binaries/data/mods/public/simulation/components/BuildRestrictions.js
9 ↗(On Diff #13534)

"<Category>Special</Category>" is actually unused, keeping it only for this example is rather unnecessary.
CC was chosen as replacement because of line 11.

Nescio updated this revision to Diff 13537.Fri, Sep 25, 1:05 PM
  • forgot other/ structures (not the first time)
Nescio updated this revision to Diff 13545.Sat, Sep 26, 11:23 AM
Nescio retitled this revision from clean up structure templates sounds etc to clean up structure templates build restriction categories, sounds etc.
Nescio edited the summary of this revision. (Show Details)
  • remove unnecessary campaign_city_* <Attack> lines

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/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
                        ^~~~~~~~~~~~~~~~
                        {               }
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.
1 warning generated.
builderr-release-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.
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libsimulation2.a(precompiled.o) has no symbols
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
                        ^~~~~~~~~~~~~~~~
                        {               }
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.
1 warning generated.

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

Freagarach added inline comments.Wed, Sep 30, 6:45 PM
binaries/data/mods/public/simulation/components/BuildRestrictions.js
9 ↗(On Diff #13534)

Using the same as below kind of implies that they ought to be the same. I'd say just leave this file.

binaries/data/mods/public/simulation/templates/campaigns/campaign_city_minor_test.xml
11 ↗(On Diff #13545)

Although the entity is unused, better not change effectively any templates.

binaries/data/mods/public/simulation/templates/campaigns/campaign_city_test.xml
11 ↗(On Diff #13545)

Idem.

Nescio added inline comments.Fri, Oct 2, 1:10 PM
binaries/data/mods/public/simulation/components/BuildRestrictions.js
9 ↗(On Diff #13534)

It could also be changed to e.g. Structure. Keeping Special suggests it exists and is used elsewhere.

binaries/data/mods/public/simulation/templates/campaigns/campaign_city_minor_test.xml
11 ↗(On Diff #13545)

What do you mean? The entity is used (campaign_test_map.xml), but removing these lines does not change anything: see parent.

Freagarach added inline comments.Fri, Oct 2, 1:24 PM
binaries/data/mods/public/simulation/templates/campaigns/campaign_city_minor_test.xml
11 ↗(On Diff #13545)

If I see it correctly the parent has a minrange of 0?

Nescio added inline comments.Fri, Oct 2, 1:28 PM
binaries/data/mods/public/simulation/templates/campaigns/campaign_city_minor_test.xml
11 ↗(On Diff #13545)

It does, however, setting <MinRange> to 1 has no effect in practice, or even 10 in this case, since it's much smaller than the footprint radius.

Nescio added inline comments.Fri, Oct 2, 1:39 PM
binaries/data/mods/public/simulation/templates/campaigns/campaign_city_minor_test.xml
11 ↗(On Diff #13545)

See also D611/rP19747.

Freagarach added a comment.EditedThu, Oct 8, 6:47 PM
binaries/data/mods/public/simulation/components/BuildRestrictions.js
9 ↗(On Diff #13534)

We have more bogus examples in the codebase? (Reason I nag about this is if this is unneeded we don't need to touch the file at all ^^ This my last one on this -- if you still don't agree we can just make this Structure and be done with it xD)

219–221 ↗(On Diff #13545)

Unrelated and not close to changed code.

binaries/data/mods/public/simulation/templates/campaigns/campaign_city_minor_test.xml
11 ↗(On Diff #13545)

Convinced.

binaries/data/mods/public/simulation/templates/structures/ptol_lighthouse.xml
51 ↗(On Diff #13545)

Isn't a radius of 0 equal to just disabled?
EDIT: Moreover this fails since the node is incomplete.

Nescio updated this revision to Diff 13608.Fri, Oct 9, 10:58 AM
binaries/data/mods/public/simulation/components/BuildRestrictions.js
9 ↗(On Diff #13534)

It's an example and thus shouldn't be misleading.
For comparison, the entries in line 35 were also corrected in D2549/rP23853.

binaries/data/mods/public/simulation/templates/structures/ptol_lighthouse.xml
51 ↗(On Diff #13545)

Nice catch!

Nescio updated this revision to Diff 13612.Fri, Oct 9, 12:03 PM
  • removed unnecessary <TerritoryInfluence> from ptol_lighthouse.xml
Freagarach accepted this revision.Fri, Oct 9, 12:54 PM
  • Units demo loads fine.
  • Changes are complete.
  • Changes are nice (cleans up nicely; we can add categories when needed).

Reverts: rP8854, partly rP7547, refs. rP8836.

binaries/data/mods/public/simulation/components/BuildRestrictions.js
9 ↗(On Diff #13534)

Different, but okay.

This revision is now accepted and ready to land.Fri, Oct 9, 12:54 PM
This revision was automatically updated to reflect the committed changes.