[1.14.4] KeyBinding falsely reporting isPressed after using inventory

[kirstym]

I've been using the following pattern to react to key presses in my mods:

@SubscribeEvent
public void onKeyInput(KeyInputEvent event) {
 
	if (myKeyBinding.isPressed()) {
		LOGGER.debug("key has been pressed!");
		// Do stuff here
	}
}

 

This generally works as expected - isPressed() returns true on the initial event (where event.action = GLFW_PRESS). The docstring for isPressed() says "Returns true on the initial key press. For continuous querying use {@link isKeyDown()}. Should be used in key events." which makes me think it's a reasonable approach.

 

However, I've noticed that some of my key events were being unexpectedly fired upon closing an inventory GUI. I've stripped it down to a simple example, and what I'm seeing is that if the key in question was pressed at any point while the inventory was open, then isPressed() continues to report true for a short time after. It's not firing a new event for the key in question, but it means my mod code gets triggered for any other key event (including the GLFW_RELEASE event of the Escape key from closing the inventory). This happens on both creative and survival inventories, but not the IngameMenuScreen or AdvancementsScreen.

 

This standalone example that shows I'm not doing anything funny elsewhere in my code:

 

Spoiler

package com.example.examplemod;

import net.minecraft.client.Minecraft;
import net.minecraft.client.settings.KeyBinding;
import net.minecraftforge.client.event.GuiOpenEvent;
import net.minecraftforge.client.event.InputEvent.KeyInputEvent;
import net.minecraftforge.common.MinecraftForge;
import net.minecraftforge.eventbus.api.SubscribeEvent;
import net.minecraftforge.fml.client.registry.ClientRegistry;
import net.minecraftforge.fml.common.Mod;
import net.minecraftforge.fml.event.lifecycle.FMLCommonSetupEvent;
import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.lwjgl.glfw.GLFW;

// The value here should match an entry in the META-INF/mods.toml file
@Mod("examplemod")
public class ExampleMod
{
    // Directly reference a log4j logger.
    private static final Logger LOGGER = LogManager.getLogger();

    private static KeyBinding myKeyBinding;
    private final int myKeyCode = GLFW.GLFW_KEY_P;
	
    public ExampleMod() {
        // Register the setup method for modloading
        FMLJavaModLoadingContext.get().getModEventBus().addListener(this::setup);

        // Register ourselves for server and other game events we are interested in
        MinecraftForge.EVENT_BUS.register(this);
    }

    private void setup(final FMLCommonSetupEvent event)
    {
        // Register key bindings
    	myKeyBinding = new KeyBinding("example", myKeyCode, "mod keys");
     	ClientRegistry.registerKeyBinding(myKeyBinding);
    }

	@SubscribeEvent
	public void onKeyInput(KeyInputEvent event) {

		// Ignore if GUI is open (e.g. an inventory)
		if (Minecraft.getInstance().currentScreen != null ) { return; }
		
		// Is our registered key binding pressed?
		if (myKeyBinding.isPressed()) {
			LOGGER.debug("my key is pressed!");
			if (event.getKey() != myKeyCode) {
				// If you're only ever pressing one key at a time, you shouldn't ever get here... (but you do!)
				LOGGER.error("unexpected pressed state!");
			}
		}
	}
		
	@SubscribeEvent
	public void onGuiOpen(GuiOpenEvent event) {
		if (event.getGui() == null) {
			LOGGER.debug("GUI closed");
		}
		else {
			LOGGER.debug("GUI opened");
		}
	}
}

 

 

Is this expected behaviour or a bug in the KeyBinding code? Is it fundamentally a broken pattern? From looking at the KeyBinding source, I don't really get the logic behind isPressed. I'm quite happy to bin this pattern and revert to the following one, which always works - but if it's a bug I wanted to document it somewhere, or at least save someone else the debugging time!

 

@SubscribeEvent
public void onKeyInput(KeyInputEvent event) {
 
	if (event.getKey() == myKeyBinding.getKey().getKeyCode() &&
			event.getAction() == GLFW.GLFW_PRESS) {
			// do stuff
	}
}

 

Edited March 6, 2020 by kirstym
fix verb, emphasis

[xSlart01x]

Hi, i've solved some issues by adding this to the if statement:

Minecraft.getInstance().currentScreen == null

Maybe could help

Edited March 5, 2020 by xSlart01x

[kirstym]

I actually do usually use that myself to return early if a GUI is open   - I just left it out to keep the examples really simple. Thanks for the suggestion though!

 

This doesn’t change the behaviour I described, because the bug is happening in any Key event shortly after closing the gui.

[kirstym]

To clarify: 

if my key binding is “P”, then when I press “P” with the inventory open, events immediately fire with the key code for “P”. This is expected. Then, I close the inventory and press “O” (another unrelated key). An event fires with the key code for “O”, but inside the event handler, the isPressed method for the “P” key binding wrongly returns true. 

[DragonITA]

@kirstym you should see this: https://github.com/DaRealTurtyWurty/1.15-Tut-Mod/blob/master/src/main/java/com/turtywurty/tutorialmod/util/helpers/KeyboardHelper.java. Then the next time you need help then pls show your Code on Github or paste the full class on Forge, but not one part.

[kirstym]

@DragonITA sorry, I’m not sure what I’m supposed to be looking at - are you just showing an example of querying key state? Should I look in the whole repo or just the file you linked to?
 

I gave a full mod in the spoiler in the first post. I just stripped it down so that it demonstrates the bug without any extra logic from my mods. It’s a fully functional mod class.