MoveKey gets bugged

Dec 28 2015, 8:52 am In response to GinjaNinja32
Ter13	GinjaNinja32 wrote: AFAICT caused by one-verb-per-tick throttling dropping the key-down verb calls. Just a mistake I noticed in variants of my code. Give your key detection verbs set instant = 1 That will prevent the one-verb-per-tick throttling. Not sure if something else is at play, though. I suspect it is.

Dec 28 2015, 8:55 am In response to Ter13
GinjaNinja32	Mentioned instant=1 in http://www.byond.com/forum/?post=2006801#comment17888059 ; it entirely fixed this problem for me, AFAICT it's just the verb throttling that's doing this.

Dec 28 2015, 9:00 am
Zasif	wait wait wait wait so show me where to fix my issue xD

Dec 28 2015, 9:01 am
Ter13	Alright, you know about proc/verb settings, right? Add "set instant = 1" below the line in MoveKey() that says "set hidden = 1". That should at least help alleviate the issue a litle.

Dec 28 2015, 9:09 am
Zasif	Just to be clear for Lummo, this is still a bug and needs addressed. We don't want workarounds.

Dec 28 2015, 9:14 am

Ter13

I agree that something is going on, we just need to get Lummox a clearer picture of what's happening and how to reproduce it so he can find the problem.

@Zasif: When you mentioned that I tend to get better results from Lummox than you via PM, it's not because Lummox is more willing to listen to me --It's just that I speak his language and I try my damndest to reduce bugs to very reproducible and very obvious test cases and then poke at them to find workarounds that also clue Lummox into a very narrow bit of the engine's code to sift through.

You've just gotta speak the language. =D

Dec 28 2015, 9:52 am
Zasif	This instant = 1 didnt fix it the bug still happens.

Dec 28 2015, 9:30 pm

In response to Zasif

Tens of DU

Zasif wrote:

wait wait wait wait so show me where to fix my issue xD

Zasif your issue is separate. Not related to set instant=1. But if you have KeyUp and KeyDown type verbs for your macros, those verbs should be set to instant=1.

The KeyUp events being lost if window focus changes between KeyDown and KeyUp is another issue. In Dragon Universe it only really happens if a alert() or input() prompt appears between the KeyDown and KeyUp.

509.1303: "Key-up macros didn't fire if the message was not received by the right window and there was no associated key-down macro with the repeat flag. (Ter13)"

I actually think that bug was indeed fixed for a while, but now in the latest 509 version it seems to be doing it again.

Dec 30 2015, 7:15 am
Zasif	bump!

Dec 30 2015, 7:19 am In response to Zasif
Lummox JR	Zasif wrote: bump! Dude, you posted this issue during the winter break. It has not been forgotten and is on my list, but I'm not going to be running tests on it or looking into it at all until next week. Short version: Don't bump a thread that's on the front page of its forum and is less than a week old.

Dec 30 2015, 8:09 am

In response to Lummox JR

Tens of DU

Lummox JR wrote:

Zasif wrote:

bump!

Dude, you posted this issue during the winter break. It has not been forgotten and is on my list, but I'm not going to be running tests on it or looking into it at all until next week.

Short version: Don't bump a thread that's on the front page of its forum and is less than a week old.

Don't worry about it you're doing a great job. There's a TON of requests lately, it's no wonder you can't get to them all.

Dec 31 2015, 3:20 am

Ter13

I found the problem. This may be a bug. It seems that sometimes a keyup can be triggered when a keydown has not happened. My MoveKey verb doesn't have protection for this. The key_state tracker can sometimes go negative, which will result in the input key no longer triggering. A quick fix should prevent this:

The extra keyUp call may be a bug, I don't know. I can't really reliably reproduce this.

client
    verb
        //called when a movement key has been pressed.
        MoveKey(dir as num,state as num)
            set hidden = 1
            var/opp = turn(dir,180)
            var/pos = log(2,dir)+1
            if(state&&++key_state[pos]==1)
                //key track of the keytap state
                if(world.time-key_time[pos]>TAP_THRESHOLD)
                    key_taps[pos] = 1
                else if(last_key==pos)
                    ++key_taps[pos]
                else
                    key_taps[pos] = 1
                last_key = pos

                //keep track of the move keys and direction
                move_keys |= dir
                if(move_keys&opp)
                    move_dir &= ~opp
                else
                    move_dir |= dir
                //store the time of the event
                key_time[pos] = world.time
                //this is an example of handling double-taps for movement
                /*if(move_keys==dir&&key_taps[pos]==2)
                    mob.Dash(dir,0,pos)*/
            else if(!state&&--key_state[pos]==0)
                //this is a key release
                move_keys &= ~dir
                if(move_keys&opp)
                    move_dir |= opp
                else
                    move_dir &= ~dir
                //store the time of the event
                key_time[pos] = world.time
            if(key_state[pos]<0)
                key_state[pos] = 0
            //attempt to call MoveLoop if there are keys being held down
            if(move_keys)
                MoveLoop()

