search

daylen / stockfish-mac

Beautiful, powerful chess app for the Mac
http://stockfishchess.org/mac/
GNU General Public License v3.0
506 stars 116 forks source link

Bug in cmd-D "Do best move"? #37

Closed caesss closed 6 years ago

caesss commented 6 years ago

When Cmd-D (do best move) is pressed you get this screen? this is not in the prod version is there a reason for this screen? cancel works, create variation also works but overwrite crashes the app. screen shot 2017-12-18 at 9 57 08 pm

adriang133 commented 6 years ago

There was a small issue with doBestMove. Fixed in #38.

caesss commented 6 years ago

thank you. your code fixes the issue.

caesss commented 6 years ago

Thank you for your quick response @adriang133 Consider taking a look at "doMoveWithOverwritePrompt" it appears that your code is crashing when this is called now. I'm not sure if I should open a new issue or keep it here since your fix affected the other function. Thank you again.

To be clear, start a game make a few moves. Then exit and reopen. The previous game is loaded, start the engine analysis and select "do move" this triggers the overwrite prompt. When you select "override" the app crashes

"Assertion failed: (this->color_of_piece_on(from) == us), function do_move, file /Stockfish/Chess/position.cpp, line 772."

adriang133 commented 6 years ago

Can you please be more specific about reproducing this ? I couldn't do it.

I am making a few moves, reopening, previous moves are there, I start engine analysis. What do you mean by "do move" ? Is that Do Best Move ? I tried it and it works fine for me. I also tried overwrite a few times and it seems to work.

caesss commented 6 years ago

Let me know if this helps. The crash actually comes from file /Stockfish/Chess/position.cpp, line 772. By the way, I'm using: Xcode 9.2, MacOS HighSierra and the latest Stockfish Engine binaries in case that makes a difference. stockfish

adriang133 commented 6 years ago

I still could not reproduce that for some reason. Are you sure you're on the latest commit ? And can you maybe try with the binaries that are included in this repo ? (though I doubt this can have anything to do with it)

I have found another bug however, when trying to reproduce yours. For me the analysis still stays on the second ply (instead of throwing an error as you get), but I did not have time to investigate yesterday.

caesss commented 6 years ago

that's strange, yes I downloaded the whole thing again just to make sure. The only thing I can think of is to make sure the engine is "running" while you are making the overwrite move. If the engine is not running the error doesn't happen but it does crash when it is active.

adriang133 commented 6 years ago

I think I was actually getting the error but for some reason it just showed up in XCode while the execution continued. Maybe it's because I played with some XCode settings or it's just the newer XCode version...

It was a bug where the game state was updated wrongly when you overwrite a move. I fixed it in the same PR, can you please confirm ?

caesss commented 6 years ago

@adriang133 That does it. I appreciate your work. I will keep looking through the code to see if I can help but I'm more of Swift guy. I'm closing the ticket now and I hope @daylen does the merge soon. Thank you again!