Suggestion: a setting to automatically disable flight over PvP plots

[MilesBHuff]

In Minecraft's new EULA and MUG, no entitlements may be sold that provide any kind of competitive advantage. In most cases, that includes flight; however, in a situation where paid flight is only possible in towns and never in PvP areas, and where that flight offers no competitive advantage to players (merely a building convenience), then flight can still reasonably be sold under the terms of the EULA and MUG.

In order for us to return to our member perks as they were before the new EULA and MUG, we need the ability to make flight automatically disable over PvP plots. The paltry sum I get through subscriptions is being re-donated in part to you (and atm I take a large loss on the server as a whole). In-town flight and afking without kicking were the two main perks of membership, and I fear that membership may not be not sufficiently worthwhile without that flight perk.

This feature is also useful for servers that do not monetize flight, because even in those cases, it helps to preserve a fair and even playing field for participants in PvP.

TownyAdvanced/TownyFlight#86 will likely become even more important with this change.

[MilesBHuff]

It may be possible to implement this feature almost for free as a side-effect of implementing TownyAdvanced/TownyFlight#87.

[MilesBHuff]

There is apparently already a feature in TownyFlight that accomplishes compliance with the EULA, albeit differently than I proposed here. The setting is disable_Combat_Prevention, and a related setting is disable_During_Wartime. This main setting makes it so that people who are flying are unable to participate in PvP. I think I am satisfied with these, so I am going to close this ticket. But I do have questions about how exactly it works. The setting's comment says "If set to false, TownyFlight will not prevent combat of flying people.". I assume this means that flying people can't hurt other people; but does this also mean that flying people can't be hurt by other people? Because if so, then flight is still potentially a competitive advantage.

[MilesBHuff]

One thing that I noticed in testing, is that a flying player was able to hurt me in a PvP area while I was not flying. I would like to have two players test that on each other, since maybe me being an admin exempts me from certain logics.
I think there may just have been confusion around what the setting means, though. The comment says that setting disable_Combat_Prevention to false allows combat, but the name of the variable itself implies that true allows combat, not false. I'm not entirely sure which of the comment and name are correct, but I'll try changing it in the other direction (from false to true) and see what that yields.

[LlmDl]

https://github.com/TownyAdvanced/TownyFlight/blob/master/src/main/java/com/gmail/llmdlio/townyflight/listeners/PlayerPVPListener.java

	/*
	 * Listener to turn off flight if flying player enters PVP combat. Runs only if
	 * the config.yml's disable_Combat_Prevention is set to true.
	 */
	@EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true)
	private void playerPVPEvent(TownyPlayerDamagePlayerEvent event) {
		Player attackingPlayer = event.getAttackingPlayer();
		Player defendingPlayer = event.getVictimPlayer();

		if (!attackingPlayer.getAllowFlight())
			return;

		if (attackingPlayer.getGameMode().equals(GameMode.CREATIVE) || !defendingPlayer.canSee(attackingPlayer)) {
			event.setCancelled(true);
			return;
		}

		TownyFlightAPI.getInstance().removeFlight(attackingPlayer, false, true, "pvp");
		event.setCancelled(true);
	}

It only takes into consideration the attackingPlayer abusing their flight.

When the setting in the config is true, TownyFlight registers the PVPListener:

		if (Settings.disableCombatPrevention)
			pm.registerEvents(new PlayerPVPListener(), this);

The config name might be a bit dumb...

[MilesBHuff]

Thanks for the info and clarifications!

So it looks like if both players have flight enabled (even if the defending one is on the ground), then PvP can happen with flight enabled. If a player with flight enabled attacks a player without flight enabled, then the damage is cancelled; and if the victim can see the aggressor, then the aggressor's flight is disabled.

