Skip to content

Commit

Permalink
Fix force fly effect throwing exception when player wasn't allowed to…
Browse files Browse the repository at this point in the history
… fly
  • Loading branch information
FranKusmiruk committed Jul 11, 2018
1 parent b10cbd6 commit cba6552
Showing 1 changed file with 34 additions and 33 deletions.
67 changes: 34 additions & 33 deletions src/main/java/ch/njol/skript/effects/EffMakeFly.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,57 +19,58 @@
*/
package ch.njol.skript.effects;


import org.bukkit.entity.Player;
import org.bukkit.event.Event;
import org.eclipse.jdt.annotation.Nullable;

import ch.njol.skript.Skript;
import ch.njol.skript.doc.Description;
import ch.njol.skript.doc.Examples;
import ch.njol.skript.doc.Name;
import ch.njol.skript.doc.Since;
import ch.njol.skript.lang.Effect;
import ch.njol.skript.lang.Expression;
import ch.njol.skript.lang.SkriptParser;
import ch.njol.skript.lang.SkriptParser.ParseResult;
import ch.njol.util.Kleenean;
import org.bukkit.entity.Player;
import org.bukkit.event.Event;
import org.eclipse.jdt.annotation.Nullable;

@Name("Make Fly")
@Description("Forces a player to start/stop flying.")
@Examples({"make player fly", "force all players to stop flying"})
@Since("2.2-dev34")
public class EffMakeFly extends Effect {

static {
if (Skript.methodExists(Player.class, "setFlying", boolean.class)) {
Skript.registerEffect(EffMakeFly.class, "force %players% to [(1¦stop|0¦start)] fly[ing]",
"make %players% (1¦stop|0¦start) flying",
"make %players% fly");
}
}

@SuppressWarnings("null")
private Expression<Player> players;

private boolean flying;
static {
if (Skript.methodExists(Player.class, "setFlying", boolean.class)) {
Skript.registerEffect(EffMakeFly.class, "force %players% to [(start|1¦stop)] fly[ing]",
"make %players% (start|1¦stop) flying",
"make %players% fly");
}
}

@SuppressWarnings({"unchecked", "null"})
@Override
public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, SkriptParser.ParseResult parseResult) {
players = (Expression<Player>) exprs[0];
flying = parseResult.mark == 0;
return true;
}
@SuppressWarnings("null")
private Expression<Player> players;
private boolean flying;

@Override
protected void execute(Event e) {
for (Player player : players.getArray(e)) {
player.setFlying(flying);
}
}
@SuppressWarnings({"unchecked", "null"})
@Override
public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) {
players = (Expression<Player>) exprs[0];
flying = parseResult.mark != 1;
return true;
}

@Override
public String toString(@Nullable Event e, boolean debug) {
return "make " + players.toString(e, debug) + (flying ? " start " : " stop ") + "flying";
}
@Override
protected void execute(Event e) {
for (Player player : players.getArray(e)) {
player.setAllowFlight(flying);
player.setFlying(flying);
}
}

@Override
public String toString(@Nullable Event e, boolean debug) {
return "make " + players.toString(e, debug) + (flying ? " start " : " stop ") + "flying";
}

}

9 comments on commit cba6552

@TheLimeGlass
Copy link
Collaborator

@TheLimeGlass TheLimeGlass commented on cba6552 Jul 11, 2018

Choose a reason for hiding this comment

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

Skript.registerEffect(EffMakeFly.class, "force %players% to [(start|1¦stop)] fly[ing]",
+												"make %players% (start|1¦stop) flying",
+												"make %players% fly");
Skript.registerEffect(EffMakeFly.class, "force %players% to [(start|1¦stop)] fly[ing]",
					"make %players% (start|1¦stop) flying",
					"make %players% fly");

Line these up, and add the fly[ing] optional to the other second syntax, or the first and second syntax should be merged.

@Snow-Pyon
Copy link

@Snow-Pyon Snow-Pyon commented on cba6552 Jul 11, 2018

Choose a reason for hiding this comment

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

  1. I'll do it later (The IDE shows it completely different).
  2. I'm pretty sure Make player start fly isn't proper grammar.

@Blueyescat
Copy link
Contributor

Choose a reason for hiding this comment

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

Noo, the user won't know that they has to disallow flight later.

@bensku
Copy link
Member

@bensku bensku commented on cba6552 Jul 12, 2018

Choose a reason for hiding this comment

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

That would be unfortunate. Kind of exploitable issue in survival servers.

@Snow-Pyon
Copy link

Choose a reason for hiding this comment

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

Should I disallow it after setting fly or just a documentation change?

@bensku
Copy link
Member

@bensku bensku commented on cba6552 Jul 12, 2018

Choose a reason for hiding this comment

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

Documentation change should be enough.

@Nicofisi
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm.. I know stuff like that is supposed to be simple in Skript, but on the java/practical side, setAllowFlight and setFlying are two completely different things and should be split into two separate effects imo

@bensku
Copy link
Member

@bensku bensku commented on cba6552 Jul 12, 2018

Choose a reason for hiding this comment

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

Forcing something to fly should be always possible. It is unfortunate that Minecraft separate forced flight from normal, creative flight.

@Blueyescat
Copy link
Contributor

@Blueyescat Blueyescat commented on cba6552 Jul 12, 2018

Choose a reason for hiding this comment

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

I think

disallow it after setting fly

is a good idea if it wouldn't cause a problem 🤔

Please sign in to comment.