Attnam / ivan

Iter Vehemens ad Necem - a continuation of the graphical roguelike by members of http://attnam.com
GNU General Public License v2.0
303 stars 42 forks source link

'g'o command does not work with numpad #40

Closed andrewtweber closed 7 years ago

andrewtweber commented 9 years ago

On IVAN 0.50.3 Windows, when I press 'g' to go and then the arrow key to go in the direction, it simply moves the player 1 tile

ryfactor commented 9 years ago

It's true. Try minimising the window, and then maximizing again? The problem goes away. But then it comes back if I chat to npc's or do something else. Odd.

emlai commented 9 years ago

Weird. Is this with SDL2 or 1? And which version specifically?

ryfactor commented 9 years ago

SDL2, from this windows build: https://github.com/Attnam/ivan/releases/tag/v0503

andrewtweber commented 9 years ago

Yes, SDL2. For me the minimize/maximize trick doesn't work either. It happens in both fullscreen and windowed.

murlock commented 9 years ago

I've tested with my Linux version, it works with my azerty keyboard, I still have to download 'official' 0.50.3 to test it. I didn't test fullscreen / windowed mode.

emlai commented 9 years ago

Unfortunately I can't reproduce this on OS X, not even when running the Windows executable through Wineskin. What flags was that executable compiled with?

ryfactor commented 9 years ago

Mock me if you will, (and you probably will) but I use this fossil for compiling: mingw gcc version 3.4.2 Using mingw32-make.exe I invoke -B -f c:\ivan\ivanmgw-local.mak

The makefile looks like the one below.

CC = g++ -o

FeLibDIR = C:\ivan\FeLib

FeLibGCH =

FeLibOBJ = $(FeLibDIR)/Source/bitmap.o $(FeLibDIR)/Source/config.o $(FeLibDIR)/Source/error.o $(FeLibDIR)/Source/feio.o $(FeLibDIR)/Source/felist.o $(FeLibDIR)/Source/femain.o $(FeLibDIR)/Source/femath.o $(FeLibDIR)/Source/festring.o $(FeLibDIR)/Source/fetime.o $(FeLibDIR)/Source/graphics.o $(FeLibDIR)/Source/hscore.o $(FeLibDIR)/Source/rawbit.o $(FeLibDIR)/Source/save.o $(FeLibDIR)/Source/whandler.o

SDL2DIR = C:\MinGW\include\SDL2

IVANDIR  = C:\ivan\Main

IVANBIN  = IVAN.exe

IVANGCH =

IVANOBJ  = $(IVANDIR)/Source/actset.o $(IVANDIR)/Source/areaset.o $(IVANDIR)/Source/charset.o $(IVANDIR)/Source/charsset.o $(IVANDIR)/Source/command.o $(IVANDIR)/Source/coreset.o $(IVANDIR)/Source/dataset.o $(IVANDIR)/Source/dungeon.o $(IVANDIR)/Source/game.o $(IVANDIR)/Source/godset.o $(IVANDIR)/Source/iconf.o $(IVANDIR)/Source/id.o $(IVANDIR)/Source/igraph.o $(IVANDIR)/Source/itemset.o $(IVANDIR)/Source/levelset.o $(IVANDIR)/Source/main.o $(IVANDIR)/Source/materset.o $(IVANDIR)/Source/message.o $(IVANDIR)/Source/object.o $(IVANDIR)/Source/roomset.o $(IVANDIR)/Source/script.o $(IVANDIR)/Source/slotset.o $(IVANDIR)/Source/trapset.o $(IVANDIR)/Source/wmapset.o $(IVANDIR)/Source/wskill.o

FLAGS = -DGCC -DUSE_SDL -DWIZARD -IInclude -I$(SDL2DIR) -I$(FeLibDIR)/Include -O0 -ffast-math -s -W -Wall -pedantic -mwindows

LIBS =  -lmingw32 -lSDL2main -lSDL2

all:    $(IVANBIN)

$(FeLibGCH) : %.h.gch : %.h

    @echo Compiling $@...

    @$(CC) $@ -c $< $(FLAGS)

$(FeLibOBJ) : %.o : %.cpp

    @echo Compiling $@...

    @$(CC) $@ -c $< $(FLAGS)

$(IVANGCH) : %.h.gch : %.h

    @echo Compiling $@...

    @$(CC) $@ -c $< $(FLAGS) -I$(IVANDIR)/Include

$(IVANOBJ) : %.o : %.cpp

    @echo Compiling $@...

    @$(CC) $@ -c $< $(FLAGS) -I$(IVANDIR)/Include

$(IVANBIN) : $(FeLibGCH) $(FeLibOBJ) $(IVANGCH) $(IVANOBJ)

    @echo Compiling $(IVANBIN)...

    @$(CC) $(IVANBIN) $(FeLibOBJ) $(IVANOBJ) $(FLAGS) $(LIBS) C:\ivan\Main\Resource\Ivan.res
