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

Rename Slot classinfo name to Slot and not Inventory Slot #5931

Closed
1 task done
Fusezion opened this issue Aug 21, 2023 · 2 comments
Closed
1 task done

Rename Slot classinfo name to Slot and not Inventory Slot #5931

Fusezion opened this issue Aug 21, 2023 · 2 comments
Labels
completed The issue has been fully resolved and the change will be in the next Skript update. enhancement Feature request, an issue about something that could be improved, or a PR improving something. priority: lowest "Nice to have" updates that are not required (tiny low impact bug fixes or QoL enhancements).

Comments

@Fusezion
Copy link
Contributor

Suggestion

Change the .name() of classinfo Slot to "Slot" instead of "Inventory Slot"

Skript/src/main/java/ch/njol/skript/classes/data/SkriptClasses.java

Lines 479 to 483 in 7b0bfce

Classes.registerClass(new ClassInfo<>(Slot.class, "slot")
.user("(inventory )?slots?")
.name("Inventory Slot")
.description("Represents a single slot of an <a href='#inventory'>inventory</a>. " +

Why?

A slot classinfo will not always have an inventory reference, and having the classinfo name being "Inventory Slot" doesn't make sense when classes like DroppedItemSlot, ThrowableProjectileSlot and ItemFrameSlot don't have an inventory reference but say they return "Inventory Slot"

Other

up for debates, but helps with clarity of return types

Agreement

  • I have read the guidelines above and affirm I am following them with this suggestion.
@sovdeeth sovdeeth added enhancement Feature request, an issue about something that could be improved, or a PR improving something. priority: lowest "Nice to have" updates that are not required (tiny low impact bug fixes or QoL enhancements). good first issue An issue that would be good for a first-time contributor to make a PR for labels Aug 22, 2023
@MihirKohli
Copy link
Contributor

Can you assign me above issue?

@MihirKohli MihirKohli mentioned this issue Sep 13, 2023
Merged
@MihirKohli
Copy link
Contributor

MihirKohli commented Sep 13, 2023 •
edited
Loading

My First Contribution
#6001

@AyhamAl-Ali AyhamAl-Ali added the PR available Issues which have a yet-to-be merged PR resolving it label Sep 13, 2023
@MihirKohli MihirKohli removed their assignment Sep 14, 2023
@TheLimeGlass TheLimeGlass added completed The issue has been fully resolved and the change will be in the next Skript update. and removed PR available Issues which have a yet-to-be merged PR resolving it good first issue An issue that would be good for a first-time contributor to make a PR for labels Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
completed The issue has been fully resolved and the change will be in the next Skript update. enhancement Feature request, an issue about something that could be improved, or a PR improving something. priority: lowest "Nice to have" updates that are not required (tiny low impact bug fixes or QoL enhancements).
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@sovdeeth @TheLimeGlass @AyhamAl-Ali @APickledWalrus @MihirKohli @Fusezion