search

chrislicodes / obsidian-chess-study

A chess study helper and PGN viewer/editor for Obsidian.
GNU General Public License v3.0
32 stars 4 forks source link

Undo button #12

Closed latenitecoding closed 7 months ago

latenitecoding commented 7 months ago

Provides the undo-button with functionality. Removes the last move in the draft. Also displays the previous move if the current move is the last move.

latenitecoding commented 7 months ago

I also noticed a bug while I was working on this PR that causes the board to crash when drawing shapes, syncing comments, or using the arrow buttons before making any moves. It's because there are no moves to sync to so you end up using null values. I've added guards to most of the cases that check to see that there is at least one move on the board.

latenitecoding commented 7 months ago

With that change, you can still draw on an initial board, you just can't sync the shapes. There is a better fix which involves creating a default move that represents the initial board state. However, this is a much more complicated fix since that move would have to be added as the first move to your moves list. This idea does not exist anywhere in the code, so it is much more likely to introduce several bugs at this point. We would need to carefully comb through the code to ensure that a new 0th move doesn't mess up other functions like getCurrentMove, findMoveIndex, display previous, undo, etc.

latenitecoding commented 7 months ago

The current solution for the time being is to just to put a first move onto the board that can be synced to.

chrislicodes commented 7 months ago

Nice catch! I'd be happy to collaborate with you on refactoring this code, particularly the handling of variants, if you're interested. :)

latenitecoding commented 7 months ago

Nice catch! I'd be happy to collaborate with you on refactoring this code, particularly the handling of variants, if you're interested. :)

Thanks for the offer. I'm also happy to collaborate anytime. I was able to handle the variant cases as well. I'd love to take a look at variance depth > 1 at some point.

I've just finished refactoring the commit history so that each PR is somewhat self-contained. They are all dependent on PR #10 . I can refactor further, but since PR 10 is against trunk, this saves time assuming that PR gets merged in.