Page MenuHomeWildfire Games

Briton crannog dock civic center ProductionQueue
ClosedPublic

Authored by elexis on Jun 11 2017, 2:32 AM.

Details

Summary

Since the briton crannog can train the same ships as the docks, can train the same techs and units as the civic center, it would make sense to also train the same technologies as docks provide.
In particular it should be possible to research the town phase in a captured crannog.

Test Plan

Make sure the list is an exact copy of the dock list.

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

elexis created this revision.Jun 11 2017, 2:32 AM
Vulcan added a subscriber: Vulcan.Jun 11 2017, 4:05 AM

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/1533/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/173/ for more details.

fatherbushido accepted this revision.EditedJun 11 2017, 8:50 AM

One could argue it's a cc more than a dock. One can compare also to military colony or to carth super dock. One can wonder also about city phase tech and espionnage techs.
(No clue from my side, all those things are ok).
There was no real point to remove the phase_town. The only behavior change will be when a crannog will be captured. (and also we can see it in the struct tree)?

With the current patch, there is enough room in the gui so that's fine.

EDIT: but it will take an huge room in the struct tree

(so you decide :p)

This revision is now accepted and ready to land.Jun 11 2017, 8:50 AM
mimo added a subscriber: mimo.Jun 11 2017, 10:45 AM

I've also don't have a strong opinion, but

  • i rather see the crannog as a cc, and none of the cc have such techs (soldiers produced at the cc have their techs either in the barracks or blacksmith, and gatherers have dropsites and farms techs). So i'd prefer to keep the naval techs at the dock (and thus keep the dock specificity). On the opposite, i support adding back the phase techs.
  • as the crannog is more a cc than a dock, we should put the cc category in the BuildRestrictions rather than the dock (that's needed for the EntityLimits to know if we already have a cc in phase I to allow building a new one), but the current Dock category is used for placement (i.e. computing the dockAngle): that's a flaw in our design, to use the same category for two different purposes: maybe such placements could be based on the PlacementType rather than the Category?
  • britains don't have bireme (that's why this template was not in the ProductionQueue) and giving the possibility to produce a unit which was not available to britains in a captured crannog does not look right to me. So i would not add it.
  • as the crannog is more a cc than a dock, we should put the cc category in the BuildRestrictions rather than the dock (that's needed for the EntityLimits to know if we already have a cc in phase I to allow building a new one), but the current Dock category is used for placement (i.e. computing the dockAngle): that's a flaw in our design, to use the same category for two different purposes: maybe such placements could be based on the PlacementType rather than the Category?

Yes mimo, see D387 (it's certain we shouldn't use Category, still wonder if we should just use the shore placement type in the code or refers to a special constraint).

elexis updated this revision to Diff 2523.Jun 11 2017, 5:29 PM

Not convinced of leaving out the bireme yet.

I see the point for not adding elephants, siege engines and citizen and champion units to barracks and the like.
But I'd expect to be able to train bireme too after capturing the building if we can train the other ships there.
If that thought is followed, the heavy warship should be added for the same reason.
Notice we have a similar issue with the carthaginian temple:
  Carthaginians can train only spear infantry and cavalry champions, but if britons capture it, they can train swordsmen champions and mauryans can train chariots.

Drop ship technologies.

Maybe it would make sense to drop ship production there as well, as it's a civic center that just happens to be on water?
I don't think there is historical evidence for building ships there for the same reason either.
But I could not find any historical information about briton ships at all and it seems the regular briton dock name (crannoc) is just a variant of the crannog,
especially after reading that History string that talks about scotland and ireland.
Dunno, don't care.
elexis requested review of this revision.Jun 11 2017, 5:29 PM
elexis added a reviewer: mimo.
elexis edited edge metadata.
Carthaginians can train only spear infantry and cavalry champions, but if britons capture it, they can train swordsmen champions and mauryans can train chariots.

That is more due to messy naming of champions templates

mimo edited edge metadata.Jun 11 2017, 6:22 PM

That maybe fine to add biremes which are also available in captured brit docks, but not quinqueremes (which also are not availables in captured brit docks).
Building quinqueremes required some special technologies or very large shipyard which were not available for civs without quinqueremes.

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/1536/ for more details.

mimo accepted this revision.Jun 11 2017, 6:38 PM
This revision is now accepted and ready to land.Jun 11 2017, 6:38 PM
Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/176/ for more details.

This revision was automatically updated to reflect the committed changes.

Build has FAILED

Link to build: http://jw:8080/job/phabricator/1538/
See console output for more information: http://jw:8080/job/phabricator/1538/console

Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/178/ for more details.