Page MenuHomeWildfire Games

Template organization: embassy
ClosedPublic

Authored by Nescio on Nov 3 2017, 4:24 PM.

Details

Summary

Changes the parent of the “_military_embassy” template from “special” to “military”, as was already suggested by the name.

Test Plan

Test if everything works as expected.

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.Nov 3 2017, 4:24 PM
Owners added a subscriber: Restricted Owners Package.Nov 3 2017, 4:24 PM

You point out something, is the special template still needed? (before there was something like every civs had sb1 and sb2).

Nescio added a comment.Nov 3 2017, 4:30 PM

You point out something, is the special template still needed? (before there was something like every civs had sb1 and sb2).

Personally I don't think it's really needed, but that probably should be decided on a case by case basis per template.

Nescio added a comment.Nov 3 2017, 4:48 PM

Furthermore, the Athenian gymnasion, Gaul tavern, Persian hall, and Spartan syssition are fundamentally one and the same structure: a structure which trains some of a faction's champions; so perhaps they could be merged and have a template_structure_military_hall.xml as their shared parent instead?

Moreover, iber_monument and maur_pillar_ashoka are almost identical, so they could be merged as well.

It's also noteworthy that the wonder has a separate generic template, independent of special.

In D1006#39489, @Nescio wrote:

Personally I don't think it's really needed, but that probably should be decided on a case by case basis per template.

Many special things are not really under the special template now (here the embassy, the stoas, ...)
(Need also to think of the AI.)
For example, in that patch you remove SpecialBuilding class. Is it something used by the Petra?

In D1006#39493, @Nescio wrote:

Furthermore, the Athenian gymnasion, Gaul tavern, Persian hall, and Spartan syssition are fundamentally one and the same structure: a structure which trains some of a faction's champions; so perhaps they could be merged and have a template_structure_military_hall.xml as their shared parent instead?

Following you current patch, they should just inherit from barrack (with their specific cost and perhaps specific production queue).
If we think they are really the same generic type, then a barrack_foo is ok too.

Moreover, iber_monument and maur_pillar_ashoka are almost identical, so they could be merged as well.

yes (or just inherit directly from structure)

It's also noteworthy that the wonder has a separate generic template, independent of special.

yes same for some other special things (military colony and hellenic royal (*) stoa)

(*) not the royal one :/

Vulcan added a subscriber: Vulcan.Nov 3 2017, 5:13 PM

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

Updating workspaces...
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimation]’:
FCollada/FCDocument/FCDLibrary.cpp:149:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
  const T* cptr = ((const FCDLibrary<T>*)l1)->GetEntity(0);
           ^
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimationClip]’:
FCollada/FCDocument/FCDLibrary.cpp:150:34:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDCamera]’:
FCollada/FCDocument/FCDLibrary.cpp:151:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDController]’:
FCollada/FCDocument/FCDLibrary.cpp:152:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEffect]’:
FCollada/FCDocument/FCDLibrary.cpp:153:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEmitter]’:
FCollada/FCDocument/FCDLibrary.cpp:154:28:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDForceField]’:
FCollada/FCDocument/FCDLibrary.cpp:155:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDGeometry]’:
FCollada/FCDocument/FCDLibrary.cpp:156:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDImage]’:
FCollada/FCDocument/FCDLibrary.cpp:157:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDLight]’:
FCollada/FCDocument/FCDLibrary.cpp:158:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:159:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDSceneNode]’:
FCollada/FCDocument/FCDLibrary.cpp:160:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsModel]’:
FCollada/FCDocument/FCDLibrary.cpp:161:33:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:162:36:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ s
Nescio added a comment.EditedNov 3 2017, 5:24 PM

(Need also to think of the AI.)
For example, in that patch you remove SpecialBuilding class. Is it something used by the Petra?

No, it isn't.

Following you current patch, they should just inherit from barrack (with their specific cost and perhaps specific production queue).
If we think they are really the same generic type, then a barrack_foo is ok too.

Why insert an additional _barracks_ layer? Isn't _military_ sufficient for structures which train military units?

yes same for some other special things (military colony and hellenic royal (*) stoa)

(*) not the royal one :/

Now you mention it, aren't the non-royal hellenic royal stoas actually embassies? (Limited to two per player; town phase; trains mercenaries; etc.) If so, maybe they could be merged into the embassy.

In D1006#39518, @Nescio wrote:

Now you mention it, aren't the non-royal hellenic royal stoas actually embassies? (Limited to two per player; town phase; trains mercenaries; etc.) If so, maybe they could be merged into the embassy.

Nobody knows :/

bb requested changes to this revision.Nov 17 2017, 4:29 PM
bb added a subscriber: bb.
bb added inline comments.
binaries/data/mods/public/simulation/templates/template_structure_military_embassy.xml
3 ↗(On Diff #4078)

Armour values changed (maybe they were wrong, but for another patch)

25 ↗(On Diff #4078)

all childs have it and should have it => ok

This revision now requires changes to proceed.Nov 17 2017, 4:29 PM
bb accepted this revision.Dec 24 2017, 4:03 PM

Adding the armour values myself, everything except the specialBuilding class is the same, but as that is never used in the sim and is not really correct (imo) for embassies (potentially any civ can have them, not just cart) removing that is fine with me.

This revision is now accepted and ready to land.Dec 24 2017, 4:03 PM
This revision was automatically updated to reflect the committed changes.