zond / godip

A dippy adjudicator in Go.
GNU General Public License v3.0
27 stars 22 forks source link

Adjacent Convoys Issue #128

Open zond opened 3 years ago

zond commented 3 years ago

Copied from https://github.com/zond/diplicity/issues/171


There is an issue with the convoys between adjacent provinces, if the target province is empty.

In sandbox mode:

Austria: A Nap - Rom (regular move, not via convoy) F TYS C A Nap - Rom France: F WMS - TYS F ION S F WMS - TYS

Since Austria provides a convoying fleet, the move Nap - Rom is interpreted as 'via convoy', despite not being marked as 'via convoy'. Since Austrian fleet is dislodged, the convoy is disrupted. The army move from Naples to Rome fails.

Compare this to, also in sandbox mode:

Austria: A Nap - Rom via convoy F WMS - TYS F ION S F WMS - TYS

France: F TYS C A Nap - Rom

Same situation, but now the move is explicitly marked as a "move via convoy", and the convoying fleet is French. The convoy is still disrupted. In this case, the army move from Naples to Rome will default to a move via land, and it will succeed.

I believe that community consensus (and this agrees with the most probable interpretation of the rulebook) is that the army move should succeed in both cases. Either way, it's somewhat strange that the outcome is different in the two cases.

zond commented 3 years ago

@tttppp wrote (in https://github.com/zond/diplicity/issues/171):

I spent 15 minutes reading through DATC, and didn't come up with a definite answer about this. There's a lot in there about "intent", which is determined by checking if the nation moving matches the nation convoying. I'm happy to spend some more time reading rules and test cases tomorrow. Given the number of tests we have then maybe we could try to change to the requested behaviour and see if any fail?

Yeah, the reason I think we should change it is that if the order is executed 'by any means necessary' when someone else is convoying, it should also when you are convoying.

I think that using your own fleet should only be interpreted as 'via convoy', not cause the move to fail if the fleet is dislodged. At least that feels more correct.

And yeah, changing the behavior and seeing what tests fail sounds good - I haven't even looked at how to change it though. The convoying code is messy.