GNOME Bugzilla – Bug 694244
A possible error while changing default controls
Last modified: 2014-08-21 02:14:21 UTC
Please describe the problem: Four default controls are necessary in Gnome-Nibbles : Left, Right, Up and Down, which are provided for by the four arrow keys by default. When I try to change the controls, I can press the same key accidentally for more than one control. In the sense that I can have H control both the upward and rightward movement of the worm. Steps to reproduce: Run Nibbles. Go to Settings->Preferences Select Worm1(Since it's the worm controlled by you) Select the control line saying "Right" and click it. Then press any key you wish to use, say H Select the control line saying "Up" and click it. Then press H again. Actual Results: *No error is thrown while changing the preferences. *While playing the game, you can't move Up->Right and Right->Up. Interestingly, Down->Right and Left->Up are possible since Down->Up and Left->Right are impossible turns in the game. Expected results: *An alert can be sounded while choosing custom keys if two controls have been assigned the same keys. Does this happen every time? Yes.
I tried to compile last version from git repository in Ubuntu 12.04 and i had same problems.
Created attachment 242691 [details] [review] Patch for checking key duplicity in settings This patch solves the bug, but it would be proper to change default keys for worms 1-5, because now are all keys same so it may cause some problems.
Hi, is the patch ok, or there is a problem?
Review of attachment 242691 [details] [review]: Jakub, sorry for the delay in reviewing this. The patch is mostly good. A few considerations: * For function calls, there should be a space between the name of the function and the opening parentheses, as in surrounding code. * Your patch adds trailing whitespace, which should be removed. It also uses hard tabs, but gnibbles uses only spaces. * Why don't you remove the call to gtk_window_set_title; that doesn't look very good. Message dialogs don't benefit from titles. * Your error message works fine for programmers, but it isn't very user-friendly for normal humans. How about "The shortcut you selected is already assigned" * The text on the dialog needs to be marked for translation, like _("text here"). * The text should be bigger and bold. You can get this by using gtk_message_dialog_new_with_markup. I'm thinking something like this would work: dialog = gtk_message_dialog_new_with_markup (GTK_WINDOW(window), GTK_DIALOG_DESTROY_WITH_PARENT, GTK_MESSAGE_WARNING, GTK_BUTTONS_OK, "<span weight=\"bold\" size=\"larger\">%s</span>", _("The shortcut you selected is already assigned.")); If you're fast, then we can sneak this in before string freeze (two weeks away, sorry again for the delay) then this can go into 3.10.
Created attachment 252482 [details] [review] Patch for checking key duplicity in settings Thank you for your reply, I have added suggested changes and here is new patch. But still, different default keys for all worms should be set, because now are defaults for all worms same, so when you try for example change RIGHT to LEFT for worm1, that will show the error, because all other worms have RIGHT and LEFT keys assigned.
Review of attachment 252482 [details] [review]: Good except for whitespace; line up the characters in a function call like this: dialog = gtk_message_dialog_new_with_markup (GTK_WINDOW(window), GTK_DIALOG_DESTROY_WITH_PARENT, GTK_MESSAGE_WARNING, GTK_BUTTONS_OK, "<span weight=\"bold\" size=\"larger\">%s</span>", _("The shortcut you selected is already assigned.")); (I know you copypasted it from me above but I assumed you would space it out.) Erm, you're right that we need different default controls for the other worms before this can be committed. I have no idea how to come up with reasonable defaults for six players, though. I also have no idea how six players per keyboard could possibly be considered reasonable. Maybe it would make the most sense to cut the number of human players down to three at most?
Created attachment 252609 [details] [review] Patch for checking key duplicity in settings Hi, I am sorry for problems. I added whitespaces as you recommended. The number of worms I would preserve, I can imagine six children playing it.
Review of attachment 252609 [details] [review]: Looks good to me. We still need to figure out something for default shortcuts, as discussed above, before this can be pushed.
Hey Bryan, any ideas on how to handle this elegantly? I'm somewhat at a loss.
Well. not elegantly. I was considering removing some of the possible players actually to just 2. >The number of worms I would preserve, I can imagine six children playing it. I can imagine 2 per keyboard/screen. I can definitely imagine 6 children playing it on 3 computers networked (when we get networking). How would it work for 4 players/6 players and one keyboard screen? >This patch solves the bug, but it would be proper to change default keys for > worms 1-5, because now are all keys same so it may cause some problems. As part of looking at another bug I found that the key presses, can occasionally trigger on the wrong human controlled worm. Defaulting to the same keys seems quite bad.. So, I'm thinking leave player 1 alone, but make player 2 default to WASD. I unfortunately have no idea how to handle different keyboard layouts though...
(In reply to comment #10) > Well. not elegantly. I was considering removing some of the possible players > actually to just 2. This is likely the best possible approach. > So, I'm thinking leave player 1 alone, but make player 2 default to WASD. I > unfortunately have no idea how to handle different keyboard layouts though... I don't think you should try; that'd be a lot of effort to put into such a simple matter as choosing a default setting. I think it's reasonable to expect people to change these hotkeys if they don't work well. (I'm a happy Dvorak user myself, and switching some hotkeys around is a price I accept.) Using WASD as the default for Worm 2 is miles better than using the same keys that Worm 1 does. http://www.happyassassin.net/2013/05/26/more-debugging/ gets into some detail on how keyboard layouts are handled, not that I understand much of it. It's not just a matter of "what character maps to what key" but also "what physical keys exist on the keyboard."
I committed the key duplicity patch in https://git.gnome.org/browse/gnome-nibbles/commit/?id=7bbef5fef0975826a5831c53cf57290279dfd564 Thanks for the patch Jakub Motyčka. I'm still working on the better default keys part.
So it looks like this bug is solved; let's move the issue of poor defaults to bug #735134