Page MenuHomeWildfire Games

Change structure destruction art from foundation to rubble
ClosedPublic

Authored by temple on Jun 15 2017, 7:35 PM.

Details

Summary

There are a few buildings that show a foundation rather than rubble when they're destroyed, which is very confusing. This patch should fix most of the cases:

  • Iberian monument, Briton kennel, Persian hall.
  • Briton, Gaul, Rome corrals. These should use 2x4 rubble, but that doesn't exist, so instead I used 3x3. (Ptolemy uses 3x3 but they should also use 2x4.)
  • Maurya, Ptolemy, Seleucid docks. (Currently Ptolemy and Seleucid use the Hellenic dock rubble, which is wrong.)

Things still to do (by other people):

  • Palisades rubble art. They're the only structures that still show foundations rather than rubble.
  • Make 2x4 rubble art for the four corrals. (There is 4x2 rubble, so that may only need to be rotated.)
  • Make custom dock rubble art for the three civs.
  • Clean up the rubble files: I used "rubble_3x3" instead of "rubble_stone_3x3" just to be consistent with the other corrals, but it's probably better to use "rubble_stone" for everything. Most "rubble" files point to foundation art, which is not what we want, and I don't think most are used anyway? So they could be deleted. But then "rubble_stone" is kind of a repetitive name so maybe it'd be better to use "rubble" everywhere instead.
Test Plan

See that the buildings show rubble when destroyed, rather than a foundation.

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

temple created this revision.Jun 15 2017, 7:35 PM
temple edited the summary of this revision. (Show Details)Jun 15 2017, 8:21 PM
elexis added a subscriber: elexis.Jun 15 2017, 8:29 PM

If there are rubble files that don't contain rubble and all references to that are removed for that reason, then the file should be deleted as it doesn't contain anything useful and misleads people into using it, making it likely for this bug to reappear, right?

Related commits:
17:37 < fatherbushido> rP19131
17:37 < fatherbushido> rP19066
17:37 < fatherbushido> rP19095

temple updated this revision to Diff 2571.Jun 15 2017, 11:24 PM

I changed "rubble_3x3" references to "rubble_stone_3x3", in line with the others.
I deleted the "rubble_2x2.xml", etc. files because they aren't used.
I added a "rubble_stone_2x4.xml" file for the four corrals, so when someone makes 2x4 rubble they only need to change that file rather than all the corrals too. (It's currently using 3x3 rubble as a placeholder.)
I separated the two Celtic civic center files because the rubble only matches the Briton building. I'm using 6x6 rubble as the Gaul placeholder.
I added civic center files for Ptolemy and Seleucid too, using 6x6 rubble as a placeholder.
I added dock files for Maurya, Ptolemy and Seleucid, using 4x4 rubble as a placeholder.

Someone will have to make art for the 2x4 rubble (perhaps just rotate the 4x2 art), the three civic centers, and the three docks (and palisades). But now they should only have to edit where the rubble files point to.

temple updated this revision to Diff 2573.Jun 15 2017, 11:30 PM

By adding an entry to the placeablesFilter.sjon file, we can see the models in the actor viewer of atlas.
All the 1x rubble seems to be foundation-equivalent to me, so might want to check again whether you forgot some other rubble files.

Do we actually need the new rubble tempaltes? Can't we just reuse one template?

temple updated this revision to Diff 2662.Jun 22 2017, 6:05 PM

Yes, there's rubble files for the palisades, and I wasn't sure what to do with them. I guess stone wall rubble works for now. Diff is updated.

New templates, you're talking about the civ-specific buildings? They'll be needed eventually, so as part of the clean-up I thought it'd be good to get them ready. When the art is done that person will only need to replace the "Actor" line rather than create the file itself and change the dock/cc structure file.

I made a trac ticket (#4644) and posted in the art thread.

elexis requested changes to this revision.Jun 28 2017, 5:03 PM

Delete part looks good, would probably be easier to review if that is done separately.
Ok for me to use stone rubble for palisade walls rather than nothing. Maybe this trick could work too for palisade spikes, not sure (you can encounter those on the Danubius map).
Not convinced of the placeholder files, it will become harder to see what's missing and they don't add anything.

binaries/data/mods/public/simulation/templates/rubble/rubble_wall.xml
4 ↗(On Diff #2662)

I couldn't find any template for which this change is visible, because these files change the rubble_wall.xml entry already

template_structure_defense_wall_long.xml
template_structure_defense_wall_medium.xml
template_structure_defense_wall_short.xml
template_structure_defense_wall_tower.xml

Notice the entry is optional.

Since every template that inherits this one does it's own thing anyway, it would be better to not add this one here and let the children pick the one they want.

Of the templates that inherit template_structure_defense_wall that are different from the 4 above:

  • spikes dont show anything
  • palisade_curve shows the wrong thing (large non-curved wall rubble)
  • some have <SpawnEntityOnDeath disable=""/> which could be removed if the parent entry is removed as well

I rather have no foundation shown than a completely wrong one

This revision now requires changes to proceed.Jun 28 2017, 5:03 PM
temple updated this revision to Diff 2743.Jun 29 2017, 12:33 AM
temple edited edge metadata.

I removed the palisade changes and file deletions. I'll make separate patches for them.
I removed the new placeholder templates that I had added. Instead, I deleted the SpawnEntityOnDeath elements for the affected buildings so they'll get them from the generic templates.

elexis accepted this revision.Jun 30 2017, 8:08 PM

Every change looked at so far seems good, excepting the comments below.
Will split this into smaller commits, so that it's easier to see what happens.

binaries/data/mods/public/simulation/templates/structures/athen_corral.xml
8 ↗(On Diff #2743)

If you changed the parent template to use this template, then we don't need this entry anymore. Same for the other corrals. The celtic ones look like they are made from stone, so ok to have all of them add the stone rubble.

binaries/data/mods/public/simulation/templates/structures/gaul_civil_centre.xml
9 ↗(On Diff #2743)

Ack for the civic center thing (which could be done in a separate patch :P), mostly because it seems easy enough to make new rubble from the old building and the gallic one contains stuff from the briton one we can't find in the gallic one, contrary to the hellenic CCs.

rubble_celt_cc should be renamed therefore!

binaries/data/mods/public/simulation/templates/template_structure_economic_farmstead.xml
21 ↗(On Diff #2743)

The celtic ones aren't made out of stone. This also gives a nice precedent to not throw away the rubble_3x3 template.

This revision is now accepted and ready to land.Jun 30 2017, 8:08 PM
This revision was automatically updated to reflect the committed changes.

About the docks, the rubble_stone_3x3 doesn't look to well as the burned rubble is completely submerged in water (sounds wrong, how does it burn if it's in water).
But it's better to not use the foundation actor IMO (even though it somewhat fits for the rubble). I guess someone should do the actual artwork.
Can be tested on cycladic archipelago with the "salad bowl" cheat.

elexis added inline comments.Jun 30 2017, 9:21 PM
binaries/data/mods/public/simulation/templates/template_structure_economic_farmstead.xml
21 ↗(On Diff #2743)

Actually rubble_3x3 doesn't look as well as the stone one, it appears more to be some campfire.
I'll rename it to rubble_wood_3x3 and we'll keep it unused, but ready to use for good.