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

SpectateLogin implementation #2552

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/main/java/fr/xephi/authme/AuthMe.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import fr.xephi.authme.service.BackupService;
import fr.xephi.authme.service.BukkitService;
import fr.xephi.authme.service.MigrationService;
import fr.xephi.authme.service.SpectateLoginService;
import fr.xephi.authme.service.bungeecord.BungeeReceiver;
import fr.xephi.authme.service.yaml.YamlParseException;
import fr.xephi.authme.settings.Settings;
Expand Down Expand Up @@ -69,6 +70,7 @@ public class AuthMe extends JavaPlugin {
private Injector injector;
private BackupService backupService;
private ConsoleLogger logger;
private SpectateLoginService spectateLoginService;

/**
* Constructor.
Expand Down Expand Up @@ -246,6 +248,7 @@ void instantiateServices(Injector injector) {
bukkitService = injector.getSingleton(BukkitService.class);
commandHandler = injector.getSingleton(CommandHandler.class);
backupService = injector.getSingleton(BackupService.class);
spectateLoginService = injector.getSingleton(SpectateLoginService.class);

// Trigger instantiation (class not used elsewhere)
injector.getSingleton(BungeeReceiver.class);
Expand Down Expand Up @@ -316,6 +319,9 @@ public void onDisable() {
// Wait for tasks and close data source
new TaskCloser(this, database).run();

// Remove all armorstands so that the players do not get Spectator gamemode
spectateLoginService.removeArmorstands();

// Disabled correctly
Consumer<String> infoLogMethod = logger == null ? getLogger()::info : logger::info;
infoLogMethod.accept("AuthMe " + this.getDescription().getVersion() + " disabled!");
Expand Down
37 changes: 37 additions & 0 deletions src/main/java/fr/xephi/authme/listener/PlayerListener.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import fr.xephi.authme.service.AntiBotService;
import fr.xephi.authme.service.BukkitService;
import fr.xephi.authme.service.JoinMessageService;
import fr.xephi.authme.service.SpectateLoginService;
import fr.xephi.authme.service.TeleportationService;
import fr.xephi.authme.service.ValidationService;
import fr.xephi.authme.settings.Settings;
Expand All @@ -19,6 +20,7 @@
import fr.xephi.authme.settings.properties.RegistrationSettings;
import fr.xephi.authme.settings.properties.RestrictionSettings;
import org.bukkit.ChatColor;
import org.bukkit.GameMode;
import org.bukkit.Location;
import org.bukkit.entity.HumanEntity;
import org.bukkit.entity.Player;
Expand Down Expand Up @@ -49,6 +51,8 @@
import org.bukkit.event.player.PlayerQuitEvent;
import org.bukkit.event.player.PlayerRespawnEvent;
import org.bukkit.event.player.PlayerShearEntityEvent;
import org.bukkit.event.player.PlayerTeleportEvent;
import org.bukkit.event.player.PlayerToggleSneakEvent;
import org.bukkit.inventory.InventoryView;

import javax.inject.Inject;
Expand Down Expand Up @@ -90,6 +94,8 @@ public class PlayerListener implements Listener {
private PermissionsManager permissionsManager;
@Inject
private QuickCommandsProtectionManager quickCommandsProtectionManager;
@Inject
private SpectateLoginService spectateLoginService;

// Lowest priority to apply fast protection checks
@EventHandler(priority = EventPriority.LOWEST)
Expand Down Expand Up @@ -375,6 +381,36 @@ public void onPlayerRespawn(PlayerRespawnEvent event) {
if (spawn != null && spawn.getWorld() != null) {
event.setRespawnLocation(spawn);
}

if (settings.getProperty(RestrictionSettings.SPECTATE_STAND_LOGIN)
|| spectateLoginService.hasStand(event.getPlayer())) {
// If a player is dead, no stand will be created for him
// therefore we can be sure that there will not be two stands
bukkitService.runTaskLater(() -> {
spectateLoginService.createStand(event.getPlayer());
}, 1L);
}
}

@EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST)
public void onToggleSneak(PlayerToggleSneakEvent event) {
if (listenerService.shouldCancelEvent(event.getPlayer())
&& (settings.getProperty(RestrictionSettings.SPECTATE_STAND_LOGIN)
|| spectateLoginService.hasStand(event.getPlayer()))) {
event.setCancelled(true);
}
}

@EventHandler(ignoreCancelled = true, priority = EventPriority.HIGHEST)
public void onTeleport(PlayerTeleportEvent event) {
if (listenerService.shouldCancelEvent(event.getPlayer())
&& event.getCause() == PlayerTeleportEvent.TeleportCause.SPECTATE
&& event.getPlayer().getGameMode() == GameMode.SPECTATOR
&& (settings.getProperty(RestrictionSettings.SPECTATE_STAND_LOGIN)
|| spectateLoginService.hasStand(event.getPlayer()))) {
spectateLoginService.updateTarget(event.getPlayer());
event.setCancelled(true);
}
}

/*
Expand Down Expand Up @@ -508,4 +544,5 @@ public void onPlayerInventoryClick(InventoryClickEvent event) {
event.setCancelled(true);
}
}

}
16 changes: 13 additions & 3 deletions src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import fr.xephi.authme.service.CommonService;
import fr.xephi.authme.service.PluginHookService;
import fr.xephi.authme.service.SessionService;
import fr.xephi.authme.service.SpectateLoginService;
import fr.xephi.authme.service.ValidationService;
import fr.xephi.authme.service.bungeecord.BungeeSender;
import fr.xephi.authme.service.bungeecord.MessageType;
Expand All @@ -39,7 +40,7 @@
* Asynchronous process for when a player joins.
*/
public class AsynchronousJoin implements AsynchronousProcess {

private final ConsoleLogger logger = ConsoleLoggerFactory.get(AsynchronousJoin.class);

@Inject
Expand Down Expand Up @@ -81,6 +82,9 @@ public class AsynchronousJoin implements AsynchronousProcess {
@Inject
private ProxySessionManager proxySessionManager;

@Inject
private SpectateLoginService spectateLoginService;

AsynchronousJoin() {
}

Expand Down Expand Up @@ -173,7 +177,7 @@ private void handlePlayerWithUnmetNameRestriction(Player player, String ip) {
* Performs various operations in sync mode for an unauthenticated player (such as blindness effect and
* limbo player creation).
*
* @param player the player to process
* @param player the player to process
* @param isAuthAvailable true if the player is registered, false otherwise
*/
private void processJoinSync(Player player, boolean isAuthAvailable) {
Expand All @@ -191,6 +195,13 @@ private void processJoinSync(Player player, boolean isAuthAvailable) {
int blindTimeOut = (registrationTimeout <= 0) ? 99999 : registrationTimeout;
player.addPotionEffect(new PotionEffect(PotionEffectType.BLINDNESS, blindTimeOut, 2));
}

if (service.getProperty(RestrictionSettings.SPECTATE_STAND_LOGIN)) {
// The delay is necessary in order to make sure that the player is teleported to spawn
Copy link
Member

Choose a reason for hiding this comment

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

What about listening at the player spawn/respawn event?

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 the armor stand have to be deleted if the player disconnects as well?

Copy link
Author

Choose a reason for hiding this comment

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

What about listening at the player spawn/respawn event?

Looks like this will also require a delay, because at this stage it's impossible to change the player gamemode

Copy link
Author

Choose a reason for hiding this comment

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

  • doesn't the armor stand have to be deleted if the player disconnects as well?

Armorstands are removed if an unauthorized player leaves the server

// and after authorization appears in the same place
bukkitService.runTaskLater(() -> spectateLoginService.createStand(player), 1L);
}

commandManager.runCommandsOnJoin(player);
});
}
Expand All @@ -201,7 +212,6 @@ private void processJoinSync(Player player, boolean isAuthAvailable) {
*
* @param player the player to verify
* @param ip the ip address of the player
*
* @return true if the verification is OK (no infraction), false if player has been kicked
*/
private boolean validatePlayerCountForIp(final Player player, String ip) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@
import fr.xephi.authme.service.BukkitService;
import fr.xephi.authme.service.CommonService;
import fr.xephi.authme.service.JoinMessageService;
import fr.xephi.authme.service.SpectateLoginService;
import fr.xephi.authme.service.TeleportationService;
import fr.xephi.authme.service.bungeecord.BungeeSender;
import fr.xephi.authme.settings.WelcomeMessageConfiguration;
import fr.xephi.authme.settings.commandconfig.CommandManager;
import fr.xephi.authme.settings.properties.RegistrationSettings;
import fr.xephi.authme.settings.properties.RestrictionSettings;
import org.bukkit.entity.Player;
import org.bukkit.potion.PotionEffectType;

Expand Down Expand Up @@ -57,6 +59,9 @@ public class ProcessSyncPlayerLogin implements SynchronousProcess {
@Inject
private PermissionsManager permissionsManager;

@Inject
private SpectateLoginService spectateLoginService;

ProcessSyncPlayerLogin() {
}

Expand Down Expand Up @@ -98,6 +103,11 @@ public void processPlayerLogin(Player player, boolean isFirstLogin, List<String>
player.removePotionEffect(PotionEffectType.BLINDNESS);
}

if (commonService.getProperty(RestrictionSettings.SPECTATE_STAND_LOGIN)
|| spectateLoginService.hasStand(player)) {
spectateLoginService.removeStand(player);
}

// The Login event now fires (as intended) after everything is processed
bukkitService.callEvent(new LoginEvent(player));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import fr.xephi.authme.process.SynchronousProcess;
import fr.xephi.authme.service.BukkitService;
import fr.xephi.authme.service.CommonService;
import fr.xephi.authme.service.SpectateLoginService;
import fr.xephi.authme.service.TeleportationService;
import fr.xephi.authme.settings.commandconfig.CommandManager;
import fr.xephi.authme.settings.properties.RegistrationSettings;
Expand Down Expand Up @@ -44,6 +45,9 @@ public class ProcessSyncPlayerLogout implements SynchronousProcess {
@Inject
private CommandManager commandManager;

@Inject
private SpectateLoginService spectateLoginService;

ProcessSyncPlayerLogout() {
}

Expand All @@ -65,6 +69,10 @@ public void processSyncLogout(Player player) {

service.send(player, MessageKey.LOGOUT_SUCCESS);
logger.info(player.getName() + " logged out");

if (service.getProperty(RestrictionSettings.SPECTATE_STAND_LOGIN)) {
spectateLoginService.createStand(player);
}
}

private void applyLogoutEffect(Player player) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,29 @@

import fr.xephi.authme.data.limbo.LimboService;
import fr.xephi.authme.process.SynchronousProcess;
import fr.xephi.authme.service.CommonService;
import fr.xephi.authme.service.SpectateLoginService;
import fr.xephi.authme.settings.commandconfig.CommandManager;
import fr.xephi.authme.settings.properties.RestrictionSettings;
import org.bukkit.entity.Player;

import javax.inject.Inject;


public class ProcessSyncPlayerQuit implements SynchronousProcess {

@Inject
private CommonService service;

@Inject
private LimboService limboService;

@Inject
private CommandManager commandManager;

@Inject
private SpectateLoginService spectateLoginService;

/**
* Processes a player having quit.
*
Expand All @@ -26,6 +35,11 @@ public void processSyncQuit(Player player, boolean wasLoggedIn) {
if (wasLoggedIn) {
commandManager.runCommandsOnLogout(player);
} else {
if (service.getProperty(RestrictionSettings.SPECTATE_STAND_LOGIN)
|| spectateLoginService.hasStand(player)) {
spectateLoginService.removeStand(player);
}

limboService.restoreData(player);
player.saveData(); // #1238: Speed is sometimes not restored properly
}
Expand Down
104 changes: 104 additions & 0 deletions src/main/java/fr/xephi/authme/service/SpectateLoginService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package fr.xephi.authme.service;

import fr.xephi.authme.settings.properties.RestrictionSettings;
import org.bukkit.GameMode;
import org.bukkit.Location;
import org.bukkit.entity.ArmorStand;
import org.bukkit.entity.Player;

import javax.inject.Inject;
import java.util.HashMap;
import java.util.Map;

/**
* Sets the player gamemode to Spectator, puts the player in an invisible armorstand and fixes the direction of the view
*/
public class SpectateLoginService {
Copy link
Member

Choose a reason for hiding this comment

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

Good name! Allows us to remember that this will set players to SPECTATOR game mode :)

Copy link
Author

Choose a reason for hiding this comment

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

Added a description of the class behavior


private Map<Player, ArmorStand> armorStands = new HashMap<>();
private Map<Player, GameMode> gameModeMap = new HashMap<>();

@Inject
private CommonService service;

/**
* Creates a stand for the player
*
* @param player the player
*/
public void createStand(Player player) {
if (player.isDead()) {
return;
}
Location location = player.getLocation();
ArmorStand stand = spawnStand(location);

armorStands.put(player, stand);
gameModeMap.put(player, player.getGameMode());
Copy link
Member

Choose a reason for hiding this comment

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

What if the server shuts down while players are not logged in? I guess they'll be saved with the spectator game mode, and next time when they'll join, they'll already have spectator game mode and won't be able to leave it. I think this is quite relevant because it's not only an inconvenience to a player, it's giving them access to a game mode that many servers don't want to give to players...
I could imagine trying to make use of the limbo player stuff or to maybe have an additional setting that will set players to survival instead of spectator when the code would want to. It's ugly but we've had to add a few of those failsafe properties in the past and people seem happy with it

Copy link
Author

Choose a reason for hiding this comment

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

Apparently, when the plugin disabled, I can call a function that will remove all armorstands and return the players to their previous state. Is it possible to implement it this way?

Copy link
Author

Choose a reason for hiding this comment

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

Could you please tell me if I can call SpectateLoginService in onDisable directly to remove armorstands, or is this not a good practice?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the late reply.
I don't see any better way. I think you could get the SpecateLoginService with injector.getIfAvailable(SpectateLoginService.class) in AuthMe#onDisable. But still, when a server unexpectedly shuts down (if it crashes or its process is terminated abruptly) we have the problem that players are potentially in spectator mode, and there are armor stands that no one tracks anymore. I don't know if the armor stands are an issue (I have zero experience with the way you spawn them).
Wondering if we should have a property that specifies what game mode a player should change to if after authentication, we would put him into spectate game mode and/or to add the game mode to the LimboPlayer. I can elaborate on the whole limbo player thing if it helps

Copy link
Author

Choose a reason for hiding this comment

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

Untraceable armorstand is not as critical as players in Spectator gamemode. It would be really cool if it could be done with LimboPlayer

Copy link
Member

Choose a reason for hiding this comment

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

What about adding a "limboArmorStandUUID" field to LimboPlayer?

Copy link
Author

Choose a reason for hiding this comment

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

Do you mean adding a way to keep track of whether a player is assigned an armorstand?

Copy link
Member

Choose a reason for hiding this comment

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

@mrchefran yes, exactly


player.setGameMode(GameMode.SPECTATOR);
player.setSpectatorTarget(stand);
}

/**
* Updates spectator target for the player
*
* @param player the player
*/
public void updateTarget(Player player) {
ArmorStand stand = armorStands.get(player);
if (stand != null) {
player.setSpectatorTarget(stand);
}
}

/**
* Removes the player's stand and deletes effects
*
* @param player the player
*/
public void removeStand(Player player) {
ArmorStand stand = armorStands.get(player);
if (stand != null) {

stand.remove();
player.setSpectatorTarget(null);
player.setGameMode(gameModeMap.get(player));

gameModeMap.remove(player);
armorStands.remove(player);
}
}

/**
* Removes all armorstands and restores player gamemode
*/
public void removeArmorstands() {
for (Player player : armorStands.keySet()) {
removeStand(player);
}

gameModeMap.clear();
armorStands.clear();
}

public boolean hasStand(Player player) {
return armorStands.containsKey(player);
}

private ArmorStand spawnStand(Location loc) {
double pitch = service.getProperty(RestrictionSettings.HEAD_PITCH);
double yaw = service.getProperty(RestrictionSettings.HEAD_YAW);
Location location = new Location(loc.getWorld(), loc.getX(), loc.getY(), loc.getBlockZ(),
(float) yaw, (float) pitch);

ArmorStand stand = location.getWorld().spawn(location, ArmorStand.class);
Copy link
Member

Choose a reason for hiding this comment

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

Does this cause issues if two players have the same location?

Copy link
Author

Choose a reason for hiding this comment

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

This does not affect the position of the stand in any way. The only thing is that players can see the stands of others if their view is directed low enough. The players don't see each other

Copy link
Member

Choose a reason for hiding this comment

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

So the first armor stand doesn't get replaced by the second one when the same location is used?

Copy link
Author

Choose a reason for hiding this comment

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

No, it won't be replaced


stand.setGravity(false);
stand.setAI(false);
stand.setInvisible(true);

return stand;
}

}
Loading