GNOME Bugzilla – Bug 726455
Force engines to move after some timeout
Last modified: 2015-07-15 18:58:13 UTC
Right now there is nothing to prevent a buggy engine from living forever without moving, causing the game to appear to hang. We've become robust to every other engine error that I can think of, so we should handle this as well. (This isn't high priority since I've never seen this happen.) There should be a long timeout (e.g. 30 seconds) after which we attempt to force the engine to move immediately and display a fatal error if it can't.
30s is too long; should be more like 10s
(In reply to Michael Catanzaro from comment #0) > Right now there is nothing to prevent a buggy engine from living forever > without moving, causing the game to appear to hang. We've become robust to > every other engine error that I can think of, so we should handle this as > well. (This isn't high priority since I've never seen this happen.) > > There should be a long timeout (e.g. 30 seconds) after which we attempt to > force the engine to move immediately and display a fatal error if it can't. Hi Michael The fatal error needs to show up as a dialog? and post the fatal error, I assume we would want to end the game. Please correct me if you have an alternate approach in mind.
Hi! Instead of displaying a dialog, I would use the existing error reporting mechanism: end the game using ChessResult.BUG, ChessRule.BUG. That displays an infobar with a generic error message about the computer being confused. To test this, you might be able to stop the chess engine using System Monitor.
(In reply to Michael Catanzaro from comment #3) > Hi! Instead of displaying a dialog, I would use the existing error reporting > mechanism: end the game using ChessResult.BUG, ChessRule.BUG. That displays > an infobar with a generic error message about the computer being confused. > > To test this, you might be able to stop the chess engine using System > Monitor. Thanks for the inputs Michael, will get back with a patch.
Created attachment 307330 [details] Testing done Using the system monitor was a good idea, but I reduced the engine timeout to 1 second to test this.
Created attachment 307331 [details] [review] Prevent a buggy engine from living forever
Created attachment 307333 [details] [review] Prevent a buggy engine from living forever The previous patch let the timer move for another 2 seconds after the issue occurred. Marking that as OBSOL.
Review of attachment 307333 [details] [review]: Thanks! It looks good, push after fixes: ::: src/gnome-chess.vala @@ +817,3 @@ } + int engine_timeout = 10; This should be listed at the top of the class, with all the other fields (member variables). I'd name it "engine_timeout_counter." @@ +829,3 @@ + engine_timeout -= 1; + + if (engine_timeout <= 0) The check should be == 0, since it's an error for it ever to be less than 0.
(In reply to Michael Catanzaro from comment #8) > Review of attachment 307333 [details] [review] [review]: > > Thanks! It looks good, push after fixes: > > ::: src/gnome-chess.vala > @@ +817,3 @@ > } > > + int engine_timeout = 10; > > This should be listed at the top of the class, with all the other fields > (member variables). I'd name it "engine_timeout_counter." Sure! > > @@ +829,3 @@ > + engine_timeout -= 1; > + > + if (engine_timeout <= 0) > > The check should be == 0, since it's an error for it ever to be less than 0. Can I throw an exception for " < 0" case or assert on timeout > 0 and crash the game for such erroneous situations ?
> Can I throw an exception for " < 0" case Exceptions in Vala are useful for situations where you want to perform some operation but aren't sure if it will succeed or not. They're not useful for programming bugs. > or assert on timeout > 0 > and crash the game for such erroneous situations ? Yes, asserts work well for this. You could add a requires(engine_timeout >= 0) to the function declaration, for example.
(In reply to Michael Catanzaro from comment #10) > Yes, asserts work well for this. You could add a requires(engine_timeout >= > 0) to the function declaration, for example. Awesome!!!!! I didn't know vala supported contract programming. Just looked it up on https://wiki.gnome.org/Projects/Vala/Tutorial
Created attachment 307447 [details] [review] Prevent a buggy engine from living forever Leaving for another review incase there are any more comments, Will push up in a day.
Review of attachment 307447 [details] [review]: LGTM, thanks!
Pushed to master. To ssh://ssareen@git.gnome.org/git/gnome-chess ff3f644..9329b59 master -> master