Hey guys. I'm really upset because I'm thinking too much about my code and can't figure out how can I improve it. So I'm here posting my whole Arena Manager code to know if someone has something to say about it. Sorry for my bad english Code: package me.knf.flywars; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Set; import org.bukkit.Bukkit; import org.bukkit.ChatColor; import org.bukkit.DyeColor; import org.bukkit.Location; import org.bukkit.Material; import org.bukkit.entity.Player; import org.bukkit.inventory.Inventory; import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.meta.ItemMeta; import org.bukkit.material.Wool; import me.knf.flywars.utils.VectorDavson; public class ArenaManager { static ArenaManager instance = new ArenaManager(); private SettingsManager sm = SettingsManager.getInstance(); public List<Arena> arenas = new ArrayList<Arena>(); public HashMap<String, Integer> playerArenaListPage = new HashMap<String, Integer>(); public HashMap<String, Inventory> playerArenaListInventory = new HashMap<String, Inventory>(); public HashMap<String, Arena> editingArenaPerPlayer = new HashMap<String, Arena>(); public int maxPages; private ArenaManager() { } public static ArenaManager getInstance() { return instance; } public void CreateArena(Arena a){ sm.getArenas().set("arenas." + a.arenaName + ".slots", a.nSpawnSpots); sm.getArenas().set("arenas." + a.arenaName + ".world", a.world); for(int i = 0; i < a.spawnSpots.size(); i++){ sm.getArenas().set("arenas." + a.arenaName + ".slot_" + (i+1) + ".x", a.spawnSpots.get(i).x); sm.getArenas().set("arenas." + a.arenaName + ".slot_" + (i+1) + ".y", a.spawnSpots.get(i).y); sm.getArenas().set("arenas." + a.arenaName + ".slot_" + (i+1) + ".z", a.spawnSpots.get(i).z); sm.getArenas().set("arenas." + a.arenaName + ".slot_" + (i+1) + ".pitch", a.spawnSpots.get(i).pitch); sm.getArenas().set("arenas." + a.arenaName + ".slot_" + (i+1) + ".yaw", a.spawnSpots.get(i).yaw); } sm.saveArenas(); } public void StartArenaCreation(Player p, String[] args){ Arena a = new Arena(); a.arenaName = args[1]; a.nSpawnSpots = Integer.parseInt(args[2]); a.world = p.getLocation().getWorld().getName(); a.type = args[3].toUpperCase(); editingArenaPerPlayer.put(p.getName(), a); p.sendMessage(ChatColor.GREEN + "To complete the creation you must set every single spawn."); p.sendMessage(ChatColor.GREEN + "Use the command /fw setspawn <SLOT_ID> while standing at the wished place."); } public void SetSpawn(Player p, int i){ Arena a; a = editingArenaPerPlayer.get(p.getName()); if(i > a.nSpawnSpots){ p.sendMessage(ChatColor.RED + "<SLOT_ID> must be a number between 1 and " + a.nSpawnSpots); return; } Location l = p.getLocation(); VectorDavson vec = new VectorDavson(); vec.x = l.getX(); vec.y = l.getY(); vec.z = l.getZ(); vec.pitch = l.getPitch(); vec.yaw = l.getYaw(); vec.id = i; for(VectorDavson v : a.spawnSpots){ if(v.id == i){ a.spawnSpots.remove(v); break; } } a.spawnSpots.add(vec); Collections.sort(a.spawnSpots, (VectorDavson v1, VectorDavson v2) -> v1.id-v2.id); p.sendMessage(ChatColor.GREEN + "Spawn " + i + " was set!"); if(a.spawnSpots.size() == a.nSpawnSpots){ p.sendMessage(ChatColor.GREEN + "All done! Arena '" + a.arenaName + "' is ready."); editingArenaPerPlayer.remove(p.getName()); SaveArena(a); } } public void SaveArena(Arena a){ String prefix = "arenas."+a.arenaName; sm.getArenas().set(prefix+".slots", a.nSpawnSpots); sm.getArenas().set(prefix+".type", a.type); sm.getArenas().set(prefix+".world", a.world); for(VectorDavson v : a.spawnSpots){ sm.getArenas().set(prefix+".spawn"+v.id+"_Location.x" , v.x); sm.getArenas().set(prefix+".spawn"+v.id+"_Location.y" , v.y); sm.getArenas().set(prefix+".spawn"+v.id+"_Location.z" , v.z); sm.getArenas().set(prefix+".spawn"+v.id+"_Location.pitch" , v.pitch); sm.getArenas().set(prefix+".spawn"+v.id+"_Location.yaw" , v.yaw); } sm.saveArenas(); } public void storageAllArenas(){ List<Arena> arenas = new ArrayList<Arena>(); Set<String> keys = sm.getArenas().getConfigurationSection("arenas").getKeys(false); for(String s : keys){ String prefix = "arenas."+s; Arena a = new Arena(); a.arenaName = s; a.nSpawnSpots = sm.getArenas().getInt(prefix+".slots"); a.type = sm.getArenas().getString(prefix+".type"); a.world = sm.getArenas().getString(prefix+".world"); List<VectorDavson> vd = new ArrayList<VectorDavson>(); for(int i = 1; i < a.nSpawnSpots; i++){ VectorDavson v = new VectorDavson(); v.x = sm.getArenas().getDouble(prefix+"slot"+i+"_Location.x"); v.y = sm.getArenas().getDouble(prefix+"slot"+i+"_Location.y"); v.z = sm.getArenas().getDouble(prefix+"slot"+i+"_Location.z"); v.pitch = sm.getArenas().getDouble(prefix+"slot"+i+"_Location.pitch"); v.yaw = sm.getArenas().getDouble(prefix+"slot"+i+"_Location.yaw"); vd.add(v); } a.spawnSpots = vd; arenas.add(a); } this.arenas = arenas; } public void ArenaInv(Player p, String type){ if(type.equalsIgnoreCase("open")){ Inventory _inv = Bukkit.createInventory(null, 45, "Arenas"); playerArenaListPage.put(p.getName(), 0); playerArenaListInventory.put(p.getName(), _inv); p.openInventory(_inv); } Inventory inv = playerArenaListInventory.get(p.getName()); int currentPage = playerArenaListPage.get(p.getName()); if(type.equalsIgnoreCase("next")){ currentPage++; playerArenaListPage.put(p.getName(), currentPage); } else if(type.equalsIgnoreCase("previous")){ currentPage--; playerArenaListPage.put(p.getName(), currentPage); } if(maxPages != 0){ if(currentPage != maxPages-1){ ItemStack next = new ItemStack(Material.ARROW); ItemMeta nextMeta = next.getItemMeta(); nextMeta.setDisplayName("Next"); next.setItemMeta(nextMeta); inv.setItem(41, next); } else{ inv.setItem(41, null); } if(currentPage != 0){ ItemStack previous = new ItemStack(Material.ARROW); ItemMeta previousMeta = previous.getItemMeta(); previousMeta.setDisplayName("Previous"); previous.setItemMeta(previousMeta); inv.setItem(39, previous); } else{ inv.setItem(39, null); } } for(int i = currentPage*36; i < currentPage*36+36; i++){ if(i >= arenas.size()){ inv.setItem(i-currentPage*36, null); continue; } Wool wool; Arena a = arenas.get(i); if(a.status.equalsIgnoreCase("FREE")) wool = new Wool(DyeColor.LIME); else if(a.status.equalsIgnoreCase("BUSY")) wool = new Wool(DyeColor.YELLOW); else wool = new Wool(DyeColor.RED); ItemStack iS = wool.toItemStack(1); ItemMeta im = iS.getItemMeta(); im.setDisplayName(a.arenaName); im.setLore(Arrays.asList(ChatColor.GREEN + "Status: " + a.status, ChatColor.GREEN + "Players: " + a.alivePlayers.size() + "/" + a.nSpawnSpots, ChatColor.GREEN + "Observers: " + a.observers.size(), ChatColor.GREEN + "Arena type: " + a.type)); iS.setItemMeta(im); inv.setItem(i-currentPage*36, iS); } } } PS: It's not done yet, but I'm afraid that something will mess everything up in the future. PPS: Sorry for the long code.
No, it's not broken at all. I just wanna know if I will have problems in the future, since it's gonna be part of a huge project with many minigames (sorry, forgot to mention that).
@LittleUrso Well, it is not following Java naming conventions, and the settings manager might cause issues, but I can't be sure of that without seeing it.
@timtower by "Java naming convertions" you mean starting methods with uppercase and stuff? Sorry for the long code again, but here goes the Settings Manager for you: Code: package me.knf.flywars; import java.io.File; import java.io.IOException; import org.bukkit.Bukkit; import org.bukkit.ChatColor; import org.bukkit.configuration.file.FileConfiguration; import org.bukkit.configuration.file.YamlConfiguration; import org.bukkit.plugin.Plugin; import org.bukkit.plugin.PluginDescriptionFile; public class SettingsManager { private Plugin p; private FileConfiguration config; private File cfile; private FileConfiguration arenas; private File arenasFile; private SettingsManager() { } static SettingsManager instance = new SettingsManager(); public static SettingsManager getInstance() { return instance; } public void setup(Plugin p) { config = p.getConfig(); if (!p.getDataFolder().exists()) { p.getDataFolder().mkdir(); } cfile = new File(p.getDataFolder(), "config.yml"); arenasFile = new File(p.getDataFolder(), "arenasFile.yml"); if(!cfile.exists()){ try { cfile.createNewFile(); } catch (IOException e1) { Bukkit.getServer().getLogger().severe(ChatColor.RED + "Could not create config.yml!"); } } if(!arenasFile.exists()){ try { arenasFile.createNewFile(); } catch (IOException e1) { Bukkit.getServer().getLogger().severe(ChatColor.RED + "Could not create arenasFile.yml!"); } } config = YamlConfiguration.loadConfiguration(cfile); arenas = YamlConfiguration.loadConfiguration(arenasFile); if(!arenas.contains("arenas")){ arenas.createSection("arenas"); saveArenas(); } saveConfig(); saveArenas(); } public FileConfiguration getConfig() { return config; } public void saveConfig() { try { config.save(cfile); } catch (IOException e) { Bukkit.getServer().getLogger().severe(ChatColor.RED + "Could not save config.yml!"); } } public FileConfiguration getArenas() { return arenas; } public void saveArenas() { try { arenas.save(arenasFile); } catch (IOException e) { Bukkit.getServer().getLogger().severe(ChatColor.RED + "Could not save arenas.yml!"); } } public PluginDescriptionFile getDesc() { return p.getDescription(); } }
@LittleUrso Indeed the upper and lowercase stuff yes. And if the setup from this gets called before any methods from the arenamanager get called then you should be fine. I do suggest adding a method to load all arena's though. Or any loading in that regard.
@timtower Yep, setup is actually called before any arenemanager method. I think "storageAllArenas" do what you said and it's called on onEnable (and should be called when a new Arena is registered I guess). I'm pretty happy that until now there are no fatal problems visible for human eyes, thx for the feedback.