xMVanish - prosty, dzialajacy vanish!

Czesc, przedstawiam wam moj 3 plugin. Jest to plugin na vanisha. Mysle ze moze sie przydac   Jezeli ktos ma pomysl na jakis nie trudny plugin - prosze pisac  

Kliknij tu aby pobrac plugin ;)

Dwa EventHandlery w klasie od komendy, brak wiadomości w configu, brak oddzielnego package dla classy config i totalnie nie rozumiem tego:

    public static Main main;
    private static Main instance;
    public static Main plugin;
    private static Main inst;
    

    public static Main getInstance() {
        return Main.instance;
    }
    
    public static void setInstance(final Main instance) {
        Main.instance = instance;
    }
    
    public static Main getInst() {
        return Main.inst;
    }

    public static Main getPlugin() {
        return Main.plugin;
    }
    

Wystarczy zrobić to raz.

Ogólnie estetyka kodu nie za fajna  Popracuj jeszcze.

29 minut temu, paweU napisał:

Dwa EventHandlery w klasie od komendy, brak wiadomości w configu, brak oddzielnego package dla classy config i totalnie nie rozumiem tego:

    public static Main main;
    private static Main instance;
    public static Main plugin;
    private static Main inst;
    

    public static Main getInstance() {
        return Main.instance;
    }
    
    public static void setInstance(final Main instance) {
        Main.instance = instance;
    }
    
    public static Main getInst() {
        return Main.inst;
    }

    public static Main getPlugin() {
        return Main.plugin;
    }
    

 

Wystarczy zrobić to raz.

Ogólnie estetyka kodu nie za fajna  Popracuj jeszcze.

Dzieki za opinie!

12 minut temu, xMuffionex napisał:

Dzieki za opinie!

Nie ma sprawy

Dobra... To zacznijmy od początku

1. Dlaczego w plugin.yml masz komendy których nawet nie wykorzystujesz? 
2. Package najlepiej nazywaj pl.<twój nick>.<nazwa pluginu> czy coś w tym stylu bo gdy na serwerze będą 2 pluginy o takim samym package'u może to powodować błędy
3. Główna klasa w pluginie powinna się nazywać NazwaPluginuPlugin lub po prostu NazwaPlugin (zamiast nazwy NazwaPluginu oczywiście dajesz nazwę pluginu)
4. WSZYSTKIE nazwy klas powinny się zaczynać z dużej litery
5. Tak jak kolega wyżej wspomniał, po co ci aż 4 instancje w pluginie oraz właściwie po co ci settery do nich?
6. Ogólnie tych wszystkich instancji nie powinno się robić gdyż nie jest to zgodne z singleton  Jeśli w danej klasie potrzebujesz użyć głównej klasy to po prostu przekaż ją w konstruktorze klasy i przypisz do prywatnej i finalnej zmiennej w tej klasie
7. Używaj Bukkitowego loggera czyli Bukkit.getLogger() zamiast System.out.println() 
8. Wiadomości o tym, że plugin się włączył lub wyłączył są całkowicie zbędne, serwer gdy plugin się włącza lub wyłącza sam wyświetla komunikat o tym w konsoli :V
9. Dlaczego masz klasę od configu skoro jej nawet nie wykorzystujesz? 
10. Klasa od configu jest napisana strasznie dziwnie, większość kodu jest zbędna i niepotrzebna
11. Eventy w klasie vanishCommand (po co one się właściwie tam znalazły?) nie będą działać bo klasa w których się znajdują nie implementuje interfejsu Listener, nawet nie są rejestrowane gdziekolwiek
12. Po co tworzysz nową ArrayListe dopiero w constructorze? Możesz to robić przy samej zmiennej :V Sama zmienna globalna powinna być zwykłą listą List a nie ArrayListą, jak zainicjujesz ją new ArrayList<>() to java będzie wiedziała o co chodzi
13. Dlaczego 2 razy tworzysz komendę? Wystarczy dodać aliases dla komendy w plugin.yml

Ogólnie zalecam autorowi nauczenia się podstaw javy np. stąd: https://javastart.pl/baza-wiedzy/java-podstawy-jezyka oraz przeczytanie dokumentacji gdyż kod jest no po prostu beznadziejny, wręcz nie nadaje się do używania gdziekolwiek

