Всем привет. Я написал плагин на базовые команды, такие как tpa, home, day, gm и тд. Давно смотрел на написание плагинов, это мое хобби. Прощу вас рассказать, что хорошо реализовано, что плохо, что бы вы сделали лучше и тд.
Api.zip (20,8 КБ)
Всем привет. Я написал плагин на базовые команды, такие как 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);
}
В остальном вроде больше ничего не увидел такого, но я уверен можно написать и лучше
Видимо registerAll()
вообще никому не нужен…
С точки зрения 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;
}
Лучше запихнуть проверку аргументов внутрь instanceof
in_array
Не надо, это два разных случая. Хоть и количество аргументов важно в случае игрок это или нет, но все же. Это два разных случая, которые не должны друг с другом совпадать.
почитай что такое атрибуты.
Можешь создать 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(...) {
}
}
Всем спасибо за ваши рекомендации. Вроде бы получилось лучше, если кто будет использовать мой плагин, то вот улучшеная версия. Если у вас будут новые идеи или я где то накосячил, то всегда буду рад вашей критике.
Api.zip (22,0 КБ)