Page MenuHomeWildfire Games

create template_defensive_palisade.xml
ClosedPublic

Authored by Nescio on Mar 18 2019, 10:58 AM.

Details

Summary

Currently the various other/palisades_* files inherit from various wall templates. This patch introduces a new template_defensive_palisade.xml to remove duplication in the palisade files.

Minor tweaks:

  • standardized palisade armour
  • default capture points to 500 (same as tower), rather than city walls' 1200
  • removed stone costs and stone loot
  • added minor wood costs and default 10 wood loot
  • village phase requirement
Test Plan

Check if nothing is overlooked and everything still works.

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.Mar 18 2019, 10:58 AM
Vulcan added a subscriber: Vulcan.Mar 18 2019, 11:37 AM

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

Link to build: https://jenkins.wildfiregames.com/job/differential/1118/display/redirect

fatherbushido added a subscriber: fatherbushido.EditedMar 18 2019, 7:19 PM

Why do you think that tree is better? (I didn't remember how it is done for the roman walls). It's a true question ;-)

You can include that in a thought about those palisade templates. Are they structure or gaia things (and so on...)? I always found that weird.

Why do I think this proposal is better than the current situation? First of all, there is a lot of duplication in those palisade files, which could be reduced by introducing a shared palisade template.
Secondly, there are some fundamental differences, e.g. you can garrison units in or on other walls, but not palisades.
Yes, the Roman siege walls currently have the different *_wall_* templates as their respective parents, but they are rather similar to the city walls and have only the ordinary five (turret, short, medium, long, gate), whereas there are fourteen palisade files, most of which inherit from template_structure_defensive_wall.xml.

Are they structure or gaia things (and so on...)?

