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

Improve kill effect #7148

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

Conversation

EquipableMC
Copy link
Contributor

Description

This simply improves the kill effect by adding an option to prevent totems from popping


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

@SuppressWarnings("null")
private Expression<Entity> entities;
private boolean IGNORE_TOTEM;
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
private boolean IGNORE_TOTEM;
private boolean ignoreTotem;

@Efnilite Efnilite added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Oct 14, 2024
@Since("1.0")
public class EffKill extends Effect {

static {
Skript.registerEffect(EffKill.class, "kill %entities%");
Skript.registerEffect(EffKill.class, "kill %entities%", "kill %entities% ignoring [the] (resurrection|revival|[undying] totem|totem [of undying])");
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a huge fan of the syntax. I might prefer to just keep it simple:

ignoring [the|any] totem[s] [of undying]

I'd like to see what others think too though

Copy link
Contributor Author

@EquipableMC EquipableMC Oct 14, 2024

Choose a reason for hiding this comment

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

What me and others have discussed on discord, is that the kill effect should be recoded to actually kill, since as of right now, it really just feels like a hardcoded damage effect. Another point is that the vanilla /kill command ignores the totem, whereas this, it does not ignore the totem.

Copy link
Member

Choose a reason for hiding this comment

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

Pickle is talking about the pattern being overly complex, not about it's behavior.

Copy link
Member

Choose a reason for hiding this comment

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

personally i think ignoring [the|any] totem[s] of undying should be it, as ignoring totems sounds too ambiguous imo

"kill all endermen, witches and bats",
"kill the player ignoring totem of undying",
"kill target entity ignoring the totem",
"kill target player ignoring resurrection"})
@Since("1.0")
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
@Since("1.0")
@Since("1.0, INSERT VERSION (ignoring totem of undying)")

@Since("1.0")
public class EffKill extends Effect {

static {
Skript.registerEffect(EffKill.class, "kill %entities%");
Skript.registerEffect(EffKill.class, "kill %entities%", "kill %entities% ignoring [the] (resurrection|revival|[undying] totem|totem [of undying])");
Copy link
Member

Choose a reason for hiding this comment

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

personally i think ignoring [the|any] totem[s] of undying should be it, as ignoring totems sounds too ambiguous imo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants