Classical crowded #179
Classical crowded #179
Conversation
There was a problem hiding this comment.
Please could we have a statement about the svgs for the flags? Did you create them? Are we ok to use them?
Incidentally - I don't think it's necessary to repeat the units and flags from the Classical variant. We should be able to reuse the existing declaration.
| PhaseTypes = []godip.PhaseType{godip.Movement, godip.Retreat, godip.Adjustment} | ||
| Seasons = []godip.Season{godip.Spring, godip.Fall} | ||
| UnitTypes = []godip.UnitType{godip.Army, godip.Fleet} | ||
| SVGUnits = map[godip.UnitType]func() ([]byte, error){ |
There was a problem hiding this comment.
We should be able to reuse the existing images:
SVGUnits: classical.SVGUnits,
There was a problem hiding this comment.
In fact I think we only need to define Nations and SVGFlags here - the rest can be reused from Classical.
| }, | ||
| Start: ClassicalCrowdedStart, | ||
| Blank: ClassicalCrowdedBlank, | ||
| BlankStart: func() (result *state.State, err error) { |
There was a problem hiding this comment.
I don't think we need to define this here. Only Classical contains it.
There was a problem hiding this comment.
Adding the different nations changes the start units which is set in ClassicalCrowdedStart, or am I getting that mixed up?
There was a problem hiding this comment.
The variant's nations are set below in the line:
Nations: Nations,
| SVGVersion: "9", | ||
| SVGUnits: SVGUnits, | ||
| SVGFlags: SVGFlags, | ||
| CreatedBy: "Timothy J. Brooks", |
There was a problem hiding this comment.
We don't actually have a ImplementedBy field. That would be a nice feature, but you will always be credited in the git history as having implemented this variant. I think this should be CreatedBy: "Unknown" based on what vDip says?
| SVGFlags: SVGFlags, | ||
| CreatedBy: "Timothy J. Brooks", | ||
| Version: "", | ||
| Description: "An expanded version of the original Diplomacy.", |
There was a problem hiding this comment.
It might be nice to mention that there are four extra nations.
couldn't be bothered trying to figure out if the flags would be copyrighted or not, so I'm removing them
| } | ||
| } | ||
| Parser = classical.Parser | ||
| ) |
There was a problem hiding this comment.
I think the only special variables you need to declare in this block are Nations and SVGFlags. All the others you can just reference as classical.Whatever. I think that you need to do something slightly different for SVGFlags, otherwise you won't get the seven standard flags included.
| return phase.Ty == godip.Retreat && phase.Se == godip.Fall | ||
| } | ||
|
|
||
| func NewPhase(year int, season godip.Season, typ godip.PhaseType) godip.Phase { |
There was a problem hiding this comment.
I don't think NewPhase or AdjustSCs are needed either.
| } | ||
| } | ||
|
|
||
| var provinceLongNames = map[godip.Province]string{ |
There was a problem hiding this comment.
We shouldn't need this - in France vs Austria we just use:
ProvinceLongNames: classical.ClassicalVariant.ProvinceLongNames
This reverts commit 6dc4827.
| Balkans: "#CD926A", | ||
| Benelux: "#F79D10", | ||
| Iberia: "#A6517B", | ||
| Scandinavia: "#436186", |
There was a problem hiding this comment.
I think you need to also include the classical nation colours.
| return Asset("../classical/svg/turkey.svg") | ||
| }, | ||
| } | ||
| Parser = classical.Parser |
There was a problem hiding this comment.
You can just write classical.Parser below - instead of declaring a new variable up here.
Using the original map,this variant adds in the powers of Scandinavia, Iberia, Benelux, and the Balkans.