Page MenuHomeWildfire Games
Differential D343

TowersOverlapping entries added for cart_wall_stone.xml
ClosedPublic
Actions

Authored by Grugnas on Apr 16 2017, 5:02 PM.

Details

Reviewers
FeXoR
fatherbushido
Commits
rP19668: Fixes an error while placing Cartaginian walls ingame.
Summary

Carthaginian Stone Wall now has its own value.

Test Plan

E.g:
Click on the place where you want to start a wall then move the cursor around.
Whenever you move the cursor back to the starting point of the wall, you'll get an error.

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

Grugnas created this revision.Apr 16 2017, 5:02 PM
Owners added a subscriber: Restricted Owners Package.Apr 16 2017, 5:02 PM
fatherbushido edited edge metadata.Apr 16 2017, 5:18 PM

Sounds good and fix that error with carthagenian walls. So or we modify it for carth or we do like in this diff.

ERROR: No placement possible for distance=7.567, minOverlap=0.05, maxOverlap=0.85

fatherbushido added a reviewer: FeXoR.Apr 16 2017, 5:18 PM
Vulcan added a subscriber: Vulcan.Apr 16 2017, 5:51 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/789/ for more details.

Harbormaster completed remote builds in B1144: Diff 1287.Apr 16 2017, 5:51 PM
elexis added a subscriber: elexis.Apr 16 2017, 7:37 PM

Why are the removed differences irrelevant?

Some walls didn't correctly inherit TowersOverlapping entries from their template

Can you give an example?

Grugnas added a comment.Apr 16 2017, 7:42 PM

Carthage wall Overlapping was inherited from its parent wallset having a value of <MaxTowerOverlap>0.85</MaxTowerOverlap> while carthaginian wall needs a 0.90 value.

elexis added a comment.Apr 16 2017, 11:57 PM
In D343#13438, @Grugnas wrote:

Carthage wall Overlapping was inherited from its parent wallset having a value of <MaxTowerOverlap>0.85</MaxTowerOverlap> while carthaginian wall needs a 0.90 value.

I believe you, but what about all the others changed?

binaries/data/mods/public/simulation/templates/structures/brit_wallset_stone.xml
16 ↗(On Diff #1287)

this

binaries/data/mods/public/simulation/templates/structures/gaul_wallset_stone.xml
16 ↗(On Diff #1287)

and this

binaries/data/mods/public/simulation/templates/structures/iber_wallset_stone.xml
16 ↗(On Diff #1287)

and this

binaries/data/mods/public/simulation/templates/structures/rome_wallset_siege.xml
21 ↗(On Diff #1287)

Why is this difference irrelevant?

binaries/data/mods/public/simulation/templates/structures/rome_wallset_stone.xml
17 ↗(On Diff #1287)

and this?

fatherbushido resigned from this revision.Apr 23 2017, 6:04 PM
Grugnas updated this revision to Diff 1472.Apr 24 2017, 1:50 PM

as elexis pointed out, generalizing the Overlapping value isn't very mod-friendly nor well fit the actors.
Added the overlapping entries to cart_wall_stone.xml and deleted the non used inherited entry from the parent.

Grugnas retitled this revision from TowersOverlapping is now inherited from the wallset template to TowersOverlapping is now removed from the wallset template; overlapping added for cart_wall_stone.xml.Apr 24 2017, 1:53 PM
Vulcan added a comment.Apr 24 2017, 2:35 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/885/ for more details.

Harbormaster completed remote builds in B1275: Diff 1472.Apr 24 2017, 2:35 PM
FeXoR edited reviewers, added: • leper; removed: FeXoR.May 1 2017, 3:11 PM
FeXoR removed a reviewer: • leper.
FeXoR added a reviewer: FeXoR.May 27 2017, 10:17 AM
Grugnas edited the test plan for this revision. (Show Details)May 27 2017, 11:23 AM
FeXoR requested changes to this revision.May 27 2017, 12:11 PM

Applying the patch causes multiple errors:

ERROR: RelaxNGValidator: Validation error: other/wallset_palisade:1: Expecting an element MinTowerOverlap, got nothing
ERROR: RelaxNGValidator: Validation error: other/wallset_palisade:1: Invalid sequence in interleave
ERROR: RelaxNGValidator: Validation error: other/wallset_palisade:1: Element WallSet failed to validate content
ERROR: RelaxNGValidator: Validation failed for '(null)'
ERROR: Failed to validate entity template 'other/wallset_palisade'
This revision now requires changes to proceed.May 27 2017, 12:11 PM
Grugnas updated this revision to Diff 2235.May 27 2017, 12:21 PM
Grugnas edited edge metadata.

reverted the template_structure_defense_wallset.xml because of the palizades inheritance.

Grugnas retitled this revision from TowersOverlapping is now removed from the wallset template; overlapping added for cart_wall_stone.xml to TowersOverlapping entries added for cart_wall_stone.xml.May 27 2017, 12:23 PM
Grugnas edited the summary of this revision. (Show Details)
Grugnas edited the test plan for this revision. (Show Details)
Closed by commit rP19668: Fixes an error while placing Cartaginian walls ingame. (authored by FeXoR). · Explain WhyMay 27 2017, 12:59 PM
This revision was automatically updated to reflect the committed changes.
Vulcan added a comment.May 27 2017, 3:25 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1373/ for more details.

Harbormaster completed remote builds in B1990: Diff 2235.May 27 2017, 3:26 PM

Revision Contents
Changeset List

PathSize
ps/
trunk/
binaries/
data/
mods/
public/
simulation/
templates/
structures/
2 lines

Diff 2236

View Options

ps/trunk/binaries/data/mods/public/simulation/templates/structures/cart_wallset_stone.xml

Loading...
Wildfire Games · Phabricator