Personally I consider everything that inherits from template_structure* to be a structure and thus belongs under structures/ (ideally, all except territory_pull.xml ought to be moved from other/ to structures/, but that's a different discussion) and everything that inherits from template_gaia* is a gaia object and thus belongs under gaia/. Mutatis mutandis for other templates.

Stan added a subscriber: Stan.Mar 18 2019, 9:26 PM
Stan added inline comments.
binaries/data/mods/public/simulation/templates/template_structure_defensive_palisade.xml
19 ↗(On Diff #7586)

I should go before H

Nescio added inline comments.Mar 18 2019, 9:29 PM
binaries/data/mods/public/simulation/templates/template_structure_defensive_palisade.xml
19 ↗(On Diff #7586)

What do you mean?

Stan added inline comments.Mar 18 2019, 9:31 PM
binaries/data/mods/public/simulation/templates/template_structure_defensive_palisade.xml
19 ↗(On Diff #7586)

Tags are in alphabetical order :)

Nescio added inline comments.Mar 18 2019, 9:31 PM
binaries/data/mods/public/simulation/templates/template_structure_defensive_palisade.xml
19 ↗(On Diff #7586)

Yes, I know; f, g, h, i, j, k, etc. So what's wrong?

Stan added inline comments.Mar 18 2019, 9:32 PM
binaries/data/mods/public/simulation/templates/template_structure_defensive_palisade.xml
19 ↗(On Diff #7586)

Lol nevermind I'm tired

In D1796#73219, @Nescio wrote:

Why do I think this proposal is better than the current situation? First of all, there is a lot of duplication in those palisade files, which could be reduced by introducing a shared palisade template.
Secondly, there are some fundamental differences, e.g. you can garrison units in or on other walls, but not palisades.
Yes, the Roman siege walls currently have the different *_wall_* templates as their respective parents, but they are rather similar to the city walls and have only the ordinary five (turret, short, medium, long, gate), whereas there are fourteen palisade files, most of which inherit from template_structure_defensive_wall.xml.

Are they structure or gaia things (and so on...)?

Personally I consider everything that inherits from template_structure* to be a structure and thus belongs under structures/ (ideally, all except territory_pull.xml ought to be moved from other/ to structures/, but that's a different discussion) and everything that inherits from template_gaia* is a gaia object and thus belongs under gaia/. Mutatis mutandis for other templates.

Thx for your answers (again those were actual questions!)
I am still thinking the thing has to be thought a bit more :-)

I am still thinking the thing has to be thought a bit more :-)

Well, I'm open for suggestions and criticisms.
A new template_structure_defensive_siegewall.xml might make sense too, I'm not sure.

fatherbushido added a comment.EditedMar 18 2019, 10:03 PM

tomorrow :)

fatherbushido added a comment.EditedMar 19 2019, 5:22 PM

Those palisades_foo models and templates are not really well defined in my mind.

  • are there world elements or buildable?
  • are there eyecandy or functionnal?

I felt there was a bit like "gaia" things (perhaps that list https://trac.wildfiregames.com/wiki/List%3A_Entities%3A_Nature%3A_Special)
pureon and plumo worked on/created them around 2011. I don't know what they had in mind (but they did nice art work!).
Having a clear vision of that is needed imo (see maps placing that, garrisonable issues, capturable issues, special map issues...).

Among those templates there was the wooden tower. It was changed in little towers (but the wooden one was kept with the same parent template?). That one palisades_rocks_fort. Should it inherit from your new parent template? Uhm probably no as it's not a wall or yes if we think it as a palisade element or if we look at the names. But then what to do with palisades_rocks_watchtower. I don't know.

Also there is that roman siege wall we discussed before.

And mauryans walls?

And small carthagenian walls?

[...]

Sorry for providing more questions than answers!

Short answer: not all structures have to buildable in any game. It's perfectly fine if there are some non-buildable structures available for specific maps.

In D1796#73296, @Nescio wrote:

Short answer: not all structures have to buildable in any game. It's perfectly fine if there are some non-buildable structures available for specific maps.

I can't disagree with that! I emphasized that many times!

You're correct in stating that those palisade files are rather messy. I have no solution for it, they're all being used in different maps. All this patch does is merely reduce the amount of duplication in those palisade files.
Anyway, those fourteen palisade files could be subdivided into:

  • five buildable (front row): tower, short, medium, long, gate
  • three extra: curve, end, straight
  • three spikes: angle, small, tall
  • three towers (back row): fort, outpost, watchtower

And mauryans walls?

Those are simply city walls, inheriting from the generic wall templates, as they should.

And small carthagenian walls?

Those cart_s_wall* files currently have the palisades as their parent; I guess because of their similar size; could be changed later, I suppose.

bb requested changes to this revision.Apr 6 2019, 11:13 PM

Checked the difference between the templates as they are loading in game (by printing them out), it resulted in a number of actual changes, giving them below, some of them are not a problem (or actually make sense), a few need to be addressed.

binaries/data/mods/public/simulation/templates/other/palisades_angle_spike.xml
3–5 ↗(On Diff #7586)

As no parent has a resource cost, this structure will be free to build, sure no unit can build it, but it still shouldn't be free in case someone ever assigns a unit that needs to build this. The original cost was 20stone, but I guess a wood cost makes more sense

Same goes for rocks_curve, rocks_end, rocks_outpost, rocks_straight, watchtower, small_spikes, tall_spikes

14–17 ↗(On Diff #7586)

pushing the required phase down to village phase, makes sense to me, but it needs to be mentioned that we change it same for some others

binaries/data/mods/public/simulation/templates/other/palisades_rocks_end.xml
13 ↗(On Diff #7586)

loot changes from 15 stone to 10 wood (makes sense if the cost would be in wood, so see that comment too)

same for rocks_outpost, rocks_straight

binaries/data/mods/public/simulation/templates/other/palisades_rocks_gate.xml
4–7 ↗(On Diff #7586)

Values changed from 45s and 28stone (which totaly makes sense)

17 ↗(On Diff #7586)

changed from 637.5 to 640 meh

23 ↗(On Diff #7586)

Good, corrected

32 ↗(On Diff #7586)

repairTimeRatio changed from 3.8 to 4.5

binaries/data/mods/public/simulation/templates/other/palisades_rocks_long.xml
2 ↗(On Diff #7586)

Useless garrison aura removed, k

48–49 ↗(On Diff #7586)

Readd the tooltip: "This will allow you to let units circulate through your fortifications."

binaries/data/mods/public/simulation/templates/other/palisades_rocks_tower.xml
26 ↗(On Diff #7586)

It is still a Tower right, so keep that class

binaries/data/mods/public/simulation/templates/template_structure_defensive_palisade.xml
4–6 ↗(On Diff #7586)

Some of the palisades actually had somewhat different values (they inherit the ones from the wall), changing them so that all palisades have the same values is in principle to a problem, but we should be clear about it.

This revision now requires changes to proceed.Apr 6 2019, 11:13 PM
Nescio edited the summary of this revision. (Show Details)Apr 7 2019, 11:05 AM

Maybe build time, wood cost, and loot ought to be made proportional to health, e.g. health/50?

Perhaps outpost and watchtower should inherit from template_structure_defensive_tower_outpost.xml instead?


From left to right: palisade outpost, palisade watchtower, default (athen/hellenes) outpost.

binaries/data/mods/public/simulation/templates/other/palisades_rocks_tower.xml
26 ↗(On Diff #7586)

It doesn't fire any arrows, can't garrison any units, nor does it inherit from any tower template, therefore I deemed it appropiate to remove the Tower class.

bb added a comment.Apr 12 2019, 3:29 PM
In D1796#74399, @Nescio wrote:

Maybe build time, wood cost, and loot ought to be made proportional to health, e.g. health/50?

Whatever makes sense compared to the other palisade structures. e.g cost wise a rocks_curve is pretty similar to a rocks_long, right?

Perhaps outpost and watchtower should inherit from template_structure_defensive_tower_outpost.xml instead?

That would imply letting those two buildings shoot arrows, which contradicts the fact that palisades are not meant to do that, so I would vote against.

Nescio added inline comments.Apr 12 2019, 9:37 PM
binaries/data/mods/public/simulation/templates/other/palisades_rocks_gate.xml
4–7 ↗(On Diff #7586)

upgrade long palisade to gate cost

32 ↗(On Diff #7586)

Does this matter?

Nescio updated this revision to Diff 7738.Apr 12 2019, 9:38 PM
Nescio edited the summary of this revision. (Show Details)

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

Link to build: https://jenkins.wildfiregames.com/job/differential/1206/display/redirect

Angen added a subscriber: Angen.Apr 12 2019, 9:41 PM
Angen added inline comments.
binaries/data/mods/public/simulation/templates/other/palisades_rocks_gate.xml
32 ↗(On Diff #7586)

yes, D1390

Nescio added inline comments.Apr 12 2019, 9:46 PM
binaries/data/mods/public/simulation/templates/other/palisades_rocks_gate.xml
32 ↗(On Diff #7586)

Lower health, yes, but that's the case: 640 < 750.
However, all the other palisades have the same repairTimeRatio, even though their health values range from 200 to 2000. So why should the gate be the only palisade with a different ratio?

Angen added inline comments.Apr 12 2019, 9:50 PM
binaries/data/mods/public/simulation/templates/other/palisades_rocks_gate.xml
32 ↗(On Diff #7586)

if you are changing health, you should change repairtimeratio the same way to keep consistance

Angen added a subscriber: temple.Apr 12 2019, 9:55 PM
Angen added inline comments.
binaries/data/mods/public/simulation/templates/other/palisades_rocks_gate.xml
32 ↗(On Diff #7586)

There is comment by @temple:
They had been repairing each section at the same hp/s rate since health and build times were nice multiples, 1000, 2000, 3000 and 15, 30, 45. Now changing 1000 to 3000 we need to do 3x and changing 2000 to 3000 we need to do 1.5x. The repairers will then do the same hp/s repair rate on each section even though they'll initially build the shorter sections quicker.

Context not related but you got the idea

Nescio added inline comments.Apr 12 2019, 9:57 PM
binaries/data/mods/public/simulation/templates/other/palisades_rocks_gate.xml
32 ↗(On Diff #7586)

But why does the repairTimeRatio have to be changed? Isn't tweaking the build time (cost) more appropiate?

Nescio edited the summary of this revision. (Show Details)Apr 12 2019, 10:35 PM
bb accepted this revision.Apr 21 2019, 4:51 PM

I noticed that some structures have smaller cost than loot, which looks a bit weird. Leaving that as is for now, this patch doesn't change this.

This revision is now accepted and ready to land.Apr 21 2019, 4:51 PM
This revision was automatically updated to reflect the committed changes.

Thank you! Are you also interested in reviewing D1794 and D1790?