Add variation and commentary support

daylen / stockfish-mac

Beautiful, powerful chess app for the Mac

http://stockfishchess.org/mac/

GNU General Public License v3.0

511 stars 116 forks source link

Add variation and commentary support #35

Closed adriang133 closed 6 years ago

adriang133 commented 7 years ago

screen shot 2017-10-06 at 20 36 37

Added variations and commentary support - #10
Notation is now clickable
Text view scrolls to currently selected move
PGN move annotations (like !!, !?) are now visible in the notation
Undoing move is now possible in SFMPosition

daylen commented 7 years ago

Nice job! A lot of good functionality 👍

Before we merge this in, however, let's make sure undo/redo works. In your branch, if you play the moves e4 e5 from the starting position, then press cmd + z to undo, the board does not update. Granted, this could be because of my exceedingly bad state management. I took a quick look and couldn't figure it out immediately. I'll continue tracking it down, but you might be aware of a fix already?

adriang133 commented 7 years ago

Thanks! I will take a look at that when I have some time.

I am also investigating a bug with creating variations while the engine is running. For example if you make the moves 1. e4 e5 and then go to e4 and create a variation with 1... c5 with the engine running, it fails because the engine output starts at the second ply and the position already has the first 2 ply played out.

Also, about the upper-bound/lower-bound for the evaluation. You had it set to "*" for both but the unit test was not changed to reflect this so I changed it to --/++. Should I revert this change ?

adriang133 commented 7 years ago

I fixed the issues, and some other glitches that I found. The crash was happening for any moves made while the engine was running (not only variations) and was caused by an extra call to syncToViewsAndEngine, but I cannot figure out why. In your current master branch making a move still calls syncToViewsAndEngine twice but it doesn't crash.

Let me know if you find any other issues :)

adriang133 commented 6 years ago

@daylen can you take another look ?

daylen commented 6 years ago

Sorry for the month long delay! I just merged your branch.

caesss commented 6 years ago

@adriang133 @daylen Looks like the crash is still happening. I just created a new issue about it before I read this. Any fix for this yet?