GNOME Bugzilla – Bug 693423
Mines percentage slider for custom games
Last modified: 2013-11-12 17:15:11 UTC
Created attachment 235512 [details] [review] The Patch I find it very convenient and useful to be able to know/set the percentage of mines in a minefield. This patch adds a slider bellow the existing mine number slider for custom games.
Created attachment 235613 [details] [review] Display a mines percentage slider in custom games. Change: Changed the range from 1-100 to 0-100.
Review of attachment 235613 [details] [review]: Can you rebase this against master? Thanks.
Created attachment 236203 [details] [review] Mines Percentage Slider (Version 3)
Review of attachment 236203 [details] [review]: Sorry for the very late review. This looks mostly good. I think it's one of those rare great preferences that we really ought to have. It doesn't work quite right when the percentage of mines is very, very high. At some point (I guess depending on the size of your board) you can raise the percentage without affecting the number of mines on the field. You can even start a game with 100% mines, but there will be non-mine squares on the board (notably, the first square you click on, and every square around it). If you play on a 4x4 board it actually caps out at 6 mines, where 38%-100% all give six mines. This ought to be handled somewhat better, though I'm not really sure how. I also notice that the number of mines changes at different percentages depending on whether you're going up or down. Using my ridiculous 4x4 board example when pressing + on the percentage widget, you go from 5 mines to 6 mines at 38%, but when pressing -, you go from 6 mines to 5 at 31% Lastly, the + button on the original number of mines widget is no longer greyed out at the appropriate times. As a matter of code style, you've half of the time remembered to put the space between the function name and the opening parentheses, half of the time forgotten.
Created attachment 259572 [details] [review] (Version 4) Mines percentage slider Most of the issues you noted in the previous patch should be fixed with this patch. The issue that remains: > Lastly, the + button on the original number of mines widget is no longer greyed > out at the appropriate times. I can't seem to reproduce this issue or explain why it might be present. Could you please try the new patch and confirm whether the problem persists?
Review of attachment 259572 [details] [review]: Seems good asides from: > > Lastly, the + button on the original number of mines widget is no longer greyed > > out at the appropriate times. > > I can't seem to reproduce this issue or explain why it might be present. Could > you please try the new patch and confirm whether the problem persists? It seems to be happening with both + and - on all of the widgets; I'll attach a video.
Created attachment 259630 [details] Video demonstrating bug Unrelated: it would be good to set the minimum percentage of mines to 1 rather than 0.
Since the issue is present on all widgets, might it be a separate bug unrelated to the patch?
My original patch included a range between 1 and 100, however I changed this so that on large boards (eg 100x100) 1 mine in 10000 rounds down to 0 percent. I know 0 sounds like it should be 0 mines but I don't know a nice way to provide the same range without 0 percent not actually being 0.
(In reply to comment #8) > Since the issue is present on all widgets, might it be a separate bug > unrelated to the patch? Er yes it is, sorry about that. (In reply to comment #9) > My original patch included a range between 1 and 100, however I changed this so > that on large boards (eg 100x100) 1 mine in 10000 rounds down to 0 percent. I > know 0 sounds like it should be 0 mines but I don't know a nice way to provide > the same range without 0 percent not actually being 0. How about not providing the same range with the percentage spinner, and allowing the user to go lower only by using the original spinner? Or even more simply: just not allowing the user to go lower than 1%? I don't see any reason we need to allow creating a game with so few mines.
Created attachment 259676 [details] [review] (Version 5) Mines percentage slider
Accepted, thanks!