@xMuffionex Masz 24h na dodanie skanu, inaczej temat wyląduje w koszu

Umieszczam link do skanu pliku.

Dobra zacznijmy,

- Nazwy klas z małych liter,

    public static Main main;

    public static Main plugin;

- Jakie to ma zastosowanie, wiesz co to konstruktor ?
 Należało to zrobić tworząc konstruktor klasy, oraz funkcję do pobierania, aby nie dało się zmodyfikować, i było estetyczniej?

    public static Main getPlugin() {
        return plugin;
    }

- Wyżej masz już publiczne zmienne, które normalnie pobierasz, więc po co powielać 3 razy ten sam kod?

System.out.println(String.valueOf(this.getName()) + " Zostal wlaczony!");

- Kolego drogi, wiesz może co to jest Logger ?
https://docs.oracle.com/javase/7/docs/api/java/util/logging/Logger.html
W bukkitapi jest bodajże funkcja Bukkit.getLogger()

        Main.main = this;
        Main.inst = this;

    public void onLoad() {
        Main.plugin = this;
    }

 - Zbędne powielanie kodu

public static void setInstance(final Main instance) {

 - Publiczna funkcja do ustawiania instancji, którą powielasz X razy w mainie (chodzi mi o instancję)

    public static Main getInstance() {
        return Main.instance;
    }
    
    public static Main getInst() {
        return Main.inst;
    }

 - 2 funkcje do tego samego, i 2 publiczne funkcje do głównej klasy... 

    public static void loadConfig() {
        try {
            Main.getPlugin().saveDefaultConfig();
            FileConfiguration c = Main.getPlugin().getConfig();
            for (Field f : config.class.getFields()) {
                if (!c.isSet("config." + f.getName().toLowerCase().replace("_", "."))) continue;
                f.set(null, c.get("config." + f.getName().toLowerCase().replace("_", ".")));
            }
            return;
        }
        catch (Exception var5) {
            var5.printStackTrace();
        }
    }

- Mogłeś już sobie odpuścić zabawe z fieldami, oraz w  bukkitapi masz implementacje YamlConfiguration, staraj się przerzucić na YamlConfiguration

    public static void setLocation(Location loc) {
        Main.getPlugin().reloadConfig();
        config.loadConfig();
        config.saveConfig();
    }

- Jakie ma mieć zastosowanie ta funkcja, oprócz reloadu configu, później jeszcze masz identyczną funkcje nazwaną reloadConfig

- Rejestrując już komendę, i jej executor nie musisz sprawdzać jej nazwy, to jest tylko zbędne...

	public static String xmuffionex_vanish_perm = "xmvanish";

	if (sender.hasPermission("xmuffionex.getVanished")) {

- W configu zastosowałeś zmienną, która będzie odpowiadać za uprawnienie, które należy nadać, aby wykonywać komendę, ale już w komendzie okazuje się ze jest statycznie

                        for (final Player pl : Bukkit.getServer().getOnlinePlayers()) {
                            pl.hidePlayer(p);
                        }

- W komendzie zastosowałeś pętlę, która ukrywa gracza wszystkim graczom ONLINE
- Natomiast listener od wchodzenia na serwer, znajduje się w ten samej klasie co komenda, gdzie tu logika, estetyka i przejrzystość?

	p.sendMessage(ChatColor.GREEN + "Masz vanisha!");
	p.sendMessage(ChatColor.GREEN + "Juz nie masz vanisha!");

 -  Jakie ma zastosowanie w końcu ten config?

	ActionBarAPI.sendActionBar(p, ChatColor.GREEN + "Jestes niewidoczny :O");

 - Może tak, dopisałbyś jakaś funkcje sprawdzającą, czy ActionBarAPI jest zainstalowane, może tak chwile zabawy z reflections ?

  • 1.8R1 i 1.12 (powyżej 1.12 nie wiem) mają inne konstruktory pakietów niż 1.8 R2 do 1.11, które mają identyczne

- Po całym sprawdzeniu kodu, gigantyczna ilość funkcji zastosowana przez Ciebie jest zbedna (Main, config)