I wonder if it might be better / conceptually simpler to do something like the below pseudocode?:

	@EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true)
	private void playerPVPEvent(TownyPlayerDamagePlayerEvent event) {

		Player attackingPlayer = event.getAttackingPlayer();
		Player defendingPlayer = event.getVictimPlayer();

		Boolean attackerIsCreative = attackingPlayer.getGameMode().equals(GameMode.CREATIVE);
		Boolean defenderIsCreative = defendingPlayer.getGameMode().equals(GameMode.CREATIVE);

		Boolean attackerCanFly = attackingPlayer.getAllowFlight();
		Boolean defenderCanFly = defendingPlayer.getAllowFlight();

		Boolean attackerIsFlying = attackingPlayer.getIsFlying();
		Boolean defenderIsFlying = defendingPlayer.getIsFlying();

		TownyFlightInstance townyFlightInstance = TownyFlightAPI.getInstance();

		// If either player is in creative mode, disallow combat.
		if (attackerIsCreative) {
			event.setCancelled(true);
			attackingPlayer.receiveMessage("You cannot engage in PvP while in creative mode.");
			return;
		}
		if (defenderIsCreative) {
			event.setCancelled(true);
			attackingPlayer.receiveMessage("You cannot engage in PvP with a player who is in creative mode.");
			return;
		}

		// If the attacker is flying, disallow the attack.
		if (attackerIsFlying) {
			event.setCancelled(true);
			attackingPlayer.receiveMessage("You cannot engage in PvP while flying.");
			return;
		}
		// If the grounded attacker has flight enabled, disable their flight.
		if (attackerCanFly) {
			townyFlightInstance.removeFlight(attackingPlayer, false, true, "PvP");
		}

		// If the defender is airborn, warn them that they've lost fly.  Maybe also give them a few seconds of slowfall?  Idk.
		if (defenderIsFlying) {
			attackingPlayer.receiveMessage("You've been shot-down out of the sky!");
			// defendingPlayer.applyStatusEffect(STATUS_EFFECTS.SLOWFALL, 100);
		}
		// If the defender has flight enabled, disable their flight.
		if (defenderCanFly) {
			townyFlightInstance.removeFlight(defendingPlayer, false, true, "PvP");
		}
	}

The tl;dr of this, is that PvP is disabled in creative, attackers cannot start PvP while flying, and flight is removed from all participants when PvP starts.

This doesn't allow combat between flying individuals at all, even if they both have it. If some servers want in-flight combat, I guess there could be a config option to control that.

But I think in general this operates more how people might expect it to? Idk.

[LlmDl]

So I think the issue is that for attacking players, we dont test if they are flying, only whether they are allowed to fly, and because of this you're seeing issues when grounded players attack other players. Am I correct?

[MilesBHuff]

So I think the issue is that for attacking players, we dont test if they are flying, only whether they are allowed to fly, and because of this you're seeing issues when grounded players attack other players. Am I correct?

No, we tested someone who was flying attacking a grounded player. The grounded player took damage. The current code does not account for flying, only for whether flight is enabled for the attacker and whether they are technically within view of the defender. This allows for the possibility of sneak attacks in which flight offers a competitive advantage.

My proposed code prevents that by thoroughly ensuring that flighted PvP is prevented in all cases.

[LlmDl]

The code that is used right now does remove their flight when the attacker isn't in creative or vanished from the defending player.

[LlmDl]

Can I see your townyflight config?

[MilesBHuff]

Sure! Sorry for the delay -- have been really busy, and wanted to test again with a user first.

All of our version-controlled configs can be found on our GitHub. Here's a link to the file you asked for: https://github.com/SettleScape/server/blob/main/minecraft/plugins/TownyFlight/config.yml

The most-recent test was with disable_Combat_Prevention: 'true', and tested a ranged attack from an in-air player vs a grounded player (both of whom had flight enabled) in two situations: when the defender was looking at the flying attacker, and when the defender was not.
I still do not feel this provides a transparent or optimal user experience. While flight was taken away from the attacker in the back-turned test, I still had flight after being attacked in both cases, and I still received damage in both cases.
As well, this situation still provides an advantage to flighted players, since it allows them to get off an initial or retaliatory strike while in the air, which is a luxury unflighted players do not have. This opens up potential EULA/MUG concerns when flight is purchaseable.
The logic I proposed above would resolve such inconsistencies.
I am not sure why event.setCancelled(true); did not cancel the attacks.

[LlmDl]

when the defender was looking at the flying attacker, and when the defender was not.

There has been a mis-communication. The canSee test is to handle when a player is Vanished from another player. Ie: an admin is vanished and accidentally hits a player.

[LlmDl]

If you saw the attacker's flight being removed, that is only done while also setting the event to cancelled:

		TownyFlightAPI.getInstance().removeFlight(attackingPlayer, false, true, "pvp");
		event.setCancelled(true);

There is likely another plugin that is overriding the towny TownyPlayerDamagePlayerEvent or the underlying bukkit damage event.

[MilesBHuff]

There is likely another plugin that is overriding the towny TownyPlayerDamagePlayerEvent or the underlying bukkit damage event.

Interesting -- thanks. I'll look into that.