Ordering nonsensical convoys doesn't provide clear feedback to the user #227

Open
TimothyJones opened this Issue Feb 24, 2016 · 4 comments

Projects

None yet

3 participants

@TimothyJones
Collaborator

If we have a classic game with:

A Bre
F MAO
F NAO

and we try to order NAO convoy Bre->Spain (an unnecessary order), then the convoy order partially disappears as the javascript validator doesn't handle this case gracefully. The orders are then left in a state where they can't be saved.

This is particularly a problem for the World map, since nonsensical convoy orders are possible much more often (and with similarly named territories, can happen easily).

We should alter the javascript to prevent nonsense convoys from being ordered.

@TimothyJones TimothyJones added this to the Soon milestone Feb 24, 2016
@yewang
Contributor
yewang commented Jun 7, 2016

Maybe the validator should not try to eliminate nonsensical convoys, and instead just let them be ordered. When looking at only one set of orders, it can be impossible to tell if a convoy order is nonsensical/unnecessary or if it is indeed a vital segment of an internationally coordinated convoy.

Further, it is sometimes nice to be able to order nonsensical convoys to communicate to other players in gunboat, and reducing the validator complexity may speed things up and avoid bugs.

What are your thoughts? I can take a look at the javascript to see how to address this.

@kestasjk
Owner
kestasjk commented Jun 7, 2016

Is Bre-Spain really unnecessary? I vaguely remember in the DATC tests there were some cases involving bypassing a head to head because of a convoy possibility.

@TimothyJones
Collaborator

The example nonsensical order here is NAO covoy Bre->Spain, since that fleet can't ever be used meaningfully in the convoy.

@yewang: I suspect the adjudicator relies on some of the pruning that the validator does - @kestasjk, do you know if this is correct?

@kestasjk
Owner
kestasjk commented Jun 7, 2016

Ah I see, my territory acronyms are rusty. ;)

The adjudicator does assume orders are valid, but to clarify (as you probably already know) the server-side code does not assume that the client-side order validation has been run. The client will submit a convoy chain for any submitted convoy order, so that the server can quickly validate the chain rather than search for a valid chain itself.

Looking at the JS now, and bearing in mind I'm very rusty, here is rough code which I think would prune out invalid orders:
https://github.com/kestasjk/webDiplomacy/blob/master/javascript/board/model.js#L241
getConvoyFromChoices : function(ToTerritory) { if( this.convoyLink ) { this.convoyOptions = this.ConvoyGroup.Armies.select(function(a){return a.Territory!=ToTerritory;}).pluck('Territory').select(function(fromTerritory) { // Ensure there is a valid path from this possibility to ToTerritory via the fleet territory: var path = this.ConvoyGroup.pathArmyToCoastWithFleet(fromTerritory, ToTerritory, this.Territory); return !Object.isUndefined(path) && path.length > 0; },this).pluck('id'); return this.convoyOptions; } else return [ ]; }
The problem is it will do a depth first search for every potential territory the fleet could convoy from. I'm not sure if this would perform acceptably in practice (Especially in large maps).

Doing it more efficiently would probably mean adding an alternative to the NodeSetClass' findPath function which can take in a start / convoy-from territory and middle / this-fleet territory and efficiently find valid end / convoy-to territories, and making pathArmyToCoastWithFleet use this altered findPath.

This would be doable but it'd take some thought.

A simpler but less nice option would make it detect that there's no path /after/ the user has selected an invalid end / convoy-from territory, as in order.js' postUpdate(), and more gracefully handle an order being recognized as being valid after selection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment