ArenaManager

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

 

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

 

@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.