Opened 5 years ago

Closed 3 years ago

#99 closed defect (fixed)

Strange set key behavior

Reported by: HoraK-FDF@… Owned by: Thomas Bernard
Priority: major Milestone: 2.8
Component: GrafX2 Version: 2.6
Keywords: Cc:

Description

Hi,

the version I'm using is grafx2-win32-2.6.2475-win32.zip from the download page.

If i try to set keys it displays a wrong key but the desired key works as intended for example:

if press Up it shows Shift+PgDn, that kind of behavior is true for many other keys:
Left -> Alt+Shift+' '
Right -> Alt+Shift+PgDn

... greetings HoraK-FDF

Change History (14)

comment:1 by Thomas Bernard, 5 years ago

thanks for the report, I will check that.

comment:2 by Thomas Bernard, 5 years ago

Owner: changed from pulkomandy to Thomas Bernard
Status: newaccepted

comment:3 by HoraK-FDF@…, 5 years ago

It does not happen with grafx2-sdl-2.6.2475-win32.zip

comment:4 by Thomas Bernard, 5 years ago

OK I think that is because we use MOD_SHIFT/MOD_ALT_/MOD_CTRL which are defined in WinUser.h :

#define MOD_ALT         0x0001
#define MOD_CONTROL     0x0002
#define MOD_SHIFT       0x0004

it conflicts with our own definitions in global.h

Up has win32 keycode 0x26 which is 0x4 (SHIFT) + 0x22 (Page Down)

comment:6 by Thomas Bernard, 5 years ago

safe and long term solution is to rename MOD_SHIFT, etc. to a private name such as GFX2_MOD_SHIFT, etc.

comment:7 by yrizoud@…, 5 years ago

<windows.h> pollutes the global namespace a lot, I think it would be better to minimize the number of .C files that include it.

comment:8 by Thomas Bernard, 5 years ago

@yrizoud: here is what I propose to do :

  1. merge https://gitlab.com/GrafX2/grafX2/merge_requests/168 as a quick fix for v2.6
  1. wait v2.7wip to apply the patch https://gitlab.com/miniupnp/grafX2/commit/a29e2c0bda7a60fcd85091c722ef5f181d567de3

which rename MOD_* to GFX2_MOD_* so there is no more clash

comment:9 by yrizoud@…, 5 years ago

Yes I agree with both changes as a short- and long-term fix for these modifiers. But I still think that (microsoft's) windows.h is going to collide with more stuff in the future.
The current coding conventions helps avoid a lot of conflicts (T_Struct for most structs, capitalized Function() etc. but (from a quick look at MS docs in search of examples) you can easily collide common names such as #define INPUT (reserved by a typdefed struct) or if you try to call a parameter or local variable "mouse_event" (reserved by an obsolete function)

comment:10 by PulkoMandy, 5 years ago

For Haiku I moved all the platform specific code to haiku.cpp as it is written in C++.
I would recommend doing the same for other platform specific parts: define our API, and move its implementation to a specific file. This would avoid including windows.h anywhere outside this specific file.

comment:11 by Thomas Bernard, 5 years ago

I have created a specific ticket for <windows.h> name pollution :
http://pulkomandy.tk/projects/GrafX2/ticket/105

comment:13 by PulkoMandy, 4 years ago

Milestone: 2.8

comment:14 by Thomas Bernard, 3 years ago

Resolution: fixed
Status: acceptedclosed

I think we can close now, #105 is closed.

Note: See TracTickets for help on using tickets.