Have You Perhaps Heard About "Communication"

[fzzyhmstrs]

About 1 millimeter from reporting you to CF for the actually invasive practice of willfully disabling another dev's mod instead of reaching out to them to resolve a (legitimate) issue.

Curious why you think doing that is the best course of action over:

CF Comment: https://www.curseforge.com/minecraft/mc-mods/keybind-fix/comments

Github Issue (Linked in CF): https://github.com/fzzyhmstrs/kf/issues

Github Pull Request: https://github.com/fzzyhmstrs/kf/issues

Discord Invite (Clearly linked on CF page): https://discord.gg/Fnw2UbE72x

Adding KF to the "breaks" block, so a user knows in a fail-fast manner to remove the conflicting mods or perhaps actually contact me: https://fabricmc.net/wiki/documentation:fabric_mod_json_spec

Anyways...

I've updated KF in dev to simply not cancel the inject. This will allow other mixins such as your to work, but may introduce indeterminate behavior, so I'm not satisfied it'll be my final solution. As far as I can tell, you don't WrapOperation into anywhere I can usefully hook into and draw context from, as my mod is doing basically the opposite of what yours is (pressing every key bound at once, vs. contextually pressing another key instead of the normal one). In order to actually have a better chance of a true mixin-extras compatible stack, you'd have to stop the normal method execution when you are trying to do something contextual instead, otherwise I'll have no context as to when you are trying to do something on your end or not. You not calling on the original would obviously cue KF to not smash every other key at the same time, because it wouldn't be called at all in that case. A useful case for actually "cancelling" a part of the injection, ironically on your end, because you wouldn't be doing it unconditionally.

Feel free to let me know if you can think of another way to improve compat here, you know, instead of just writing a snarky Mixin Plugin.

[fzzyhmstrs]

The only other thing I can think to do is call KEY_TO_BINDINGS.get() myself, check if the wrapped result of the original get is different from that "bare" get. for example comparing against the result from this: https://github.com/enjarai/do-a-barrel-roll/blob/47a198d9c6725a21fdf835311b7e663b32680e24/src/client/java/nl/enjarai/doabarrelroll/mixin/client/key/KeyBindingMixin.java#L75

and if it's different, assume that something special is happening and don't do what I normally do. It's a pretty steep assumption, but I think it would technically enable mostly-smooth intercompat

[enjarai]

Thanks for reaching out to me in a relatively friendly manner. I realise now that I handled this very poorly.

I'd taken a quick look around your repo, and the way the mixin was designed without best practices in mind combined with the inactivity of the repo, and a recent experience I'd had with another developer ignoring my contact attempts led to me wrongfully assuming you wouldn't be receptive to compatibility requests. I shouldn't have made this assumption, and even if it turned out to be true, could've handled this significantly better.

I've now started looking for actual solutions, but I'm also having trouble figuring out how we could make these mixins work together properly. I don't think there's a simple way here that'd allow both of our mods to work completely as intended, so I suggest we set eachother to "breaks" for now in the interest of, as you mention, being clear to users.

[fzzyhmstrs]

Thank you for your reply, and I apologize for any rudeness in my original issue. It was not something I wanted to have to deal with at 5AM on a random Wednesday, and I think no one wants to hear the news that another mod is disabling their own. Still, I'll try to do better with my attitude in the future.

I believe I have scratched out a concept for an updated process on my end that would enable both systems to work together. Basically:

1. Switch my mixins to TAIL
2. Let the minecraft method do it's thing, including any mixins that may or may not exist.
3. Check the @Local keybinding against a fresh KEY_TO_BINDINGS.get() and see if there is a referential difference (they are different memory objects) a. If different, do nothing. something special has happened. b. If not different, perform my action on every keybinding that's not the one in the vanilla KEY_TO_BINDINGS map. This prevents double-incrementing or resetting state set by other mixins (you, for example, set the original keybinding isPressed to false, I wouldn't want to undo that work).

The only piece of the puzzle I think I'm missing, besides testing my concept obviously, is your conditional mapping of your Context keybinds in updateKeysByCode. Currently I would grab these contextual bindings and shove them into my Multimap. I think I can WrapOperation the same put that you WrapCondition here: https://github.com/enjarai/do-a-barrel-roll/blob/47a198d9c6725a21fdf835311b7e663b32680e24/src/client/java/nl/enjarai/doabarrelroll/mixin/client/key/KeyBindingMixin.java#L98

If I wrap at a lower(?) priority, I think my operation to put into my Multimap will be cancelled along with the vanilla put

I've updated my repo with my concept, currently directly from Github, so entirely untested.

[enjarai]

I think lower priority mixins get executed first, but im honestly not sure about that. This is definitely worth testing though, I'll see if I can compile it locally in a bit.

[enjarai]

I don't know if there's something I'm missing, but I can't figure out how to get KeybindFix to compile. It seems the MC source isnt being added to the classpath at compile time, which is an issue for obvious reasons. If there's some unusual compiling process, it may be good to add that to the README?

[fzzyhmstrs]

Sorry for the delay, I've been on international business trip and now recovering from illness. I'll check on it when I can get back to my PC tomorrow. Nothing unusual about compiling KF that I'm aware of. I typically do it intellij with the build task though; haven't tried any other way.