shaack / cm-pgn

Parse and create PGNs (Portable Game Notation for chess games)
MIT License
27 stars 21 forks source link

The first move of a variation over-writes the previous move's "next" pointer to point to the alternate line rather than its own line #20

Closed donkawechico closed 9 months ago

donkawechico commented 11 months ago

Overview

A bit tricky to describe so I've written a unit test to make it a bit clearer:

    it('should create history with correct next for main line moves when variations present', () => {
        const pgn = new Pgn(`[SetUp "1"]

            1. e4 e5 2. Nc3 (2. Nf3 Bc5) 2... Nc6 *`)

        const ply2_main_line = pgn.history.moves[1];
        const ply3_main_line = pgn.history.moves[2];
        const ply3_variation_line = pgn.history.moves[2].variations[0][0];

        assert.equal(ply2_main_line.next, ply3_variation_line); // <--- This is currently passing (and shouldn't, I think)
        assert.equal(ply2_main_line.next, ply3_main_line); // <--- This should pass but is failing
    })

Expectation

Each move in a move.variation array should have its next (and previous) pointers be consistent with what is actually previous and next in its variation array.

For example. If someMove.variation = [move1, move2, move3, move4], then move1.next should be move2, move2.next should be move3, and so on regardless of whether a given move in that line is a branching point.

Actual

If a move has a variations array of arrays, then the first move of one of those variations over-writes the previous line's last move to have its "next" pointer point to the alternate line rather than its own line.

Suggested Fix

I have a PR I've pushed (https://github.com/shaack/cm-pgn/pull/21) that fixes the issue by changing traverse to check whether a given move already has a previous or next pointer before setting it.

shaack commented 9 months ago

Thank you for fixing it with a PR.