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

Add new bindings for MobEffectInstance binding #955

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

wolfieboy09
Copy link

@wolfieboy09 wolfieboy09 commented Feb 10, 2025

Description

In my mod, when registering a thing, you can add a list of effects to it. I did not want to add bindings to the MobEffects and to make a MobEffectInstance.

This PR adds two new bindings to create a MobEffectInstance, and also to fetch effects from MobEffects.

I'm not sure of MobEffects is already a binding, but well, added it sinse I could not find it..

I added a binding for the MobEffectInstance for most of the constructors.

Example Script

Using MobEffects.CONFUSION for example (nausia)

// Using my mod's registry for example
StartupEvents.registry("qstorage:gas", event => {
    event.create("something").effects(
        MobEffectInstance.of("minecraft:absorption"),
        MobEffectInstance.of("minecraft:absorption", 4), // Duration of 4
        MobEffectInstance.of("minecraft:absorption", 4, 5), // Amplifier of 5
        MobEffectInstance.of("minecraft:absorption", 4, 5, false, true), // Not ambient, visible
        MobEffectInstance.of("minecraft:absorption", 4, 5, false, true, true) // Shows the icon
    )
})

Other details

This adds two new bindings so I would not need to do it with my mod, and would just be built into KubeJS.

Feel free to make changes.

@wolfieboy09 wolfieboy09 changed the title Add the ability to make Add new bindings for MobEffectInstance wrapping and for MobEffects Feb 10, 2025
@wolfieboy09
Copy link
Author

okay, just fixed the title

@liopyu
Copy link

liopyu commented Feb 10, 2025

yeah but why

@liopyu
Copy link

liopyu commented Feb 10, 2025

you can just bind it with Java.loadClass()

@liopyu
Copy link

liopyu commented Feb 10, 2025

also, i may be confused about this but the title insinuates you want to add MobEffectInstance binding as well but it only is adding MobEffects class
image

@wolfieboy09
Copy link
Author

????

@wolfieboy09
Copy link
Author

image

Oh, because it's not on the patch branch

@Prunoideae
Copy link
Contributor

Prunoideae commented Feb 10, 2025

Holder can be automatically type-wrapped by Rhino already in 1.21. You can test by passing string into it.

image
image

Actually, any Holder can be type-wrapped by Rhino using the resource location string of the registry object you want to represent.

@wolfieboy09
Copy link
Author

okay, there we go

@wolfieboy09
Copy link
Author

okay, now that MobEffects is deleted from bindings, and updated the script

@liopyu
Copy link

liopyu commented Feb 10, 2025

renaming the binding for your MobEffectUtils class with the alias "MobEffectInstance" will be highly confusing

@wolfieboy09
Copy link
Author

reanme it to "MobEffectUtil" in the binding?

@liopyu
Copy link

liopyu commented Feb 10, 2025

yea

@wolfieboy09 wolfieboy09 changed the title Add new bindings for MobEffectInstance wrapping and for MobEffects Add new bindings for MobEffectInstance binding Feb 10, 2025
@wolfieboy09
Copy link
Author

Add the info tags to the methods

Copy link
Contributor

@ChiefArug ChiefArug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing, we now have infinite effect durations, a helper (or two or three) should be added for those.

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.

4 participants