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

Drop requirement on Architectury API #129

Merged
merged 5 commits into from
Dec 19, 2023

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