Page MenuHomePhabricator

ChessBrowser fail to parse games with notation/comments
Open, MediumPublic

Description

See https://en.wikipedia.beta.wmflabs.org/wiki/User:קיפודנחש1/ChessBrowser_test_cases.

the page has several pgn tags. the 1st fails for a different reason, the 2nd fails because it contains comments.

the pgn is one of the test cases: includes/PgnParser/test/pgn/annotated.pgn

peace.

Event Timeline

Wugapodes triaged this task as Medium priority.Jul 18 2021, 8:29 PM
Wugapodes moved this task from Needs triage to Planned Features on the ChessBrowser board.
Wugapodes subscribed.

I've looked into this and there seem to be three main problems

  1. The validation regex doesn't (yet) allow variations and comments because
  2. The codebase doesn't (yet) translate the PHP parser's comment notation into something the JS module can read, but even if it could
  3. The given example uses numerical annotation glyphs (NAGs; see the standard) which are, in my experience, uncommon in the unicode era. I'm not sure the parser understands NAGs, and even if it did, I'm wondering whether we should support them at all.

Change 705341 had a related patch set uploaded (by Wugapodes; author: Wugapodes):

[mediawiki/extensions/ChessBrowser@master] WIP: Implement comment and variation parsing

https://gerrit.wikimedia.org/r/705341

@Wugapodes :
it says

159 | ERROR | Missing function doc comment in includes/ChessParser.php

it seems that createAnnotationJson() needs doc.

peace.

The latest patch set implements very rudimentary variation and comment display. The JS doesn't yet handle the display of variation board states, and even if it did, it's not clear how to handle the branches with the < and > buttons. Regardless, I still need to fix the tests now that it's somewhat stable so that the builds succeed, and then will submit for review. Further refinement can be done in later patches, and I'll write more about my ideas in the coming days.

@Wugapodes
now it seems that it does not like the question mark in the doc, and complains about

@param array? $fenParts The ....

on lines 149 and 214 of includes/ChessParser.php

i have no idea what the question mark means, but you may want to try to get rid of it.
peace.

Change 705341 merged by jenkins-bot:

[mediawiki/extensions/ChessBrowser@master] Implement comment and variation parsing

https://gerrit.wikimedia.org/r/705341

@Wugapodes : can you see why 3rd game in https://en.wikipedia.beta.wmflabs.org/wiki/User:%D7%A7%D7%99%D7%A4%D7%95%D7%93%D7%A0%D7%97%D7%A91/ChessBrowser_test_cases still fails to parse?
i copied it from one of the test cases in the repo.

note that this notation contains embedded comments, both curlies and parentheses. maybe a tough cookie (theoretically the pgn could be indeed invalid - i don't have tools to verify.
the standalone took the easy rout and only supports single level, with curlies only.

i also noticed the commit message, which implies, iiuc, that the front-end can display variations.
surprisingly, the front-end does not have(yet?) any code to support it.
is this functional?

peace

@Wugapodes : can you see why 3rd game in https://en.wikipedia.beta.wmflabs.org/wiki/User:%D7%A7%D7%99%D7%A4%D7%95%D7%93%D7%A0%D7%97%D7%A91/ChessBrowser_test_cases still fails to parse?
i copied it from one of the test cases in the repo.

The examples uses numeric annotation glyphs. See T290543.

i also noticed the commit message, which implies, iiuc, that the front-end can display variations.
surprisingly, the front-end does not have(yet?) any code to support it.
is this functional?

It displays the algebraic notation but does not yet support viewing the actual board position. The blocker on that is the user interface for navigating between main line and variations. I'm thinking the navigation buttons always default to mainline and readers can select a variation by clicking on it.

This seems mostly an issue with nested variants or something. Especially this fragment seems problematic.

<pgn>
[Event ""]
[Site ""]
[Date ""]
[White "Svidler"]
[Black "Shirov"]
[Result "1-0"]
[Source "Russian Chess"]
[Annotator "Svidler"]

15...Qf6
{Played very quickly again. Other moves arex } (15...Qh4 16.Be3 exd4 17.cxd4 Ba5 18.Nc3 { and since Black queen is not attacking f3, White is just a pawn up. })
(15...exd4 16.Nxd4 $1 (16.cxd4 $6 Qf6 17.Be3 Ba5 {transposes to Ivanchuk-Shirov}) 16...Bxd4 (16...Nxd4 17.cxd4 Qf6 18.Ra4+) 17.cxd4 Nxd4 (17...Qf6 18.Ra4+) 18.Qxd4 Rxb3 { and here comes the highpoint of White's ideax } 19.Qd5 $1 $20)
1-0
</pgn>

Yes there were two issues. Around move 20 there was a period and unmatched close-brace that were messing with the validation. Those were fixed, but the remaining problem is nested variations. This is partly a UI/UX issue touched on at T239440.

  1. Displaying nested variations inline is not user friendly and makes the game hard to read. It should be avoided if possible, and the hierarchical nature of the variations conveyed visually (and structurally in the HTML for accessibility)
  2. The problem of navigating to and from the variation is amplified. As it is, readers cannot navigate to moves within a variation because we don't have an interface for moving in and out of these lines other than clicking on them. With nested variations this is still a problem that will need resolved.