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 accepts change condition #94

Open
wants to merge 24 commits into
base: 2.x
Choose a base branch
from
Open

Conversation

Pikachu920
Copy link
Member

@Pikachu920 Pikachu920 commented Feb 4, 2024

Aims to add an easy to to see if an expression accepts a changemode

@Fusezion
Copy link

Fusezion commented Feb 4, 2024

let me know if I'm over simplifying this but couldn't we just add ChangeMode as a enum in skript-reflect and do something like

%objects% supports %changemode% changer
if slot supports delete changer
if player's tool supports set changer

might be better to suffix modes with changer by default since idk if it'll conflict with related effects

since expressions and likely classinfos have getAcceptedChangeModes which returns a map it should be as simple as a null check

@Pikachu920
Copy link
Member Author

getAcceptedChangeModes

getAcceptedChangeModes is intended for documentation purposes, not real logic

@Pikachu920 Pikachu920 changed the title Add CondChange Add accepts change condition Feb 24, 2024
@@ -214,6 +215,31 @@ public String getVariableNamePattern() {
Classes.registerClass(new ClassInfo<>(Section.class, "section")
.user("sections?")
);

Classes.registerClass(new ClassInfo<>(Expression.class, "expression")
Copy link
Member Author

Choose a reason for hiding this comment

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

skript-reflect actually already returns unwrapped Expression objects, but there hasn't been a registered classinfo for it

@Pikachu920 Pikachu920 changed the base branch from 2.x to feature/test February 24, 2024 00:52
@Pikachu920 Pikachu920 changed the base branch from feature/test to 2.x February 24, 2024 00:52
@Pikachu920 Pikachu920 marked this pull request as ready for review February 24, 2024 00:58
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.

Only optional changes! nice work

.github/workflows/gradle.yml Show resolved Hide resolved
Comment on lines +20 to +21
{"%classinfo% can be added to %expressions%", ChangeMode.ADD},
{"%classinfo% (can't|cannot) be added to %expressions%", ChangeMode.ADD},
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{"%classinfo% can be added to %expressions%", ChangeMode.ADD},
{"%classinfo% (can't|cannot) be added to %expressions%", ChangeMode.ADD},
{"%classinfo% can be (added|given) to %expressions%", ChangeMode.ADD},
{"%classinfo% (can't|cannot) be (added|given) to %expressions%", ChangeMode.ADD},

Not sure if it's worth doing this, but if you agree, the other modes should be done as well, like clear for DELETE

Copy link
Member Author

Choose a reason for hiding this comment

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

sounds like a good idea. i can't recall if skript has a dirt can be given to player type syntax though. i'll have to double check so we dont accidentally cause a conflict if there is

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.

3 participants