Jump to content

[1.14.4] KeyBinding falsely reporting isPressed after using inventory


kirstym

Recommended Posts

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 by kirstym
fix verb, emphasis
Link to comment
Share on other sites

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.

Link to comment
Share on other sites

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. 

Link to comment
Share on other sites

@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.

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Unfortunately, your content contains terms that we do not allow. Please edit your content to remove the highlighted words below.
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Announcements



×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.