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.

Sign up for GitHub

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

Jump to bottom

Item Skull Owner #7174

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

Item Skull Owner #7174

wants to merge 12 commits into from

Conversation

TheAbsolutionism
Copy link
Contributor

Description

This PR aims to allow items to be used in ExprSkullOwner

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

@Romitou Romitou added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Nov 1, 2024
Efnilite
Efnilite requested changes Nov 1, 2024
View reviewed changes
Copy link
Member

@Efnilite Efnilite left a comment

Choose a reason for hiding this comment

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

tests 🥺

src/main/java/ch/njol/skript/expressions/ExprSkullOwner.java Outdated Show resolved Hide resolved
cheeezburga
cheeezburga approved these changes Nov 2, 2024
View reviewed changes
Copy link
Member

@cheeezburga cheeezburga left a comment

Choose a reason for hiding this comment

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

Looks good. Are the deprecated method calls able to be replaced with updated ones? I only had a quick glance at the api, but could stuff like setOwner("") be replaced with setOwningPlayer(null), and stuff like that?

@Efnilite Efnilite requested a review from sovdeeth November 12, 2024 21:55
@Efnilite Efnilite added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Efnilite Efnilite Efnilite approved these changes

@NotSoDelayed NotSoDelayed NotSoDelayed approved these changes

@cheeezburga cheeezburga cheeezburga approved these changes

@Moderocky Moderocky Moderocky approved these changes

@sovdeeth sovdeeth sovdeeth approved these changes

Assignees
No one assigned
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@TheAbsolutionism @sovdeeth @Moderocky @Efnilite @cheeezburga @NotSoDelayed @Romitou