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 #2745

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
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
Next Next commit
Commit base by mrchefran.
  • Loading branch information
Bagietka committed Aug 31, 2023
commit 5fd543cb35a4faf05a0e7aae77573e2c320c3724
5 changes: 5 additions & 0 deletions src/main/java/fr/xephi/authme/AuthMe.java
Original file line number Diff line number Diff line change
@@ -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;
@@ -69,6 +70,7 @@ public class AuthMe extends JavaPlugin {
private Injector injector;
private BackupService backupService;
private ConsoleLogger logger;
private SpectateLoginService spectateLoginService;

/**
* Constructor.
@@ -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);
@@ -316,6 +319,8 @@ public void onDisable() {
// Wait for tasks and close data source
new TaskCloser(this, database).run();

spectateLoginService.removeArmorstands();
Copy link
Member

@ljacqu ljacqu Sep 3, 2023

Choose a reason for hiding this comment

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

Potential null pointer if the plugin crashes before this service is set on L251

The proper way to do this is to not have the field, but to use another method from the injector (not sure what methods exist by heart, but if there's something that returns a singleton if it's been registered, that would be it, otherwise maybe injector.createIfHasDependencies though it's not ideal to still potentially create it at this point)

Copy link
Author

Choose a reason for hiding this comment

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

Fixed it.

Copy link
Member

Choose a reason for hiding this comment

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

  1. The field on the class is no longer needed (an IDE shows an unused warning)
  2. injector.createIfHasDependencies(SpectateLoginService.class) might return null, so the issue of a potential null pointer still remains. And I think it would be better to use injector#getIfAvailable

Copy link
Author

Choose a reason for hiding this comment

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

Done.


// Disabled correctly
Consumer<String> infoLogMethod = logger == null ? getLogger()::info : logger::info;
infoLogMethod.accept("AuthMe " + this.getDescription().getVersion() + " disabled!");
34 changes: 34 additions & 0 deletions src/main/java/fr/xephi/authme/listener/PlayerListener.java
Original file line number Diff line number Diff line change
@@ -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;
@@ -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;
@@ -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;
@@ -91,6 +95,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)
@@ -376,6 +382,34 @@ 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);
Copy link
Member

@ljacqu ljacqu Sep 3, 2023

Choose a reason for hiding this comment

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

I can't confirm the comment about stands not being created if a player is dead but maybe I'm missing the relevant point, but interestingly the logic here specifies a stand should be created if the setting is enabled, or if the player has a stand? I don't think that makes much sense.

Copy link
Author

Choose a reason for hiding this comment

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

Removed unnecessary comment.

Copy link
Member

Choose a reason for hiding this comment

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

Now an armor stand is created for a player every time he respawns, regardless of whether an armor stand already exists or not

Copy link
Author

Choose a reason for hiding this comment

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

Done.

}
}

@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);
}
}

/*
11 changes: 11 additions & 0 deletions src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java
Original file line number Diff line number Diff line change
@@ -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;
@@ -83,6 +84,9 @@ public class AsynchronousJoin implements AsynchronousProcess {
@Inject
private ProxySessionManager proxySessionManager;

@Inject
private SpectateLoginService spectateLoginService;

AsynchronousJoin() {
}

@@ -199,6 +203,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
// and after authorization appears in the same place
bukkitService.runTaskLater(() -> spectateLoginService.createStand(player), 1);
}

commandManager.runCommandsOnJoin(player);
});
}
Original file line number Diff line number Diff line change
@@ -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;

@@ -58,6 +60,9 @@ public class ProcessSyncPlayerLogin implements SynchronousProcess {
@Inject
private PermissionsManager permissionsManager;

@Inject
private SpectateLoginService spectateLoginService;

ProcessSyncPlayerLogin() {
}

@@ -99,6 +104,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));

Original file line number Diff line number Diff line change
@@ -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;
@@ -44,6 +45,9 @@ public class ProcessSyncPlayerLogout implements SynchronousProcess {
@Inject
private CommandManager commandManager;

@Inject
private SpectateLoginService spectateLoginService;

ProcessSyncPlayerLogout() {
}

@@ -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) {
Original file line number Diff line number Diff line change
@@ -2,14 +2,23 @@

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 SpectateLoginService spectateLoginService;

@Inject
private LimboService limboService;

@@ -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
}
103 changes: 103 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,103 @@
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 {

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());

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);

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

return stand;
}

}
Original file line number Diff line number Diff line change
@@ -193,6 +193,22 @@ public final class RestrictionSettings implements SettingsHolder {
public static final Property<Set<String>> UNRESTRICTED_INVENTORIES =
newLowercaseStringSetProperty("settings.unrestrictions.UnrestrictedInventories");

@Comment({
"While in unregistered/logged out state, should players be set to spectator mode and",
"forced to spectate within a spawn point invisible armor stand, for 0 movement and head",
"pitch + yaw? may be more effective than 'allowMovement' at locking the player in place."
})
public static final Property<Boolean> SPECTATE_STAND_LOGIN =
newProperty("settings.restrictions.spectateStandLogin.enabled", false);

@Comment("Head Yaw position for 'spectateStandLogin'.")
public static final Property<Double> HEAD_YAW =
newProperty("settings.restrictions.spectateStandLogin.headYaw", 0.0f);

@Comment("Head Pitch position for 'spectateStandLogin'.")
public static final Property<Double> HEAD_PITCH =
Copy link
Member

@ljacqu ljacqu Sep 3, 2023

Choose a reason for hiding this comment

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

I think this should work with the version of ConfigMe that AuthMe is using (please ignore this suggestion if it doesn't)

Suggested change
public static final Property<Double> HEAD_PITCH =
public static final Property<Float> HEAD_PITCH =

Also the naming is a bit weird, I don't know that RestrictionSettings#HEAD_PITCH refers to the spectate stand feature, and as a user I'm not sure I'll understand what this setting represents. Is it possible to elaborate in the comment? I would make a suggestion but I'm actually not really sure what the yaw & pitch settings are for

Copy link
Author

Choose a reason for hiding this comment

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

I don't know If i understood you correctly, but HEAD_PITCH and HEAD_YAW are under spectateStandLogin so It's obvious what are these settings for.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for changing the comment. I also meant the field name, RestrictionSettings.HEAD_PITCH in the code sounds very generic. Could we maybe name it SPECTATE_HEAD_PITCH, or maybe you have a better idea?

Copy link
Author

Choose a reason for hiding this comment

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

Renamed.

newProperty("settings.restrictions.spectateStandLogin.headPitch", 0.0f);

private RestrictionSettings() {
}