Sengoku beautified [WIP]

[JorenC]

Hi @tttppp

Here you can find the latest version of Sengoku.

Question: 1) Why do the fleet units misalign?

Todo: 1) Make it build anywhere 2) Change 19 to 25 SCs needed for victory 3) Add description + attribution 4) Create army unit

[JorenC]

~~Todo: fix izu connections ~~

[JorenC]

@tttppp I think this is ready to go.

The only thing it still needs is the "build everywhere" rule, but this is one level too high. Can you help me make it build anywhere, do a final check and then upload?

[tttppp]

1. This needs orders for the neutral units too.

2. Did you add coasts yet? If not then you need to add those.

3. Build anywhere is fairly easy to add - just look at the example in Chaos:https://github.com/zond/godip/blob/master/variants/chaos/chaos.go#L274 You have to replace any reference to the classical.Parser with the hundred.BuildAnywhereParser (make sure to import the new parser).

4. You also haven't removed you debugging change in the test file that I pointed out last time.

5. There are a load of svg files in the svg directory. Are these needed? Please remove any extras and then regenerate the bindata file.

[JorenC]

@tttppp 4/5 should be fixed now.

1. This needs orders for the neutral units too. The neutral units should always hold. In Europe1939, I can't find any description or any other reference to orders?

2. Did you add coasts yet? If not then you need to add those. I think these are created automatically? What coasts should I create? There are Coast... connections and the orders look to be good?

3. Build anywhere is fairly easy to add - just look at the example in Chaos:https://github.com/zond/godip/blob/master/variants/chaos/chaos.go#L274 You have to replace any reference to the classical.Parser with the hundred.BuildAnywhereParser (make sure to import the new parser). Done

4. You also haven't removed you debugging change in the test file that I pointed out last time. Because I'm still debugging. I wanted to wait for this as last step. Otherwise I have to manually update each time I want to run a test. Fixed now.

5. There are a load of svg files in the svg directory. Are these needed? Please remove any extras and then regenerate the bindata file. I've added the crests of all the different clans, which are the same as the flags of the classical countries, so each SVG is actually a SVG that is required.

[tttppp]

1. You're exactly right, I didn't notice that. If you want them to be rebuilt then you need the code from Western World 901, otherwise they'll disband like in Europe 1939.
2. If a province has multiple coasts then you need to add this to the svg and go file. I only spotted one province that looked like it should have two coasts, but maybe there are others? The Coast... connections are unrelated - they refer to routes that armies and fleets can take. 3-5. Great work!

[JorenC]

@tttppp

1) You're exactly right, I didn't notice that. If you want them to be rebuilt then you need the code from Western World 901, otherwise they'll disband like in Europe 1939. I thought they should disband, misread the orders. Added the extra import from WW901, but please double check this carefully as I have no clue what I'm actually doing.

If a province has multiple coasts then you need to add this to the svg and go file. I only spotted one province that looked like it should have two coasts, but maybe there are others? The Coast... connections are unrelated - they refer to routes that armies and fleets can take. Duh, forgot about THOSE coasts - because this doesn't have any. Unless you spotted them with your amazing eye as you always find stuff I missed...

If (1) and (2) are clear, I think we're completely clear to go.

[tttppp]

Hizen is the only one I can spot. It looks like it should have North and South coasts.

[JorenC]

@tttppp you and your amazing eye you! Hizen actually has one coast only, but there was still a big in the merging of files that I fixed in the province itself, but not in the background layer - thus it appeared with weird lines in between. Should be fixed now.

[tttppp]

Oh, amazing stuff! I assume you also want to hide the old-style SC symbols?

[JorenC]

@tttppp doh. Should be fixed now -_-

[tttppp]

New issues I spotted:

6. You need to regenerate the bindata file.
7. There's no land movement between Suruga and Sagami.
8. Some of the movement arrows are very short, causing them to have inverted tails. Some are also significantly overlapping, although this doesn't necessarily need anything changing - I just wanted you to check them.
9. Sagami Bay is marked as land (coast), and Izu is marked as sea. I assume these two are swapped over somehow.
10. There are some small issues with the go code. I've fixed these locally as follows:

