diff --git a/TODO.md b/TODO.md index 4d361a4..f7fb203 100644 --- a/TODO.md +++ b/TODO.md @@ -14,7 +14,7 @@ | MET-010 | Done | Refactor plugin logging to use JavaPlugin.getLogger(). | | MET-011 | Done | Refactor event listeners and registration to use Listener, @EventHandler, and the modern plugin manager API. | | MET-012 | Done | Replace deprecated configuration APIs with the modern Bukkit/Paper configuration API, including load, save, and reload behavior. | -| MET-013 | Ready | Replace Player-keyed storage with UUID-based storage wherever player identity is persisted or tracked. | +| MET-013 | Done | Replace Player-keyed storage with UUID-based storage wherever player identity is persisted or tracked. | | MET-014 | Ready | Review player lookup logic and use exact or otherwise explicit name-matching behavior where appropriate. | | MET-015 | Ready | Refactor remaining code that uses deprecated or removed Bukkit, Spigot, or Paper APIs so it is compatible with the target Paper API. | | MET-016 | Ready | Add fast unit tests for plot and home allocation, reservation validation, player lookup and UUID-based ownership, configuration persistence, command validation, and event behavior. Keep domain rules independent of a running Minecraft server and mock external plugin boundaries as needed. | diff --git a/src/main/java/com/majinnaibu/bukkitplugins/metropolis/MetropolisPlugin.java b/src/main/java/com/majinnaibu/bukkitplugins/metropolis/MetropolisPlugin.java index a5a0114..5b8b543 100644 --- a/src/main/java/com/majinnaibu/bukkitplugins/metropolis/MetropolisPlugin.java +++ b/src/main/java/com/majinnaibu/bukkitplugins/metropolis/MetropolisPlugin.java @@ -7,6 +7,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Set; +import java.util.UUID; import org.bukkit.Location; import org.bukkit.Material; @@ -66,9 +67,9 @@ public class MetropolisPlugin extends JavaPlugin { public RegionManager regionManager = null; private List _occupiedPlots; - private HashMap> _ownedPlots; - private HashMap _userOverrides; - private HashMap _currentHomes; + private HashMap> _ownedPlots; + private HashMap _userOverrides; + private HashMap _currentHomes; private PlayerJoinListener _playerJoinListener = null; @@ -117,9 +118,9 @@ public class MetropolisPlugin extends JavaPlugin { public void onEnable() { pdf = getDescription(); - _ownedPlots = new HashMap>(); - _userOverrides = new HashMap(); - _currentHomes = new HashMap(); + _ownedPlots = new HashMap>(); + _userOverrides = new HashMap(); + _currentHomes = new HashMap(); loadCurrentHomes(); if(DEBUG){getLogger().info("Checking config");} @@ -311,11 +312,22 @@ public class MetropolisPlugin extends JavaPlugin { } _currentHomes.clear(); - for(String username : homes.getKeys(false)){ - int homeNumber = homes.getInt(username, 0); - if (homeNumber > 0) { - _currentHomes.put(username, homeNumber); + boolean migratedLegacyNames = false; + for(String ownerKey : homes.getKeys(false)){ + UUID ownerId; + try { + ownerId = UUID.fromString(ownerKey); + } catch (IllegalArgumentException ex) { + ownerId = getServer().getOfflinePlayer(ownerKey).getUniqueId(); + migratedLegacyNames = true; } + int homeNumber = homes.getInt(ownerKey, 0); + if (homeNumber > 0) { + _currentHomes.put(ownerId, homeNumber); + } + } + if (migratedLegacyNames) { + saveCurrentHomes(); } } @@ -350,8 +362,9 @@ public class MetropolisPlugin extends JavaPlugin { } } + UUID playerId = getServer().getOfflinePlayer(username).getUniqueId(); UserOverride override = new UserOverride(username, plotMultiplier, maxPlots); - _userOverrides.put(username, override); + _userOverrides.put(playerId, override); } } } @@ -465,12 +478,12 @@ public class MetropolisPlugin extends JavaPlugin { ProtectedCuboidRegion cuboidRegion = (ProtectedCuboidRegion) region; if(cuboidRegion.getId().startsWith("h_")){ PlayerHome home = PlayerHome.get(region); - if(!_currentHomes.containsKey(home.getPlayerName())) + if(!_currentHomes.containsKey(home.getPlayerId())) { - _currentHomes.put(home.getPlayerName(), home.getNumber()); + _currentHomes.put(home.getPlayerId(), home.getNumber()); } _occupiedPlots.add(home); - addOwnedPlot(home.getPlayerName(), home); + addOwnedPlot(home.getPlayerId(), home); }else if(cuboidRegion.getId().startsWith("r_")){ _occupiedPlots.add(Plot.get(cuboidRegion)); } @@ -480,14 +493,14 @@ public class MetropolisPlugin extends JavaPlugin { size=calculateCitySize(); } - private void addOwnedPlot(String substring, Plot plot) { - if(_ownedPlots.containsKey(substring)){ - List plots = _ownedPlots.get(substring); + private void addOwnedPlot(UUID playerId, Plot plot) { + if(_ownedPlots.containsKey(playerId)){ + List plots = _ownedPlots.get(playerId); plots.add(plot); }else{ List plots = new ArrayList(); plots.add(plot); - _ownedPlots.put(substring, plots); + _ownedPlots.put(playerId, plots); } } @@ -499,16 +512,23 @@ public class MetropolisPlugin extends JavaPlugin { public PlayerHome getPlayerHome(Player player) { PlayerHome home = null; - String regionName = "h_" + player.getName(); + int homeNumber = _currentHomes.getOrDefault(player.getUniqueId(), 1); + String regionName = String.format("h_%d_%s", homeNumber, player.getUniqueId()); ProtectedRegion homeRegion = regionManager.getRegion(regionName); if(homeRegion == null){ + PlayerHome existingHome = getOwnedHome(player.getUniqueId(), homeNumber); + if (existingHome != null) { + existingHome.setPlayerName(player.getName()); + return existingHome; + } if(DEBUG){ getLogger().info(String.format("Creating home for player %s", player.getName())); } - home = generateHome(player.getName()); + home = generateHome(player); }else{ home = new PlayerHome(homeRegion); + home.setPlayerName(player.getName()); } return home; @@ -841,25 +861,36 @@ public class MetropolisPlugin extends JavaPlugin { return (cuboid.minZ - roadWidth/2)/gridSizeZ; } - private void setHomeOccupied(String owner, BlockVector minimumPoint, BlockVector maximumPoint) { - - PlayerHome home = new PlayerHome(owner, minimumPoint, maximumPoint); + private void setHomeOccupied(UUID ownerId, String ownerName, int homeNumber, BlockVector minimumPoint, BlockVector maximumPoint) { + PlayerHome home = new PlayerHome(ownerId, ownerName, homeNumber, minimumPoint, maximumPoint); if(!_occupiedPlots.contains(home)){ _occupiedPlots.add(home); + addOwnedPlot(ownerId, home); } } public PlayerHome generateHome(String playerName) { - int multiplier = getPlotMultiplier(playerName); + return generateHome(getServer().getOfflinePlayer(playerName)); + } + + public PlayerHome generateHome(OfflinePlayer player) { + UUID playerId = player.getUniqueId(); + String playerName = player.getName() == null ? playerId.toString() : player.getName(); + int homeNumber = _currentHomes.getOrDefault(playerId, 1); + int multiplier = getPlotMultiplier(playerId); if(DEBUG){getLogger().info(String.format("Generating home for %s", playerName));} Cuboid homeCuboid = null; ProtectedRegion phomeRegion = null; - String regionName = "h_1_" + playerName; + String regionName = getHomeRegionName(playerId, homeNumber); phomeRegion = regionManager.getRegion(regionName); if(phomeRegion != null){ return PlayerHome.get(phomeRegion); } + PlayerHome existingHome = getOwnedHome(playerId, homeNumber); + if (existingHome != null) { + return existingHome; + } homeCuboid = findNextUnownedHomeRegion(multiplier); @@ -875,7 +906,7 @@ public class MetropolisPlugin extends JavaPlugin { newHomeRegion.setFlag(DefaultFlag.TNT, StateFlag.State.DENY); DefaultDomain d = newHomeRegion.getOwners(); - d.addPlayer(playerName); + d.addPlayer(playerId); newHomeRegion.setPriority(1); regionManager.addRegion(newHomeRegion); @@ -902,7 +933,9 @@ public class MetropolisPlugin extends JavaPlugin { newHomeRegion.getMaximumPoint().getBlockZ() )); - setHomeOccupied(playerName, newHomeRegion.getMinimumPoint(), newHomeRegion.getMaximumPoint()); + setHomeOccupied(playerId, playerName, homeNumber, newHomeRegion.getMinimumPoint(), newHomeRegion.getMaximumPoint()); + _currentHomes.putIfAbsent(playerId, homeNumber); + saveCurrentHomes(); createRoads(homeCuboid); @@ -917,7 +950,9 @@ public class MetropolisPlugin extends JavaPlugin { if(DEBUG){getLogger().info(String.format("Done generating home for %s", playerName));} - return new PlayerHome(newHomeRegion); + PlayerHome home = new PlayerHome(newHomeRegion); + home.setPlayerName(playerName); + return home; } private void generateSign(Cuboid plotCuboid, String playerName) { @@ -1006,9 +1041,9 @@ public class MetropolisPlugin extends JavaPlugin { } } - public int getNumPlots(String name) { - if(_ownedPlots.containsKey(name)){ - List plots = _ownedPlots.get(name); + public int getNumPlots(UUID playerId) { + if(_ownedPlots.containsKey(playerId)){ + List plots = _ownedPlots.get(playerId); if(plots == null){ return 0; }else{ @@ -1019,22 +1054,21 @@ public class MetropolisPlugin extends JavaPlugin { } } - public int getMaxPlots(String name) { - if(_userOverrides.containsKey(name)){ - return _userOverrides.get(name).getMaxPlots(); + public int getMaxPlots(UUID playerId) { + if(_userOverrides.containsKey(playerId)){ + return _userOverrides.get(playerId).getMaxPlots(); }else{ return _maxPlots; } } public void assignPlot(OfflinePlayer player) { - //PlayerHome home = generateHome(player.getName()); - generateHome(player.getName()); + generateHome(player); } - private int getPlotMultiplier(String name) { - if(_userOverrides.containsKey(name)){ - return _userOverrides.get(name).getPlotMultiplier(); + private int getPlotMultiplier(UUID playerId) { + if(_userOverrides.containsKey(playerId)){ + return _userOverrides.get(playerId).getPlotMultiplier(); }else{ return _plotMultiplier; } @@ -1073,9 +1107,9 @@ public class MetropolisPlugin extends JavaPlugin { return null; } - public boolean homeExists(String playerName, int homeNumber) { + public boolean homeExists(UUID playerId, int homeNumber) { for(Plot plot: _occupiedPlots){ - if(plot.getRegionName().equalsIgnoreCase(String.format("h_%d_%s", homeNumber, playerName))){ + if(plot instanceof PlayerHome home && home.getPlayerId().equals(playerId) && home.getNumber() == homeNumber){ return true; } } @@ -1083,15 +1117,39 @@ public class MetropolisPlugin extends JavaPlugin { return false; } - public void setHome(String name, int newHomeNumber) { - _currentHomes.put(name, newHomeNumber); + public void setHome(UUID playerId, int newHomeNumber) { + _currentHomes.put(playerId, newHomeNumber); saveCurrentHomes(); } + public String getHomeRegionName(UUID playerId, int homeNumber) { + PlayerHome existingHome = getOwnedHome(playerId, homeNumber); + return existingHome == null + ? String.format("h_%d_%s", homeNumber, playerId) + : existingHome.getRegionName(); + } + + public String getCurrentHomeRegionName(UUID playerId) { + return getHomeRegionName(playerId, _currentHomes.getOrDefault(playerId, 1)); + } + + private PlayerHome getOwnedHome(UUID playerId, int homeNumber) { + List plots = _ownedPlots.get(playerId); + if (plots == null) { + return null; + } + for (Plot plot : plots) { + if (plot instanceof PlayerHome home && home.getNumber() == homeNumber) { + return home; + } + } + return null; + } + private void saveCurrentHomes() { File homesFile = new File(getDataFolder(), "currentHomes.yml"); YamlConfiguration homes = new YamlConfiguration(); - _currentHomes.forEach(homes::set); + _currentHomes.forEach((playerId, homeNumber) -> homes.set(playerId.toString(), homeNumber)); try { getDataFolder().mkdirs(); homes.save(homesFile); diff --git a/src/main/java/com/majinnaibu/bukkitplugins/metropolis/PlayerHome.java b/src/main/java/com/majinnaibu/bukkitplugins/metropolis/PlayerHome.java index ad1e01c..f5f0841 100644 --- a/src/main/java/com/majinnaibu/bukkitplugins/metropolis/PlayerHome.java +++ b/src/main/java/com/majinnaibu/bukkitplugins/metropolis/PlayerHome.java @@ -1,8 +1,14 @@ package com.majinnaibu.bukkitplugins.metropolis; +import java.util.Set; +import java.util.UUID; + import javax.persistence.Entity; import javax.persistence.Table; +import org.bukkit.Bukkit; +import org.bukkit.OfflinePlayer; + import com.avaje.ebean.validation.NotNull; import com.sk89q.worldedit.BlockVector; import com.sk89q.worldguard.protection.regions.ProtectedCuboidRegion; @@ -11,48 +17,56 @@ import com.sk89q.worldguard.protection.regions.ProtectedRegion; @Entity() @Table(name="Metropolis_PlayerHome") public class PlayerHome extends Plot{ + private UUID playerId; @NotNull private String playerName; + public UUID getPlayerId(){return this.playerId;} public String getPlayerName(){return this.playerName;} public void setPlayerName(String playerName){this.playerName = playerName;} private int number; - public PlayerHome(String owner, BlockVector min, BlockVector max) { - super("h_" + owner, min, max); - this.playerName = owner; + public PlayerHome(UUID ownerId, String ownerName, int homeNumber, BlockVector min, BlockVector max) { + super(String.format("h_%d_%s", homeNumber, ownerId), min, max); + this.playerId = ownerId; + this.playerName = ownerName; + this.number = homeNumber; } public PlayerHome() { + this.playerId = null; this.playerName = ""; } public PlayerHome(ProtectedRegion homeRegion){ - try{ - String rname = homeRegion.getId(); - - if(rname.startsWith("h_")){ - int secondUnderscore = rname.indexOf('_', 2); - if(secondUnderscore > 2){ - try{ - this.number = Integer.parseInt(rname.substring(2, secondUnderscore)); - this.playerName = rname.substring(secondUnderscore+1); - }catch(Exception ex){ - this.number = 0; - } - }else{ - this.number = 0; - this.playerName = rname.substring(2); - } - - setCuboid(new Cuboid(homeRegion.getMinimumPoint(), homeRegion.getMaximumPoint())); - } - else{ - throw new RuntimeException("Method not implemented."); - } - }catch(Exception ex){ - throw new RuntimeException("Method not implemented.", ex); + String regionName = homeRegion.getId(); + if (!regionName.startsWith("h_")) { + throw new IllegalArgumentException("Not a Metropolis home region: " + regionName); } + String regionOwner = regionName.substring(2); + int separator = regionOwner.indexOf('_'); + if (separator > 0) { + try { + this.number = Integer.parseInt(regionOwner.substring(0, separator)); + regionOwner = regionOwner.substring(separator + 1); + } catch (NumberFormatException ex) { + this.number = 1; + } + } else { + this.number = 1; + } + try { + this.playerId = UUID.fromString(regionOwner); + OfflinePlayer offlinePlayer = Bukkit.getOfflinePlayer(playerId); + this.playerName = offlinePlayer.getName() == null ? regionOwner : offlinePlayer.getName(); + } catch (IllegalArgumentException ex) { + Set ownerIds = homeRegion.getOwners().getUniqueIds(); + this.playerId = ownerIds.isEmpty() + ? Bukkit.getOfflinePlayer(regionOwner).getUniqueId() + : ownerIds.iterator().next(); + this.playerName = regionOwner; + } + setCuboid(new Cuboid(homeRegion.getMinimumPoint(), homeRegion.getMaximumPoint())); } @Override @@ -63,7 +77,7 @@ public class PlayerHome extends Plot{ PlayerHome otherPlayerHome = (PlayerHome)other; - if(!this.playerName.equals(otherPlayerHome.playerName)){ + if(!java.util.Objects.equals(this.playerId, otherPlayerHome.playerId)){ return false; } diff --git a/src/main/java/com/majinnaibu/bukkitplugins/metropolis/commands/MetropolisHomeAcquire.java b/src/main/java/com/majinnaibu/bukkitplugins/metropolis/commands/MetropolisHomeAcquire.java index ea84cf4..716fe97 100644 --- a/src/main/java/com/majinnaibu/bukkitplugins/metropolis/commands/MetropolisHomeAcquire.java +++ b/src/main/java/com/majinnaibu/bukkitplugins/metropolis/commands/MetropolisHomeAcquire.java @@ -23,7 +23,7 @@ public class MetropolisHomeAcquire implements CommandExecutor { Player player = (Player) sender; - if(_plugin.getNumPlots(player.getName()) >= _plugin.getMaxPlots(player.getName())){ + if(_plugin.getNumPlots(player.getUniqueId()) >= _plugin.getMaxPlots(player.getUniqueId())){ sender.sendMessage("You cannot have any more plots"); return false; } diff --git a/src/main/java/com/majinnaibu/bukkitplugins/metropolis/commands/MetropolisHomeEvictCommand.java b/src/main/java/com/majinnaibu/bukkitplugins/metropolis/commands/MetropolisHomeEvictCommand.java index 0ffd648..1b73687 100644 --- a/src/main/java/com/majinnaibu/bukkitplugins/metropolis/commands/MetropolisHomeEvictCommand.java +++ b/src/main/java/com/majinnaibu/bukkitplugins/metropolis/commands/MetropolisHomeEvictCommand.java @@ -34,15 +34,15 @@ public class MetropolisHomeEvictCommand implements CommandExecutor { return false; } - ProtectedRegion region = _plugin.getRegion(String.format("h_%s", player.getName())); + ProtectedRegion region = _plugin.getRegion(_plugin.getCurrentHomeRegionName(player.getUniqueId())); if(region == null){ sender.sendMessage(String.format("The player {%s} has no home to be evicted from.")); return false; } //remove the player as owner and/or member of the region - region.getMembers().removePlayer(playerName); - region.getOwners().removePlayer(playerName); + region.getMembers().removePlayer(player.getUniqueId()); + region.getOwners().removePlayer(player.getUniqueId()); //if the region has no owners delete the region if(region.getMembers().size() == 0 && region.getOwners().size() == 0){ diff --git a/src/main/java/com/majinnaibu/bukkitplugins/metropolis/commands/MetropolisHomeMoveCommand.java b/src/main/java/com/majinnaibu/bukkitplugins/metropolis/commands/MetropolisHomeMoveCommand.java index 407f681..67edbd8 100644 --- a/src/main/java/com/majinnaibu/bukkitplugins/metropolis/commands/MetropolisHomeMoveCommand.java +++ b/src/main/java/com/majinnaibu/bukkitplugins/metropolis/commands/MetropolisHomeMoveCommand.java @@ -41,11 +41,11 @@ public class MetropolisHomeMoveCommand implements CommandExecutor { return false; } - if(player == null || !_plugin.homeExists(player.getName(), newHomeNumber)){ + if(player == null || !_plugin.homeExists(player.getUniqueId(), newHomeNumber)){ return false; } - _plugin.setHome(player.getName(), newHomeNumber); + _plugin.setHome(player.getUniqueId(), newHomeNumber); return true; }