emlai commented 9 years ago

Can you build it with a different version of SDL2 and see if the problem persists? For example the latest 2.0.4 release candidate, etc.

If it's not a bug in SDL then my guess is that it has something to do with our way of handling the window events, so that maybe one of those events interrupts the 'go' action somehow. Just a guess though.

ryfactor commented 9 years ago

Thanks for the link to the latest SDL release candidate Emil, I was looking for that :)

ryfactor commented 9 years ago

I think digging might be affected as well :(

ryfactor commented 9 years ago

Hey I think I have found a pattern to this bug: The (g)o command works normally if I only use the conventional direction arrows, but as soon as I want to (g)o using the keypad (diagonal directions etc), then it only moves the player character one square. Cap do you get the same behaviour?

ryfactor commented 9 years ago

The problem is with the number lock. Toggle the number lock off, and it should restore your keypad useability for the (g)o function. It seems that DJGPP plays a role in recording the numlock state and toggling it off and back on again: https://github.com/Attnam/ivan/blob/master/Main/Source/main.cpp#L42 and here: https://github.com/Attnam/ivan/blob/master/Main/Source/main.cpp#L121 I don't know why it is broke. Maybe more reason to move to C++11?

andrewtweber commented 9 years ago

Interesting. I'll try it when I get home in several hours. If numlock changes the keycode, is it possible to say something like if (key == "keypad left arrow" || key == "keypad number 4")? That way it works either way?

andrewtweber commented 9 years ago

Hmm, seems like it's fixed in Ivan 0.50.3. I'll re-open if I see it again

andrewtweber commented 8 years ago

It would be nice for this to work with or without the numlock. Since regular navigation seems to work with or without, it should be possible to also make the "go" command work.

I'm trying on OSX right now, and my full Logitech keyboard doesn't let me "go". Unfortunately this keyboard also does not have a numlock button on it... (strangely the Windows version of the exact same keyboard, which I have at home, does have a numlock)

andrewtweber commented 8 years ago

I'm not convinced this is a key input problem. The character movement is handled exactly the same as the 'go' command

https://github.com/Attnam/ivan/blob/master/Main/Source/char.cpp#L2225 https://github.com/Attnam/ivan/blob/master/Main/Source/game.cpp#L1050

andrewtweber commented 8 years ago

Update: digging also does not work. It just digs once and then stops.

serin-delaunay commented 8 years ago

This bug affects 'g'oing, digging, 'C'hatting with tame NPCs, and 'o'pening chests which are piled on the ground. In the latter two cases, the expected menu appears for a split-second before disappearing, which suggests that an extra keypress (something like ESC) is being inserted after the direction key. Using the arrow keys or hjklyubn are two workarounds, but for players with a numpad and no numlock key a fix would be much preferred.

andrewtweber commented 7 years ago

Bumping this as it makes it really difficult for me to test out the new maps.

emlai commented 7 years ago

@andrewtweber Mac keyboards don't have numlock, so the issue might be in the interaction between your Windows keyboard and macOS (and maybe SDL too?). Have you read this?

andrewtweber commented 7 years ago

Sorry, also does not work on my work computer:

I think ideally it should work with the numpad whether the numlock is on or off

emlai commented 7 years ago

Apparently others are having this problem too: https://github.com/JACoders/OpenJK/issues/626

I don't have a keyboard with a numpad to test on right now, but can you see if their solution could work for us?

andrewtweber commented 7 years ago

Thanks but I don't think it will.

The SDL bug they reference says RESOLVED FIXED

Also IVAN's input code doesn't seem to use SDL... uses FeLib instead? And that looks like a library that the IVAN devs wrote. Anyways I couldn't find any reference to SDL's KMOD_NUM in the code

emlai commented 7 years ago

FeLib uses SDL under the hood.

ryfactor commented 7 years ago

Let's review the current behaviour: NumLock is on (LED is on), in IVAN press 'g' for go, player character moves one step. NumLock is off (LED is off), in IVAN press 'g' for go, player character moves continuously in that direction.

Seems like we need a function to suspend the numlock state in a psuedo way...

andrewtweber commented 7 years ago

Not sure if I was unclear, but the numpad just does not work for 'g'o period. Either if numlock is on or off.

'g'o only works with the arrow keys. So I can run in 4 directions but not diagonally (and I would prefer to use the numpad)

ryfactor commented 7 years ago

Do you have a "clear" button on your keypad?

ryfactor commented 7 years ago

This is such a tricky bug :\ I think there are two issues at hand: 1) Keypad smarts for windows 2) Keypad as a directional controller in Mac

I did some fooling around in the code to output some SDL events to a command window and I discovered the following: Case 1: Arrow key case (normal behaviour) press 'g' and you get: SDL_KEYDOWN = (0x300) SDL_TEXTINPUT = (0x303) SDL_KEYUP = (0x301) Then press left arrow and you get: SDL_KEYDOWN = (0x300) SDL_KEYUP = (0x301) The character moves continually to the left until it reaches some obstacle and then stops.