diff --git a/variants/sengoku/sengoku.go b/variants/sengoku/sengoku.go
index 78f15ca..853a1a3 100644
--- a/variants/sengoku/sengoku.go
+++ b/variants/sengoku/sengoku.go
@@ -3,12 +3,11 @@ package sengoku
 import (
    "github.com/zond/godip"
    "github.com/zond/godip/graph"
-   "github.com/zond/godip/state"
    "github.com/zond/godip/phase"
+   "github.com/zond/godip/state"
    "github.com/zond/godip/variants/classical"
    "github.com/zond/godip/variants/common"
    "github.com/zond/godip/variants/hundred"
-   "github.com/zond/godip/variants/westernworld901"
 )

 const (
@@ -52,35 +51,12 @@ func Phase(year int, season godip.Season, typ godip.PhaseType) godip.Phase {
    return newPhase(year, season, typ)
 }

-func NeutralOrders(state state.State) (ret map[godip.Province]godip.Adjudicator) {
-   ret = map[godip.Province]godip.Adjudicator{}
-   switch state.Phase().Type() {
-   case godip.Movement:
-       // Strictly this is unnecessary - because hold is the default order.
-       for prov, unit := range state.Units() {
-           if unit.Nation == godip.Neutral {
-               ret[prov] = orders.Hold(prov)
-           }
-       }
-   case godip.Adjustment:
-       // Rebuild any missing units.
-       for _, prov := range state.Graph().AllSCs() {
-           if n, _, ok := state.SupplyCenter(prov); ok && n == godip.Neutral {
-               if _, _, ok := state.Unit(prov); !ok {
-                   ret[prov] = orders.BuildAnywhere(prov, godip.Army, time.Now())
-               }
-           }
-       }
-   }
-   return
-}
-
 var SengokuVariant = common.Variant{
    Name:              "Sengoku",
    Graph:             func() godip.Graph { return SengokuGraph() },
    Start:             SengokuStart,
    Blank:             SengokuBlank,
-   Phase:             NewPhase,
+   Phase:             Phase,
    Parser:            hundred.BuildAnywhereParser,
    Nations:           Nations,
    PhaseTypes:        classical.PhaseTypes,
@@ -108,10 +84,9 @@ var SengokuVariant = common.Variant{
 }

 func SengokuBlank(phase godip.Phase) *state.State {
-   return state.New(SengokuGraph(), phase, classical.BackupRule, map[godip.Flag]bool{godip.Anywhere: true}, NeutralOrders)
+   return state.New(SengokuGraph(), phase, classical.BackupRule, map[godip.Flag]bool{godip.Anywhere: true}, nil)
 }

-
 func SengokuStart() (result *state.State, err error) {
    startPhase := Phase(1570, godip.Spring, godip.Movement)
    result = SengokuBlank(startPhase)

Note that I assumed you wanted the neutral units to disband when dislodged, and never be rebuilt (I think you said this earlier). If you want them to be rebuilt then I can provide a different patch.

[JorenC]

6. You need to regenerate the bindata file. Done, I'll do it after every 'done' again.
7. There's no land movement between Suruga and Sagami. Well, well spotted. Fixed.
8. Some of the movement arrows are very short, causing them to have inverted tails. Some are also significantly overlapping, although this doesn't necessarily need anything changing - I just wanted you to check them. Indeed there are some quite short arrows, but I don't think it's an issue. I don't see inverted tails (e.g. the arrow pointing the wrong direction?), but I think this is fine. Let's push how it looks in a couple of real matches, maybe we can tweak it later - but I'd do this lean and first try, then fix.
9. Sagami Bay is marked as land (coast), and Izu is marked as sea. I assume these two are swapped over somehow. Yes, I fixed them as connections (Sea/Coast), but I missed there was a flag. Fixed now.
10. There are some small issues with the go code. I've fixed these locally as follows: This one SHOULD rebuild. Could you help me provide a different patch - when you have time?