getGame(index) and getGames() not returning the real games pgn

[franusierra]

Issue
The getGame(index) and getGames() when returning the games it returns the proper tags but on all the games returns the current game moves as reflected on the code:

    getGame(index: number): PgnGame {
        // this.loadOne(index)
        if ( (this.games === undefined) || ! this.games[index]) {
            return { moves: [] }
        }
        let tags: Tags = this.games[index]?.tags || <Tags>{}
        return { moves: this.getMoves(), gameComment: this.getGameComment() , tags: tags }
    }

As it is reflected here, always return this.getMoves() and this.getGameComment()
The moves variable also seems to have this behaviour from the tests I have done. Searching the code I didn't found a straightforward solution for this.

Motivation
For learning and recreational purposes I am currently working on a new github block with the functionality to view pgn games directly from github using the new functionality github is adding: Github Blocks
The repo is this, if you enter the preview you can view the current functionality here (Very WIP at the moment but pgn games can already be read properly): Blocks ChessPGN

PgnViewerJS seemed an amazing fit and it's what it allows this for proper visualization without any complications, then I thought of a way to rewrite the pgn file to allow users to annotate their pgn with variations and comments from github directly. However I found no sensible way of getting full list of games in a manyGames pgn. Is this actually imposible to perform? I can think of a couple workarounds like using only one game and having external selectors and parsing, but it would be really helpful if the getGames/getGame(index) returned the actual pgn of the concrete game.

Pd: Thanks for the amazing code, i will attribute most of the work that allowed the functionality of the Block to this repo on the finished proyect README

[mliebelt]

I have to dig through some of the issues, if I remember correctly, I have gotten such a feedback already. The viewer is nice, but the code is a mess. I have to search for a reasonable way to have the following APIs available and usable (in the viewer and reader)

• getGames() --> Get a list of games
• getGame(index) --> Get one game

I hope that the internal model allows to improve the interface without too much refactoring. Let us see, if that is possible. I will have a look at your repo, to get an idea how you are using the viewer.

[franusierra]

Yes I am working towards a functional proof of concept at the moment, just been working on it for a couple days, experimenting with the edit content of github blocks and this library as it is the first time using both and didn't know if it would work, before I add the proper code structure, unit testing, better ui, documentation...

I can adapt to any way you see fit to implement it if you see it's plausible. I will work on the code a bit and make it more readable next, and implement another approach without using the readMany and creating an external selector in the meantime. Thanks for the feedback 😄

[mliebelt]

Ok, checked the code again (after some months not touching it). I think the problem is, that the reader has a complex data model (not functional), and you need the following:

• When iterating through the games,
• call first loadOne() with the current index. This will ensure that all code is setup accordingly, and that moves, ... is filled correctly
• then call getGame() with the same index.

I hope that will fix your current problem. Perhaps it would be a good idea to call that either way (when being called with getGame), but the only service using the reader is the viewer, with only one call (when selecting a different game in the drop down list of the viewer).

[franusierra]

I tried that as I saw it commented on the getGame code with the index but with the following line of codes it returned the following error (probably the reason it was commented):

function getFullCurrentPgn(){
    let pgnReader : any= currentBase.current.getPgn();
    let editedCurrentPgnContent="";
    for(let i=0;i<pgnReader.getGames().length;i++){
      pgnReader.loadOne(i);
      let game=pgnReader.getGame(i);
      console.log(game);
      console.log(pgnReader.configuration);
      editedCurrentPgnContent+=writeGame(game, pgnReader.configuration)+"\n\n";
    }
    return editedCurrentPgnContent;
  }

Actually after some thinking (should do that more often) I guess it would be a better approach for the block project to do the following:

1. Parse the pgn loaded from the file using your parser
2. Create a custom selector with all the games previously loaded
3. Create an individual view for the loaded pgn and allow the user to edit, allowing to save the current modifications
   This would allow to implement with the github styling the game selector on the top of the block along with the save button and possibly an stockfish analisis button in the future, so I don't think the getGame and getGames are neccesary in this use case.

Thanks a lot for getting back to me so fast and developing this library.

[mliebelt]

Sorry for being late ... I want at least to ensure that your usage will work. I have only a few test cases that check the switch of games in a very rough way (for the reader), the rest is done in the viewer. So the reader should be robust in switching games. I will try to create some more test cases for it. Could you give me the data you were using (the PGN, including the configuration) so that I can reproduce the problem?

So the plan will be:

1. Fix the current problem without much refactoring.
2. See if a refactoring is (easily) possible, so that the reader cares only about one game, not many. This is the normal use case, and should not be polluted with handling many games.
3. Pre-requisit for that is, that games are only loaded on demand. Rough idea is: split games, hold each text in an array (outside the reader); instantiate the reader with one game only.