Note the letter 'g' hasSDL_TEXTINPUT = (0x303) but the arrow key does not have this. This is the desired behaviour.

Case 2: Numlock off - keypad acting as a directional controller: press 'g' and you get: SDL_KEYDOWN = (0x300) SDL_TEXTINPUT = (0x303) SDL_KEYUP = (0x301) Then press left arrow on keypad (number 4) and you get: SDL_KEYDOWN = (0x300) SDL_KEYUP = (0x301) The character moves continually to the left until it reaches some obstacle and then stops.

Note that case 2 is consistent with case 1.

Case 3: Numlock on - keypad acting as a number pad: press 'g' and you get: SDL_KEYDOWN = (0x300) SDL_TEXTINPUT = (0x303) SDL_KEYUP = (0x301) Then press left arrow on keypad (number 4) and you get: SDL_KEYDOWN = (0x300) SDL_TEXTINPUT = (0x303) <=== As you would if the key was a normal alphanumeric! SDL_KEYUP = (0x301) The character moves only one space to the left and then stops...

The SDL_TEXTINPUT seems to interrupt the character's activity somehow. It seems like the numlock state needs to be suspended when the player is asked a direction question? Otherwise if numlock is on and the player inputs for example number pad 7, it leads to Case 3 above.

The solution to the Mac problem, however, might require a new keyboard layout MoveMacKeypadCommandKey a la: https://github.com/Attnam/ivan/blob/master/Main/Source/game.cpp#L128 This would be successful only if the Mac keypad codes can be distinguished from the normal numeric keys on the small format keyboard layout.

andrewtweber commented 7 years ago

I do have a "clear" button, and I've never tried using it :( Unfortunately I won't be back at the office until next week but I'll try it then.

Thanks for the research. Let me know if there's anything I can do to help debug. My C++ is rusty but I can follow instructions pretty well if you tell me what to do

ryfactor commented 7 years ago

That's cool, if you can test out the clear button at some point then that would be great. I'm not expecting there to be a breakthough.

I looked here: https://github.com/M-griffin/EtherTerm/issues/13 and found a further link confirming my fears: https://bugzilla.libsdl.org/show_bug.cgi?id=2886

For us, we would need to know the numlock state and then ignore this SDL_TEXTINPUT for directional inputs on the keypad. Don't know if it's possible to do this with the way SDL is set up at the moment.

emlai commented 7 years ago

I just tested this on a Linux machine at the university, and the numpad only works when the "movement control scheme" is set to "normal". It doesn't work with the "alternative" or "NetHack" options. @andrewtweber are you testing with the "normal" control scheme?

andrewtweber commented 7 years ago

are you testing with the "normal" control scheme?

Yes I am

andrewtweber commented 7 years ago

The clear button has no effect

emlai commented 7 years ago

Could we temporarily solve this by doing what they did in https://github.com/JACoders/OpenJK/commit/d0c58aa188c2effd73e96c588dd2767bf9084f6e, i.e. use the native Windows API GetKeyState to check the actual numlock state? I don't have a keyboard with a numlock so I can't test it, but if someone wants to give it a try, go ahead.

ryfactor commented 7 years ago

It would be easy to use the example from JACoders/OpenJK@d0c58aa to map the keypad back to numerical inputs while the numlock state is ON. That would make it at least intuitive in the sense that numlock ON gives you the numbers 1, 2, 3, ... etc, while numlock OFF gives you the directional keys. I think there is some merit in implementing it that way. But we want the keypad to give directional keys no matter what state the numlock is in, right? I think for our case, the problem is complicated by the fact that when numlock is OFF, the keypad behaves like a normal directional keypad, but when numlock is ON both the directional key and the number value is activated.

In-game, try re-naming kenny or reading for example, a scroll of wishing, and use the keypad to enter text into the field, for both cases of the numlock state. The OFF state yields directional codes only, while the ON state yields both the direction and the number code. It is this number code that is treated as an SDL_TEXTINPUT. When using the 'g'o command, the action is interrupted by the additional instance of the SDL_TEXTINPUT, and so the player character stops moving.

ryfactor commented 7 years ago

What if we just use the numbers as directional keys and replace the wizard mode functions with different keys?

emlai commented 7 years ago

@fejoa I like that. IMO the function keys (F1, F2, etc.) would be suitable for this.

andrewtweber commented 7 years ago

Sounds good to me!

ryfactor commented 7 years ago

Can someone kindly test #276 and verify? I didn't end up implementing the numbers as directional keys suggestion, but instead opted to solve the problem at the root cause. It should mean that you can dig omnidirectionally with the number pad again, amongst other joys.

andrewtweber commented 7 years ago

Thanks @fejoa! I tested Go and Dig and both worked!

ryfactor commented 7 years ago

So relieved this bug is gone.