Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor VirtualInput #179

Merged
merged 57 commits into from
Feb 22, 2024

Conversation

ScribbleTAS
Copy link
Member

@ScribbleTAS ScribbleTAS commented Aug 10, 2023

Big redesign of the VirtualInput system, which is responsible for hooking into the vanilla input system and redirecting the inputs inbetween LWJGL and the keybindings.

Terminology

  • State: A state of the keyboard/mouse at a given time. Has a list of all buttons that are currently pressed.
  • Event: An action to change the keyboard/mouse to a new state. Can only change one key at a time

Example:

Keyboard-State:[]  
-> Keyboard-Event:W, true
Keyboard-State[W]
-> Keyboard-Event:S, true
-> Keyboard-Event:A, true
-> Keyboard-Event:D, true
Keyboard-State[W,S,A,D]
-> Keyboard-Event:A, false
Keyboard-State[W,S,D]

Current system

As it stands, LWJGL only transfers keyboard events, which update the vanilla keybinding state.
VirtualInput was therefore designed to mimic the vanilla keybinding system, by storing inputs as a state, but then being able to export events to update the vanilla keybindings.

And to better handle savestates, exporting events is done by comparing 2 states and then looking at the differences. If there are differences in the keys that are pressed, then an appropriate event can be generated from that.

Changes

Storing only pressed keys

I recently had an epiphany, which made this PR all the more necessary... Why am I storing which keys are unpressed?
Yes, you heard that right, every tick 140ish VirtualKey-Objects are created and 95% of them only store that they are currently not pressed.
In my defense, back then I had no sense of memory management and this system would've made sense if I were to store how often the key is being pressed in a tick, which ended up never happening.

So, to clean up, we now only store the keycodes of the keys that are currently pressed, in a list.

Changing the collection types

3 years ago, my past self apparently didn't know that there are more collection types in java than ArrayList and HashMap.
So, the pressed keycodes are now stored in a LinkedHashSet, because keycodes should never duplicate
and the events that update the vanilla keybindings are now stored in a Queue.

Subticks

Since we only update the keyboards every tick instead of every frame, users would notice some odd behaviour, that were unavoidable with the current system.

When playing the game in low tickrates for example, pressing the inventory key and releasing it inbetween ticks, would not be recognised by the game and the inventory would not open.
In vanilla however, the inventory would indeed open.
Other parts of the game also relied on keyboard pressed inbetween ticks. Back then I didn't know it was necessary so I didn't implement a fix... Later on I had to tack on subtick support for the mouse in the form of "paths", which worked but was in many ways a bad idea.

Now we store key presses inbetween ticks as well, which should now allow for the full input control, while still being updated only 20 times per second. This PR does not include changes to the Playback System and the TASFile, but a preview of how it will look can be seen in #164

Removing Unicode hack

Some chat functions like the arrow keys etc, did not have an associated character and instead used the keycode. This is a problem as we update the keycodes to pressed or unpressed every tick, but the chat relying on characters does not have this constrain.

And the solution I had is still something I really like, but sadly not very future proof and unwieldy to use.

I just assigned special unicode characters to each chat function! And since these symbols are usually not used in chat, you would never run into issues.

The symbols are:

  • Tab: ↦ (21A6)
  • ArrowUp: ↑ (2191)
  • Arrow Left: ← (2190)
  • Arrow Right: → (2192)
  • Arrow Down: ↓ (2193)
  • Pos1: ⇤ (21E4)
  • End: ⇥ (21E5)
  • Prior: ⇧ (21E7)
  • Next: ⇩ (21E9)

Now with subticks, there are no need for these symbols, however maybe I'll add them in the new playback file format as an easteregg/visualisation, as this was somewhat helpful to discern what you were pressing.

Camera Subticks

In vanilla, the player rotation is updated every frame.
This is an issue, when keyboard and mouse are updated every tick and everything should still be synced.
After a lot of testing I decided to make the player rotation update every tick.
This introduced another problem, now rotating the camera was tied to the tickrate and felt like e.g. 20fps even though the framerate was usually higher.

Pancake came up with a solution. We can seperate the rotation from the player from the rotation of the camera.
The "camera" is responsible for the actual view rotation, so what you see on screen, while the player rotation is used for game logic, like at which block you are aiming at.

By updating the camera every frame and updating the player rotation every tick, the change becomes almost unnoticable.

But, only recording every tick meant that during playback, the camera was back at being 20fps. Pancake, yet again,
wrote interpolation code to smooth out the camera path.

Now with subticks on the camera, we have more control over the interpolation frames on the camera, instead of guessing we can just set the camera coordinates.

Unit testing and documentation

Finally... Do I need to say anything?

TODO

  • Store only a list of pressed buttons in keyboard/mouse
  • Add unit testing for VirtualKeyboard
  • Add unit testing for VirtualMouse
  • Add subtick system to VirtualCameraAngle
  • Refactor VirtualKeybindings

@ScribbleTAS ScribbleTAS added this to the Beta1 milestone Aug 10, 2023
@ScribbleTAS ScribbleTAS added Core Issue relates to core concepts Refactor This issue talks about refactoring code labels Aug 10, 2023
@ScribbleTAS ScribbleTAS self-assigned this Aug 10, 2023
@ScribbleTAS ScribbleTAS force-pushed the refactor/virtualinput branch 2 times, most recently from 1b73117 to 4d0ba48 Compare January 6, 2024 20:42
@ScribbleTAS ScribbleTAS changed the title Refactor virtual Package Refactor VirtualInput Jan 13, 2024
- Changed Keys from a Hashmap-List Kombi to an enum
- Added VirtualPeripheral as base class for Mouse and keyboard,
  Containing the button pressing code shared between both

- Moved keyboard keys and mouse keys to VirtualKey.
- Moved all key related functions into VirtualKey
- Moved special unicodes into VirtualKey,
  making it possible to assign them to mosue buttons

- Reworked getDifference and how to store presses alltogether (WIP)
- Added documentation
- Finished get difference calcultion for mouse and keyboard
- Getting ready for new VirtualInput
- VirtualCamera (yet again) renamed to VirtualCameraAngle
- Deleted VirtualChar
- Removed unnecessary constructor in VirtualPeripheral
- Added getters in Peripheral and Keyboard
- Added documentation
- Added test for VirtualKeyboard
- Fixed package name errors
- More implementation in VirtualKeyboard and VirtualMouse2
- More Documentation
- More tests
…ate file

- Added copyFrom, copying only the internals without creating new objects
@ScribbleTAS ScribbleTAS force-pushed the refactor/virtualinput branch from ec97824 to 5303e85 Compare January 15, 2024 21:39
- Removed unnecessary TODO in comments...
- Minor documentation changes
- Remade "copyFrom" to "moveFrom" and making it clear the keyboard that is put in
- Added more documentation to MixinMinecraft
- Attempted to reduce memory footprint in Virtual keyboard and mouse
…nput

- Added missing constructors and methods in Events and Mouse
- Does not make sense if the camera is capped at 20 tps...
- Removed getters in VirtualInput and replaced them with public final variables for better readybility
- Removed InputContainerView
- Added isKeyDown() and willKeyBeDown() to VirtualInput
@ScribbleTAS
Copy link
Member Author

@PancakeTAS
I'm in the process of finalizing the VirtualInput2 and I thought to seek out your feedback on this.

Previously, VirtualInput had Keyboard, Mouse and Subtick (now camera angle) very confusingly mixed up...
For better visibility, I added inner classes to better distinguish between them...
Now VirtualInput2 has an object for each inner class.

And, for better visibility, I chose to make them public final and uppercase, to really show which part of VirtualInput you are using, instead of a getter:
TASmodClient.virtual.KEYBOARD.nextTick() for example.

I was also tryharding a bit to make the relation between currentKeyboard and nextKeyboard as memory efficient as possible, by sometimes passing in queues as a reference so that they can be filled without returning a new Queue...

I'm still missing some documentation and tests for VirtualMouse2 and VirtualInput2 but I first want to see if the style of VirtualKeyboard2 is to your liking before committing on that...

As for "subticks", basically each Keyboard (or the base class VirtualPeripheral) stores all of it's changes.

Everytime update() is called, a clone is created and stored in subtickList, while the "parent" just contains the most recent change. That way I can get rid of the horrendous subtick-path stuff in VirtualMouse and add subtick functionality to VirtualKeyboard, which means that you won't need to hold the keys until the next tick anymore to get stuff applied... Horray...

Pls don't make me rewrite the whole thing

- test-reporter breaks with the new versions

This reverts commit 420b51f.
The parent tick is now always the most recent state, while the subtickList
contains previous states of the peripheral in question.

- Added getAll next to getSubtick, to also return the parent peripheral
@ScribbleTAS
Copy link
Member Author

Note to self:
First mouse click after joining a world is not recognized.
Seems like my old issue "The loading screen doesn't fire LWGJL events" is back...
Since I leftclick to enter the world, the key release event is never fired in the loading screen... Thanks Minecraft!

@ScribbleTAS
Copy link
Member Author

ScribbleTAS commented Feb 15, 2024

Note to self:

  • You are not a massive idiot

- Can now be done via fabric.mod.json
- [Util] Added documentation
- [Util] Refactored PointerNormalizer
- [Gui] Disabled trajectories from InfoHud
- [VirtualInput] Refactored MixinEntityRenderer
- [VirtualInput] Fixed camera angle resetting to 0 on restart
@ScribbleTAS ScribbleTAS force-pushed the refactor/virtualinput branch from bba6ac7 to 5681cd8 Compare February 15, 2024 21:02
@ScribbleTAS ScribbleTAS marked this pull request as ready for review February 22, 2024 13:03
- Removed VirtualButtonEvent inner class in VirtualEvent
@ScribbleTAS ScribbleTAS force-pushed the refactor/virtualinput branch from 5044ecd to 4a5e9f5 Compare February 22, 2024 13:27
@ScribbleTAS ScribbleTAS merged commit 430fe3f into MinecraftTAS:develop Feb 22, 2024
2 checks passed
@ScribbleTAS ScribbleTAS deleted the refactor/virtualinput branch February 22, 2024 14:27
@ScribbleTAS ScribbleTAS mentioned this pull request Feb 27, 2024
5 tasks
@ScribbleTAS ScribbleTAS mentioned this pull request Apr 14, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Issue relates to core concepts Refactor This issue talks about refactoring code
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Refactor VirtualInput
2 participants