Jan 1 2016, 1:07 pm
Zasif	Don't know if this is a byond bug or a code issue. If you move in any direction and then popup an alert window the movement will get bugged and move on that direction. Only relog fixes it.

Jan 2 2016, 4:48 am

In response to Zasif

Tens of DU

Zasif wrote:

Don't know if this is a byond bug or a code issue. If you move in any direction and then popup an alert window the movement will get bugged and move on that direction. Only relog fixes it.

Yea I've reported this bug too

509.1303: "Key-up macros didn't fire if the message was not received by the right window and there was no associated key-down macro with the repeat flag. (Ter13)"

I think that means they fixed it in 509.1303. Except really it's not fixed. Maybe Lummox didn't take into account input() and alert() popups.

Jan 4 2016, 8:40 am
Lummox JR	So far I can confirm on the MoveKey project submitted by GinjaNinja32 that there is indeed some kind of premature release going on, as Ter13 noted. I'm looking into the code now to see if I can find the cause.

Jan 4 2016, 8:52 am
Ter13	So far I can confirm on the MoveKey project submitted by GinjaNinja32 that there is indeed some kind of premature release going on, as Ter13 noted. I'm looking into the code now to see if I can find the cause. It seems that there are cases where the default input/alert windows get focus where input states are not properly reported. I can't quite get a consistent reproduction of this, though.

Jan 4 2016, 9:41 am (Edited on Jan 4 2016, 9:49 am)

Lummox JR

Well, I just found an interesting result. The test I ran showed that the unmatched key-up was actually not unmatched at all. The key-down that went with it was registered properly on the client end, but somehow that did not send through as a command.

Looking into this further, I found that the reason for this behavior was that, in the past, we had a built-in limit on repeating commands to avoid problems from key repeats being processed faster than the commands could be sent. The correct solution here, I think, is to modify the command processing so that non-repeat macros are specially marked and thus avoid this behavior altogether.

My investigation also explained why the problem went away with instant=1: That forced the command to queue. I should add that with a movement system of this type, instant=1 is required for best results; otherwise no two key-downs/key-ups can be processed on the same tick. So while there is a bug in play here--namely that non-repeat macros can get lost rather than queuing up--the larger issue is that the "workaround" is actually the correct implementation and failing to add instant=1 is a code problem. The only way to encounter this bug with correct code would be a case where a non-repeat macro might be expected to be pressed quickly (e.g., hitting a key to upgrade a tower in a tower defense game).

[edit]
I'll mark this as fixed once I decide what release it's going in. It may be eligible for a maintenance release if I do one.

Jan 4 2016, 9:43 am

In response to Ter13

Lummox JR

Ter13 wrote:

So far I can confirm on the MoveKey project submitted by GinjaNinja32 that there is indeed some kind of premature release going on, as Ter13 noted. I'm looking into the code now to see if I can find the cause.

It seems that there are cases where the default input/alert windows get focus where input states are not properly reported. I can't quite get a consistent reproduction of this, though.

Zasif mentioned the input/alert thing as well, but I need a test project that demonstrates the issue. He had a project up and running the other day, but I was pretty explicit that 1) I really do need to run the project myself, not join it live, and 2) it was Saturday.

The input/alert thing though is really a separate issue. This one has a completely different cause. (At least I think it is; I noticed in further testing of this fix that key releases can be affected the same way as presses, just not as often.)

Jan 4 2016, 10:31 am

In response to Lummox JR

Tens of DU

Lummox JR wrote:

Well, I just found an interesting result. The test I ran showed that the unmatched key-up was actually not unmatched at all. The key-down that went with it was registered properly on the client end, but somehow that did not send through as a command.

Looking into this further, I found that the reason for this behavior was that, in the past, we had a built-in limit on repeating commands to avoid problems from key repeats being processed faster than the commands could be sent. The correct solution here, I think, is to modify the command processing so that non-repeat macros are specially marked and thus avoid this behavior altogether.

My investigation also explained why the problem went away with instant=1: That forced the command to queue. I should add that with a movement system of this type, instant=1 is required for best results; otherwise no two key-downs/key-ups can be processed on the same tick. So while there is a bug in play here--namely that non-repeat macros can get lost rather than queuing up--the larger issue is that the "workaround" is actually the correct implementation and failing to add instant=1 is a code problem. The only way to encounter this bug with correct code would be a case where a non-repeat macro might be expected to be pressed quickly (e.g., hitting a key to upgrade a tower in a tower defense game).

[edit]
I'll mark this as fixed once I decide what release it's going in. It may be eligible for a maintenance release if I do one.

Lummox the bug where input/alert causes KeyUp events to be lost happens even with "set instant=1"

Jan 4 2016, 10:37 am

Ter13

Zasif mentioned the input/alert thing as well, but I need a test project that demonstrates the issue. He had a project up and running the other day, but I was pretty explicit that 1) I really do need to run the project myself, not join it live, and 2) it was Saturday.

The bug is much easier to reproduce across a network, but yeah, I explained the same thing to him. I'll be implementing a similar input control scheme into my project in a few days, so when I've got that done, I'll see if I can simplify it down to the brass tacks with explicit steps. Might be about a week before I can put that all together, though.