Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Occasional Exception Thrown running 2 AIs #11

Closed
codefactor opened this issue Dec 11, 2013 · 7 comments
Closed

Occasional Exception Thrown running 2 AIs #11

codefactor opened this issue Dec 11, 2013 · 7 comments

Comments

@codefactor
Copy link

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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/:

  • Chrome ends with a white queen promotion to checkmate (63 f8=Q#).
  • Firefox comes to a black queen promotion (48 ... a1=Q), which isn't the end of the game. The default display code continues past this without reporting any error.

@douglasbagnall
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants