Breaks toggle sprint

[srnyx]

Current Status: #7 (comment)

---

Version
1.16.5, 1.6.4
Describe the bug
When using the mod, the Minecraft toggle sprint doesn't work

To Reproduce

1. Install this mod and RandomPatches
2. Turn on toggle sprint/sneak
3. Try it out

Expected behavior
Toggle sprint should work fine

Log
There aren't really any logs for this

Additional context
Modpack: https://www.curseforge.com/minecraft/modpacks/simpearth/files/3209728

[Fourmisain]

I was trying to find the cause for this and it appears to be a conflict with RandomPatches!

Either mod on its own is fine, just the combination is not.

A temporary workaround is to set
mixin_blacklist = ["KeyBinding"]
insinde randompatches.toml, which suggests that this mixin is at least partly to blame.

My guess is that it's this piece of code since everything else seems to be a NOP with default settings:
https://github.com/TheRandomLabs/RandomPatches/blob/291a49bf05ec37312680287202e5d6b65eeb1d73/src/main/java/com/therandomlabs/randompatches/mixin/client/keybindings/KeyBindingMixin.java#L108-L114

And recompiling without it does indeed eliminate the conflict!

I'm not sure however what part of Mouse Wheelie is conflicting.
I'm guessing it may be SortKeyBinding, since that can use a shift modifier and given the name PriorityKeyBinding, maybe that blocks every other key binding from executing - or something like that?

[Fourmisain]

Oh, just noticed, I was talking about Toggle Sneak the whole time while the original issue was about Toggle Sprint, so that means it's not just shift but also control which is affected - which again matches SortKeyBinding.

[Fourmisain]

It seems SortKeyBinding was a red hering, kinda.

Turns out it's not about Mouse Wheelie, it's about amecs!
Probably related to Siphalor/amecs#31.

[Fourmisain]

Hm... looks like this is not fixed, I built mouse-wheelie from source, but the conflict with RandomPatches is still there.
Tried using only amecs (after updating the build, because it wouldn't work for me), wouldn't budge.
Tried only amecs-api and it still conflicted, so I have to update my statement: it's about amecs-api!

Comparing

amecs-api/src/main/java/de/siphalor/amecs/impl/mixin/MixinKeyBinding.java

Lines 121 to 124 in b5f72b1

	@Inject(method = "setKeyPressed", at = @At("HEAD"))
	private static void setKeyPressed(InputUtil.KeyCode keyCode, boolean pressed, CallbackInfo callbackInfo) {
	KeyBindingManager.setKeyPressed(keyCode, pressed);
	}

with
https://github.com/TheRandomLabs/RandomPatches/blob/291a49bf05ec37312680287202e5d6b65eeb1d73/src/main/java/com/therandomlabs/randompatches/mixin/client/keybindings/KeyBindingMixin.java#L108-L114
it looks like KeyBindingManager.setKeyPressed(keyCode, pressed); might not execute because of RandomPatches' info.cancel(); (depending on mixin load order).

[Siphalor]

Ok, I think the main thing is that Siphalor/amecs#31 is not actually related to this. (That issue is resolved now and therefore I also closed this one).

I'm gonna see what I can do about the RandomPatches compat.

[Fourmisain]

it looks like KeyBindingManager.setKeyPressed(keyCode, pressed); might not execute because of RandomPatches' info.cancel(); (depending on mixin load order).

Just added a println in amecs-api and it does execute. This could still cause issues, but it's not the reason for the conflict.

[Siphalor]

Wild guess without looking at the code too much: Since both mods manually call setPressed on all matching keybindings maybe it double-toggles and does nothing?

[Fourmisain]

That makes a lot of sense!

It actually looks like amecs-api and RP are trying to do the exact same thing: executing every keybinding with the key that was just pressed.
RP: .forEach(keyBinding -> keyBinding.setPressed(pressed));
amec-api (setKeyPressed): getMatchingKeyBindings(keyCode).forEach(keyBinding -> keyBinding.setPressed(pressed));

It should probably be RP which disables the mixin when amecs-api is present - the way it's done (@Inject + info.cancel) is actually no better than an @Overwrite.

I wonder tough, since amecs-api does not cancel the vanilla call, doesn't that mean that one keybinding will execute twice?

[Fourmisain]

Hm... or does it make sense?
It's setting if the key is pressed, it just assigns a boolean value...

[Fourmisain]

AmecsKeyBinding does a little more than just that:

amecs-api/src/main/java/de/siphalor/amecs/api/AmecsKeyBinding.java

Lines 35 to 38 in b5f72b1

	if (pressed)
	onPressed();
	else
	onReleased();

But I'm not sure how AmecsKeyBinding is used in the context of sneaking.

[Fourmisain]

Aha, it looks like sprinting uses a StickyKeyBinding and that does toggle the internal state via setPressed!

[Siphalor]

Yea, that's what I feared.
I think I'm gonna mixin to StickyKeyBinding and try to fix it there :)

[Fourmisain]

I think there's a really simple fix: adding an info.cancel() in amec-api's setKeyPressed mixin.

That'll ensure setPressed will only be executed once, either in RP or in amecs-api - and it'll also fix a potential issue with the vanilla method calling setPressed again on 1 keybinding.

Only thing is that I'm not 100% sure if RP's and amces-api's code is equivalent.
If not, it really should be RP which just doesn't execute it's method mixin, via something like
if (FabricLoader.getInstance().isModLoaded("amecsapi")) return;

[Siphalor]

The code is not equivalent as Amecs handles a bunch of other things concerning the modifiers in that method as well.

Yea, you're right that should probably be in RP.

[Fourmisain]

The code is not equivalent as Amecs handles a bunch of other things concerning the modifiers in that method as well.

In that case, it definitively needs to be in RP, because when the mixin load order is different, amecs-api's code won't execute at all.

I'd write a PR, but I'm currently having a lot of trouble with gradle and IDEA, building RP randomly starts failing, the project won't open with the message .editorconfig (Access denied) which I can't find anything about and all the standard tricks don't help fixing it...

[Siphalor]

Just a small status update: This issue and the associated one at Random Patches are waiting for the pull request in Random Patches to get approved and merged.
Current workaround would be to disable the keybinding mixin in the Random Patches config.