Плагин на базоовые команды(API) PM5

Всем привет. Я написал плагин на базовые команды, такие как tpa, home, day, gm и тд. Давно смотрел на написание плагинов, это мое хобби. Прощу вас рассказать, что хорошо реализовано, что плохо, что бы вы сделали лучше и тд.

Api.zip (20,8 КБ)

“API” в незвании не уместно.

Не в одном классе, хотя бы что то радует.

        $this->getServer()->getPluginManager()->registerEvents(new Listener($config->get("onJoinMessage"), $config->get("onLeaveMessage"), $config->get("pvpInModes")), $this);
        $this->getServer()->getCommandMap()->register("Api", new SpawnCommand());
        $this->getServer()->getCommandMap()->register("Api", new RtpCommand($this->cooldown, $this->getScheduler()));
        $this->getServer()->getCommandMap()->register("Api", new TpCommand($this->cooldown));
        $this->getServer()->getCommandMap()->register("Api", new HomeCommand($this->homes,));
        $this->getServer()->getCommandMap()->register("Api", new DelhomeCommand($this->homes));
        $this->getServer()->getCommandMap()->register("Api", new SetHomeCommand($this->homes, $config->get("maxHomes")));
        $this->getServer()->getCommandMap()->register("Api", new HomesCommand($this->homes));
        $this->getServer()->getCommandMap()->register("Api", new DayCommand());
        $this->getServer()->getCommandMap()->register("Api", new NightCommand());
        $this->getServer()->getCommandMap()->register("Api", new GamemodeCommand());
        $this->getServer()->getCommandMap()->register("Api", new ClearCommand());
        $this->getServer()->getCommandMap()->register("Api", new FlyCommand());
        $this->getServer()->getCommandMap()->register("Api", new TpaCommand($this->tpaManager, $this->getScheduler()));
        $this->getServer()->getCommandMap()->register("Api", new TpcCommand($this->tpaManager));
        $this->getServer()->getCommandMap()->register("Api", new TpdCommand($this->tpaManager));
 

Это ■■■■■■, учись сокращать

$this->getServer()->getPluginManager()->registerEvents(new Listener(
    $config->get("onJoinMessage"), 
    $config->get("onLeaveMessage"), 
    $config->get("pvpInModes")
), $this);

$commands = [
    new SpawnCommand(),
    new RtpCommand($this->cooldown, $this->getScheduler()),
    new TpCommand($this->cooldown),
    new HomeCommand($this->homes),
    new DelhomeCommand($this->homes),
    new SetHomeCommand($this->homes, $config->get("maxHomes")),
    new HomesCommand($this->homes),
    new DayCommand(),
    new NightCommand(),
    new GamemodeCommand(),
    new ClearCommand(),
    new FlyCommand(),
    new TpaCommand($this->tpaManager, $this->getScheduler()),
    new TpcCommand($this->tpaManager),
    new TpdCommand($this->tpaManager)
];

foreach ($commands as $command) {
    $this->getServer()->getCommandMap()->register("Api", $command);
}

В остальном вроде больше ничего не увидел такого, но я уверен можно написать и лучше

1 лайк

Видимо registerAll() вообще никому не нужен…

1 лайк

С точки зрения APi PMMP я тебе вообще ничего не скажу, я сам его не знаю. Но вот по поводу просто улучшения - если ты решил классифицировать некоторые классы в каталоги, то применяй это ко всем отдельным классам\либо те, где как минимум из них 2-3 подтипа.

TP-a-c-d

И еще, при построении условий, заключающая конструкция (else) у тебя выполняет роль откидывания ошибки. Однако, наоборот, все ошибки и не те запросы нужно откидывать в первом варианте if, дабы не приходилось читать весь код дабы понять, почему выкинуло это сообщение.

public function execute(CommandSender $sender, string $commandLabel, array $args): bool
{
if (!($sender instanceof Player)) {
    $sender->sendMessage(TextFormat::RED . "Команда доступна только в игре");
    return false;
}

if (count($args) < 1) {
    $sender->sendMessage("§cИспользуйте /delhome <имя дома>");
    return false;
}

$homeName = $args[0];
$homes = $this->homes->getPlayerHomes($sender->getUniqueId(), $sender->getWorld());

foreach ($homes as $home) {
    if (strtolower($home['home_name']) === strtolower($homeName)) {
        $this->homes->delHome($sender->getUniqueId(), $homeName);
        $sender->sendMessage("§aДом §f" . $homeName . " §aуспешно удалён");
        return true;
    }
}

$sender->sendMessage("§cУ вас нет дома с названием §f" . $homeName);
return false;

}

до:

   public function execute(CommandSender $sender, string $commandLabel, array $args): bool
    {
        if (count($args) < 1) {
            $sender->sendMessage("§cИспользуйте /delhome <имя дома>");
            return false;
        }

        if ($sender instanceof Player) {
            $homeName = $args[0];
            $homes = $this->homes->getPlayerHomes($sender->getUniqueId(), $sender->getWorld());

            foreach ($homes as $home) {
                if (strtolower($home['home_name']) === strtolower($homeName)) {
                    $this->homes->delHome($sender->getUniqueId(), $homeName);
                    $sender->sendMessage("§aДом §f" . $homeName . " §aуспешно удалён");
                    return true;
                }
            }

            $sender->sendMessage("§cУ вас нет дома с названием §f" . $homeName);
        } else {
            $sender->sendMessage(TextFormat::RED . "Команда доступна только в игре");
        }
        return false;
    }
2 лайка

Лучше запихнуть проверку аргументов внутрь instanceof

1 лайк

in_array

1 лайк

Не надо, это два разных случая. Хоть и количество аргументов важно в случае игрок это или нет, но все же. Это два разных случая, которые не должны друг с другом совпадать.

почитай что такое атрибуты.
Можешь создать EssentialCommand который будет наследовать Command, в execute() с помощью рефлексии искать атрибут, например, OnlyForPlayerCommand, и, если он найден, проверять отправитель игрок или нет. Если все проверки пройдены, вызывать абстрактный onExecute(), который ты уже будешь реализовывать в классе конкретной команды

<?php

declare(strict_types=1);

use Attribute;

#[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_CLASS)]
final class OnlyForPlayerCommand{}
<?php

declare(strict_types=1);

class EssentialCommand extends Command{

    final public function execute(CommandSender $sender, string $commandLabel, array $args) : void{
		$reflectionClass = new \ReflectionClass($this);
		$onlyForPlayerAttributes = $reflectionClass->getAttributes(OnlyForPlayerCommand::class);

		if(!empty($onlyForPlayerAttributes) && !$sender instanceof Player){
			$sender->sendMessage('Only for players');
			return;
		}

		$this->onExecute($sender, $commandLabel, $args);
	}

	abstract protected function onExecute(CommandSender $sender, string $commandLabel, array $args) : void;

}
<?php

declare(strict_types=1);

#[OnlyForPlayerCommand]
final class TpaCommand extends EssentialCommand{

    public function onExecute(...) {
        
    }
}
2 лайка

Всем спасибо за ваши рекомендации. Вроде бы получилось лучше, если кто будет использовать мой плагин, то вот улучшеная версия. Если у вас будут новые идеи или я где то накосячил, то всегда буду рад вашей критике.

Api.zip (22,0 КБ)