Skip to content

Cow Variants #7866

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

Open
wants to merge 4 commits into
base: dev/feature
Choose a base branch
from

Conversation

Absolutionism
Copy link
Contributor

Description

This PR aims to add support for cow variants to Skript.
A bit of reflect was needed in order to set and get the variant.


Target Minecraft Versions: any
Requirements: none
Related Issues: none

@Absolutionism Absolutionism requested a review from a team as a code owner May 15, 2025 07:38
@Absolutionism Absolutionism requested review from sovdeeth and erenkarakal and removed request for a team May 15, 2025 07:38
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label May 15, 2025
@ShaneBeee
Copy link
Contributor

Rather than all the reflection, that will be removed in a couple years, why not just only register the CowData class if the variant exists?
(same for simple data, if the class doesn't exist, register a simple cow)

@Absolutionism
Copy link
Contributor Author

Rather than all the reflection, that will be removed in a couple years, why not just only register the CowData class if the variant exists? (same for simple data, if the class doesn't exist, register a simple cow)

Even if I made it where CowData would only be registered on 1.21.5+ it would still require the reflection.

@ShaneBeee
Copy link
Contributor

why would it require reflection?

@Absolutionism
Copy link
Contributor Author

Absolutionism commented May 15, 2025

why would it require reflection?

I'm not entirely sure of how it works. But for some reason Cow.class was returning/redirecting to AbstractCow.class
and within #set(Cow cow) it was throwing a NoSuchMethod for the #setVariant, because for some reason cow was an AbstractCow. I asked in Paper discord couple hours ago, and it has something to do with the api-version in plugin.yml.

Before making the PR, I had changed the api-version to 1.21.5 and it worked perfectly, but since it needed to be 1.19, then that wasn't a viable option.

@ShaneBeee
Copy link
Contributor

convo continued on Discord. Due to some silly API stuff in Paper, I guess the reflection is necessary.

@sovdeeth
Copy link
Member

Can we get a short summary of why in a comment/in the PR description? I'm curious about it too

@ShaneBeee
Copy link
Contributor

Can we get a short summary of why in a comment/in the PR description? I'm curious about it too

Basically as Smurfy already said.
By not having api-version: "1.21.5" ... Paper is forcing the Cow class to return as AbstractCow.
So when doing Cow#setVariant its forcing AbstractCow#setVariant (which doesn't exist)

@sovdeeth
Copy link
Member

Can we get a short summary of why in a comment/in the PR description? I'm curious about it too

Basically as Smurfy already said. By not having api-version: "1.21.5" ... Paper is forcing the Cow class to return as AbstractCow. So when doing Cow#setVariant its forcing AbstractCow#setVariant (which doesn't exist)

Wacky, thanks

Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

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

looks pretty good

@Absolutionism Absolutionism requested a review from sovdeeth May 15, 2025 22:25
@sovdeeth sovdeeth added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label May 16, 2025
@skriptlang-automation skriptlang-automation bot removed the needs reviews A PR that needs additional reviews label May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants