douglasbagnall / p4wn

a smallish turn-of-the-century javascript chess engine
http://p4wn.sf.net
113 stars 33 forks source link

Occasional Exception Thrown running 2 AIs #11

Closed codefactor closed 10 years ago

codefactor commented 10 years ago

I'm new to this library - but I found a possible issue - and it doesn't always happen due to randomness. I am basically running a game over and over again with AI on both sides until the user asks to join. I've noticed that there is an error during the state.move()

Here is some javascript - if you let it run for a little while (5 minutes or so) you will get the issue near the end of the game. I am guessing perhaps it was supposed to be a draw.

Here is the simple test code:

Live Example: http://jsfiddle.net/MB9kk/

(function(){
    var g, flags;
    function start() {
        flags = 1;
        g = p4_new_game();
        next();
    }
    function next() {
        if ((P4_MOVE_FLAG_DRAW | P4_MOVE_FLAG_MATE) & flags) {
            console.log(P4_MOVE_FLAG_MATE & flags ? 'Mate!' : 'Draw!');
            start();
        } else {
            var mv = g.findmove(5);
            flags = g.move.apply(g, mv).flags; // The move function throws an error
            setTimeout(next, 10);
        }
    }
    start()
})();

The error output is like this:

Uncaught TypeError: Cannot read property '91' of undefined 

And the line is at engine.js:389

function p4_parse(state, colour, ep, score) {
    var board = state.board;
    var s, e;    //start and end position
    var E, a;       //E=piece at end place, a= piece moving
    var i, j;
    var other_colour = 1 - colour;
    var dir = (10 - 20 * colour); //dir= 10 for white, -10 for black
    var movelist = [];
    var captures = [];
    var weight;
    var pieces = state.pieces[colour];
    var castle_flags = (state.castles >> (colour * 2)) & 3;
    var values = state.values[other_colour];
    var all_weights = state.weights;
    for (j = pieces.length - 1; j >= 0; j--){
        s = pieces[j][1]; // board position
        a = board[s]; //piece number
        var weight_lut = all_weights[a];
        weight = score - weight_lut[s];  // <-------- This line is throwing the error

Temporarily what I've done is to put a try { } catch around the call to .move() and if there is an exception, I make it so the current player has resigned.

Note - I am only using the engine part with my own entirely different display.

douglasbagnall commented 10 years ago

Thanks codefactor. This is a good report.

Uncaught TypeError: Cannot read property '91' of undefined

Is it always 91? I think that is a rook's corner, which possibly points to problems with the castle checks.

If you want to remove the randomness, you could search for a problematic game a bit like this:

(function(){
    var g, flags;
    var seed = 1;
    function start() {
        console.log("using random seed " + seed);
        flags = 1;
        g = p4_initialise_state();
        p4_random_seed(g, seed);
        p4_fen2state(P4_INITIAL_BOARD, g);
        seed++;
        next();
    }
  //...

Then when you get the error, if you change the initial seed to the logged number, it should occur right away.

I don't have time to look at it right away, but I will try to soon.

codefactor commented 10 years ago

Thank you for replying so quickly - The number 91 is not consistent from one run to the next.

In the code example its hard to follow the game from just the logs - but on my real page you can see the game and the 2 AI players duke it out until there are no pieces left on one side except the black player's king. I can't remember all of the other pieces left on the white side.

codefactor commented 10 years ago

Using your way of setting the random seed will give the error on the 2nd time - so setting the seed to 2 will do the trick. Here is the fiddle: http://jsfiddle.net/MB9kk/1/

douglasbagnall commented 10 years ago

Aha. Does this version work better? https://github.com/douglasbagnall/p4wn/blob/b99a9e0ca037f794f258bf2e85422a0c99a117b9/src/engine.js

That's from before commit 660dcac, which tries to recognise a draw from insufficient material.

Edit: the raw JS is here: https://raw.github.com/douglasbagnall/p4wn/b99a9e0ca037f794f258bf2e85422a0c99a117b9/src/engine.js

codefactor commented 10 years ago

Seems still to give the same problem. I pasted the code from the link and ran it in jsFiddle, same error appears to happen.

douglasbagnall commented 10 years ago

It looks like these are queening moves. Firefox and Chromium play different games (which is a bug in itself).

With the random seed 2, as per http://jsfiddle.net/MB9kk/1/:

douglasbagnall commented 10 years ago

This is the problem:

            var mv = g.findmove(5);
            flags = g.move.apply(g, mv).flags;

findmove() returns [start, end, score], while move() wants start, end, promotion, where promotion is the desired promotion (i.e. queen, knight, whatever) if the move gets a pawn to the end. So this use of apply is wrong, but that only shows up when you're trying to promote a pawn. Unless the score returned by findmove() happens to represent a valid piece, move() will try to use an undefined lookup table.

I can see the attraction of symmetry between findmove and move, but the score can actually be useful to findmove() callers (so they can, e.g., decide to try again with a deeper search). At the same time, it seems to me that findmove ought to return the chosen promotion, rather than relying on the display side to assume a queen (which is the only thing findmove considers, but that could change one day).

So the upshot is this works (http://jsfiddle.net/MB9kk/2/) :

            var mv = g.findmove(5);
            flags = g.move(mv[0], mv[1], P4_QUEEN).flags;

and I won't change the signatures of the functions before the next big version, so I'll close this.