bugBattle for Wesnoth - Bugs: bug #20251, Non-letters not working as hotkeys

 
 
Show feedback again

bug #20251: Non-letters not working as hotkeys

Submitted by:  J Tyne <jamit>
Submitted on:  Sat 20 Oct 2012 01:17:17 AM UTC  
 
Category: BugSeverity: 5 - Blocker
Priority: 5 - NormalItem Group: User Interface
Status: FixedPrivacy: Public
Assigned to: Fabian Müller <fendrin>Open/Closed: Closed
Release: 1.11.0+svn (r55555)Operating System: Linux

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Tue 27 Nov 2012 12:57:55 PM UTC, comment #7:

Ctrl+space doesn't work in 1.10.5 either so I think I can consider it a bug unrelated to the issue originally reported.

'=' as a hotkey seems to be a display issue, not a functional one, since it works but isn't displayed properly in the dialog.

Simon Forsyth <alarantalara>
Project Member
Tue 27 Nov 2012 04:07:58 AM UTC, comment #6:

It's a lot better than it was, but there are still some minor issues.

Setting CTRL+SPACE as a hotkey still doesn't work, and some hotkey combinations aren't saving and restoring properly. For example, CTRL-` won't work after a restart.

CTRL-4 works, but looks very strange.

The '=' key can't be used as a hotkey by itself or with CTRL.

This issue should no longer be a severity 5 blocker.

Galen Brooks <fluffbeast>
Sun 28 Oct 2012 10:19:51 PM UTC, comment #5:

Attached is a diff showing a fix for this problem.

Control characters are in the range 0-31, but any character < 64 was getting the full remap treatment.

By changing isprint to isalnum, the display of ctrl+3 and other numbers now works as expected.

Setting character to -1 when isalnum is false fixes the space key, and makes all the other non-character keys to be processed in the same way.

There is still an issue with the '=' key. This problem exists regardless of whether ctrl or alt are used. The reason for this problem is that '=' is COLUMN_SEPARATOR, and does not display correctly.

(file #16722)

Galen Brooks <fluffbeast>
Sat 27 Oct 2012 12:10:24 PM UTC, SVN revision 55614:

Fixing the bugs:
bug #20252
bug #20251
bug #20249

(Browse SVN revision 55614)

Fabian Müller <fendrin>
Project MemberIn charge of this item.
Sun 21 Oct 2012 05:27:54 PM UTC, comment #3:

The hotkey definitions aren't being saved. There is an inverted null check in the save() method.

svn diff src/hotkeys.cpp
Index: src/hotkeys.cpp
===================================================================
--- src/hotkeys.cpp (revision 55567)
+++ src/hotkeys.cpp (working copy)
@@ -569,12 +565,10 @@
for(std::vector<hotkey_item>::iterator i = hotkeys_.begin();
i != hotkeys_.end(); ++i)
{
- if (i->get_command() != "null") {
+ if (i->null()) {
continue;
}

config& item = cfg.add_child("hotkey");
i->save(item);
}
}

Galen Brooks <fluffbeast>
Sun 21 Oct 2012 04:40:29 PM UTC, comment #2:

The patch does not fix numbers or other keys in combination with ctrl. Perhaps a more general solution involves using keycodes whenever the ctrl modifier is present, and removing the ascii conversion math. A quick test shows this approach is workable.

There is another problem: the saved hotkeys all have a null command.

Galen Brooks <fluffbeast>
Sun 21 Oct 2012 01:59:47 PM UTC, comment #1:

I noticed this problem too, and did some digging. The problem is that there is some confusion when to use the character typed, and when to use the keycode. I have a patch that fixes this problem.

svn diff src/hotkeys.cpp
Index: src/hotkeys.cpp
===================================================================
--- src/hotkeys.cpp (revision 55565)
+++ src/hotkeys.cpp (working copy)
@@ -468,7 +468,7 @@
<< "\n";

// Sometimes control modifies by -64, ie ^A == 1.
- if (character < 64 && ctrl) {
+ if (0 < character && character < 64 && ctrl && !isspace(character)) {
if (shift) {
character += 64; }
else {
@@ -652,7 +652,7 @@
<< "\n";

// Sometimes control modifies by -64, ie ^A == 1.
- if (0 < character && character < 64 && ctrl) {
+ if (0 < character && character < 64 && ctrl && !isspace(character)) {
if (shift) {
character += 64;
} else {
@@ -666,6 +666,13 @@
if (cmd && character > 96 && character < 123 && shift) {
character -= 32; }

+ if (isprint(character) && !isspace(character)) {
+ LOG_G << "type = BY_CHARACTER\n";
+ } else {
+ character = -1;
+ LOG_G << "type = BY_KEYCODE\n";
+ }
+
bool found = false;

for (itor = hotkeys_.begin(); itor != hotkeys_.end(); ++itor) {

Galen Brooks <fluffbeast>
Sat 20 Oct 2012 01:17:17 AM UTC, original submission:

The three predefined hotkeys space (end unit turn), shift+space (hold position), and ctrl+space (end turn) are no longer working for me. I hit them, and nothing happens.

I went into the preferences to make sure they were still set, and discovered that I can assign space or shift+space to any number of hotkeys (the old assignments are not replaced). The same sort of holds for ctrl+space, but that key combination gets displayed as ctrl+` instead of ctrl+space.

Further investigation reveals similar behavior for other non-letters. (Numbers are somewhat better behaved, but still go crazy when combined with ctrl.)

J Tyne <jamit>
Project Member

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #16722:  hotkeys.diff added by fluffbeast (1kB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by alarantalara (Updated the item)
  • -unavailable- added by fendrin (Updated the item)
  • -unavailable- added by fluffbeast (Posted a comment)
  • -unavailable- added by jamit (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 09 Dec 2012 09:40:27 AM UTCivanovicOpen/ClosedOpen=>Closed
    Mon 26 Nov 2012 09:16:22 PM UTCalarantalaraStatusIn Progress=>Fixed
    Sun 28 Oct 2012 10:19:51 PM UTCfluffbeastAttached File-=>Added hotkeys.diff, #16722
    Fri 26 Oct 2012 08:54:38 PM UTCjamitSeverity3 - Normal=>5 - Blocker
    Fri 26 Oct 2012 10:19:04 AM UTCfendrinStatusNone=>In Progress
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup