Skip to content

Conversation

@MattSturgeon
Copy link
Member

@MattSturgeon MattSturgeon commented Dec 15, 2023

Motivation

Fewer dependencies tied to minecraft versions/platforms makes it easier to support new/different mc versions without relying on others to do so first.

Although this means we have to manually use the various platform APIs we target, this isn't actually a big deal in our case, since we use very few APIs:

  • Tick events
  • Init event
  • Keybind registration

Using the APIs directly may also give us more control.

Implementation

Key bindings

Previously, keybinds were stored as discrete fields on the main Freecam class. This meant having to construct a separate List (or stream) in order to iterate over them when registering.

Rather than maintain a list of keybinds twice, which would be tedious and prone to error, I've introduced the ModBindings enum in the config package.

This has several advantages:

  • We automatically get a list of all our key binds via Enum::values
  • We can make declarations more readable using a custom constructor
  • We can (more easily) construct the keybinds lazily, ensuring they aren't added to KeyBinding's maps until we're ready to register them.

To minimize boilerplate elsewhere I've wrapped the isPressed() and wasPressed() methods. We could also introduce our own wasPressedReset() to improve handler code, e.g:

public boolean wasPressedReset() {
    if (wasPressed()) {
        // Alternatively, use AW to access KeyBinding::reset
        // get().reset();
        while (wasPressed()) {}
        return true;
    }
    return false;
}

Extract the `ClientTickEvent.CLIENT_POST` handler into its own method `postTick` and clean up the code a little.

Replace preTick mixin with a `ClientTickEvent` listener, which uses Fabric's `START_CLIENT_TICK`[1] and Forge's `ClientTickEvent` with `Phase.START`[2].
This is functionally equivalent.

[1]: https://maven.fabricmc.net/docs/fabric-api-0.91.1+1.20.2/net/fabricmc/fabric/api/client/event/lifecycle/v1/ClientTickEvents.html#START_CLIENT_TICK
[2]: https://nekoyue.github.io/ForgeJavaDocs-NG/javadoc/1.19.3/net/minecraftforge/event/TickEvent.ClientTickEvent.html#%3Cinit%3E(net.minecraftforge.event.TickEvent.Phase)
- Move keybind declarations out of `Freecam`
- Removes the need for a `ALL_KEYS` list,
  (registering is more robust).
- Constructor overloads allow more readable declarations.
- Used a lazy constructor as recommended by forge docs.
@hashalite
Copy link
Collaborator

Thank you sir!

@hashalite hashalite merged commit 98e87dd into MinecraftFreecam:main Dec 19, 2023
MattSturgeon referenced this pull request Dec 24, 2023
@MattSturgeon MattSturgeon deleted the drop-arch-api branch January 2, 2024 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants