Exopandora / ShoulderSurfing

Shoulder Surfing Reloaded is a highly configurable third person camera mod for minecraft.
MIT License
63 stars 13 forks source link

[Suggestion] [1.19.2+] API for adaptive crosshair #104

Closed ZsoltMolnarrr closed 1 year ago

ZsoltMolnarrr commented 1 year ago

API for adaptive crosshair My mod adds magical staves into the game, those shoot fireballs and stuff. I would like to register a condition for my weapons, to make adaptive crosshair recognise them, so the aiming is user friendly like with bows.

Suggested interface: Package: com/teamderpy/shouldersurfing/api

public final class ShoulderSurfing {
  /// The given function determines about the currently held itemstack if it should use adaptive crosshair
  /// Multiple functions registered in this way are in logical `OR` relation
  public static void registerForAdaptiveCrosshair(Function<ItemStack,Boolean> condition) { ... }
}
Exopandora commented 1 year ago

Thanks for the suggestion! I will look into it.

Exopandora commented 1 year ago

I made a new push to master with plugin support. I also added a short documentation in the wiki. My intention is to port the plugin support throughout the week to other mc versions and release them all together at the end of the week. If you have any feedback for changes in the api, feel free to comment.

ZsoltMolnarrr commented 1 year ago

Hello! Thank you very much! I am very busy these days. I would like to ask for a few days to give feedback.

Exopandora commented 1 year ago

No problem! I will wait. No need to rush this.

ZsoltMolnarrr commented 1 year ago

Excuse me for asking for more, could you please provide an example for registering an item? (Any item, an apple would be fine too)

Exopandora commented 1 year ago

You can take a look at the wiki on how to register your plugin and what callbacks are supported (note: there is a helper function to register an IAdaptiveItemCallback with just an ItemStack predicate). If you still need more infos, feel free to leave another comment and i will give you a full example.

ZsoltMolnarrr commented 1 year ago

Hello! The API looks fully functional and I am grateful for the documentation and implementation. (I wasn't able to try the API yet, because the project driving the needs is only available for 1.19.2 at the moment)

Based on the wiki, there is only one thing that isn't entirely clear: shouldersurfing_plugin.json at root folder. Does it mean it has to be at the same location as mixin.json at resources/shouldersurfing_plugin.json?

Some additional opinions on the API, just for the sake of improvement (feel free ignore it if you don't like it or disagree):

Overall great stuff!

Exopandora commented 1 year ago

Thank you for the feedback! Here is my response:

Does it mean it has to be at the same location as mixin.json at resources/shouldersurfing_plugin.json?

Thats correct. I adjusted the wiki to make it more clear.

[the complexity] looks unnecessary to me, because the goal essentially is, to just inject a supplier from external code

I initially designed IShoulderSurfingPlugin to be exactly like the contents of IAdaptiveItemCallback. But i figured that it lacks flexibility. i.e. if i wanted to add another API method, plugins would have to implement both (yes, i know default interface methods exists but its poor design). I also wanted it to work for any modloader in the exact same way, eliminating the usage of forge/fabric events. I tried to make it a little bit more "userfriendly" by providing the helper funtion i mentioned earlier. An example plugin for your use case would look as follows (ignoring the shouldersurfing_plugin.json):

public class ExamplePlugin implements IShoulderSurfingPlugin {
    @Override
    public void register(IShoulderSurfingRegistrar registrar) {
        // Register simple ItemStack predicate
        registrar.registerAdaptiveItemCallback(itemStack -> itemStack.is(Items.APPLE));
        // Register another one
        registrar.registerAdaptiveItemCallback(itemStack -> itemStack.is(Items.GOLDEN_APPLE));
        // Register a full callback as inner class
        registrar.registerAdaptiveItemCallback(new IAdaptiveItemCallback() {
            @Override
            public boolean isHoldingAdaptiveItem(Minecraft minecraft, LivingEntity entity) {
                for(ItemStack stack : entity.getHandSlots()) {
                    if(stack.is(Items.ENCHANTED_GOLDEN_APPLE)) {
                        return true;
                    }
                }
                return false;
            }
        });
        // Register a full callback as lambda
        registrar.registerAdaptiveItemCallback((minecraft, entity) -> {
            for(ItemStack stack : entity.getHandSlots()) {
                if(stack.is(Items.BAMBOO)) {
                    return true;
                }
            }
            return false;
        });
    }
}

For developer documentation Github readme is more commonly used

Yes, i agree, but i think its way too long to be included in the readme.

Exopandora commented 1 year ago

I can also add the following helper methods to simplify it even more:

registrar.registerAdaptiveItemCallback(IAdaptiveItemCallback.anyHandMatches(Items.APPLE));
registrar.registerAdaptiveItemCallback(IAdaptiveItemCallback.mainHandMatches(Items.APPLE));
registrar.registerAdaptiveItemCallback(IAdaptiveItemCallback.offHandMatches(Items.APPLE));
ZsoltMolnarrr commented 1 year ago

Thank you for the example code! From my side everything looks fine!:)

ZsoltMolnarrr commented 1 year ago

An idea that just popped, about the json file. In a Fabric environment an entry in fabric.mod.json for the the plugin class, could be a more native solution. Just like Mod Menu does it:

"entrypoints": {
  "client": [
    "net..."
  ],
  "main": [
    "net..."
  ],
  "modmenu": [
    "net..."
  ]
},

Does Forge has any similar option? Would be bad if this was supported (additionaly) for Fabric?

Exopandora commented 1 year ago

Yes that would be possible. I think you can also do this forge with the mods.toml. But i intentionally decided against it because using a separate file makes it modloader agnostic, simplifying the process for modders, as they only have to define one file. It also enables modders to fully implement a functional plugin within a 'common' or 'shared' project. Addionally, it provides flexibility for me to adjust the data scheme of the file and its also much easier to parse.

ZsoltMolnarrr commented 1 year ago

Okay, all good! I am happy with this API either way!:)

Exopandora commented 1 year ago

I created a new release including the new api. Thanks again for the suggestions! If you have any further questions or suggestions, feel free to add more comments to this issue or open up another one. I'm closing this one now.

ZsoltMolnarrr commented 1 year ago

Works like a charm

https://user-images.githubusercontent.com/4989469/216463331-19eeb6fe-fe45-4838-b6c6-67bd5026b3f8.mov