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

Apply potion effects without adding onto current duration #2621

Merged
merged 3 commits into from
Feb 3, 2020
Merged

Apply potion effects without adding onto current duration #2621

merged 3 commits into from
Feb 3, 2020

Conversation

APickledWalrus
Copy link
Member

Description

This PR adds in what is described in #2598. The syntax may be able to be improved and I'm open to any suggestions. I have not really made any changes in Skript's code before, so please let me know if I'm doing anything wrong :)

I built these changes and they worked perfectly when I tested them.

I was thinking we could also add in a test for the entire effect but I was not 100% sure on how to do it properly.


Target Minecraft Versions: Any
Requirements: None
Related Issues: #2598

Add replace existing effect
@ShaneBeee ShaneBeee added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Nov 4, 2019
Copy link
Member

@bensku bensku left a comment

Choose a reason for hiding this comment

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

Sorry for late review! I'd like to see this in 2.5, but there is a tiny issue in the code.

src/main/java/ch/njol/skript/effects/EffPotion.java Outdated Show resolved Hide resolved
@APickledWalrus
Copy link
Member Author

I made a few changes, and I believe it should be good now :)

}
}
}
en.addPotionEffect(new PotionEffect(t, duration, a, ambient, particles), true);
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this prevent applying new effects altogether without using replace syntax?

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like I missed this. It will not work it if replace syntax is not used and the user does not currently have that potion effect, so I will fix this soon.

@bensku bensku merged commit fa237c8 into SkriptLang:master Feb 3, 2020
@APickledWalrus APickledWalrus deleted the APickledWalrus-apply-effect branch September 24, 2020 20:19
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.

3 participants