Code looks clean, however I've never worked with reflection before as far as I'm aware.
Would you be so kind to explain me the benefits of using this opposed to the system most RSPS use?
|
Download this and add as a dependency: https://oss.sonatype.org/service/loc...ner-2.0.19.jar
or if you are using Maven (doubt it):
in com.elvarg.net.impl.command add these classesCode:<dependency> <groupId>io.github.lukehutch</groupId> <artifactId>fast-classpath-scanner</artifactId> <version>LATEST</version> </dependency>
Spoiler for Command.java:
Spoiler for CommandHandler.java:
Spoiler for CommandInfo.java:
Spoiler for RelfectionCommand.java:
Now you need to go to com.elvarg.net.packet.impl and replace CommandPacketListener.java to
Spoiler for CommandPacketListener.java:
Final step: go to Elvarg.java and add
underCode:CommandHandler.getInstance().loadCommands();
Code:logger.info("Initializing the game...");
Adding commands
create a package in com.elvarg.net.packet.impl.command.impl
and create a name that implements Command and has a @CommandInfo annotation
for example:
Spoiler for Commands:
Code looks clean, however I've never worked with reflection before as far as I'm aware.
Would you be so kind to explain me the benefits of using this opposed to the system most RSPS use?
You don't need a third party library to write a class loader, just pointing that out.
Similarities - https://www.rune-server.ee/runescape...-commands.html
But aside from that, the annotation schema is a pretty cool idea(as I'm the one that originally did it), but when it comes down to it; it's just unnecessary, just syntax sugar.
EDIT:
I personally threw out the implementation I had a while ago as the plugin design that now replaced it has no need for annotated implementations; since the deployed PODs are dynamically loaded into memory during runtime anyways. But just a reason why something like this is uneccessary.
I love the idea of annotations also. It could really bring a better design. Thanks for your feedback.
Your example does show that you are still entitled to add
registerCommand(new PickupItemCommand());
Where as I've eliminated that purpose.
Still a great example although.
The reflection and loading parts are nice. Annotations are also nice.
However you really need to get rid of this monstrosity in CommandHandler#executeCommand:
Utilize your map as map :/Code:for (Entry<String, ReflectionCommand> entry : commands.entrySet()) { for (PlayerRights r : entry.getValue().getRights()) { if (r.higherRight(player.getRights())) { if (entry.getKey().startsWith(command.split("-")[0].toLowerCase())) { if (!entry.getValue().getCommand().execute(player, command)) { player.getPacketSender().sendMessage("try using: " + entry.getValue().getSyntaxHelp()); } return; } } } }
final String commandString = command.split("-")[0].toLowerCase(); //I'd split it up further for clarity
final ReflectionCommand rfc = map.get(commandString);
if(rfc.higherRights(player.getRights))
//bla
ALSO, there is no reason for ReflectionCommand and Command to be different, ESPECIALLY considering that ReflectionCommand doesn't even inherent from Command. Its super confusing. I see what you're doing with the classes, but you don't have to do it like that. (You can get annotations without making an instance iirc, then just make the new instance using Constructor - you can put in arguments via reflections too).
Thanks a lot for your feedback, looking at it yea, i should probably have made Command abstract passing in the values via constructor, that way i can get the correct syntax without having the ReflectionClass
Map itself is an interface, hashmap is the easiest and probably most recommended in this instance (i'm sure)
edit- updated
I don't think you understand what I meant. Pretty much there is no reason for the "Command" class to even exist. Making ReflectionCommand inherent from Command and then TAKE Command as an argument in its constructor is counter-intuitive (weird mix of composition and inheritance). ReflectionCommand does nothing special for reflection, but simply takes PlayerRights and syntax - whcih is something needed for ALL commands
Let me show you what I mean:
Instead of reflectioncommand & command have this:
and then inside your load commands do this instead:Code:public abstract class Command { private String syntaxHelp; private PlayerRights[] rights; public Command() { } public String getSyntaxHelp() { return syntaxHelp; } public PlayerRights[] getRights() { return rights; } //These will act as setters that can be chained in the construction of //Command, this is so you're able to keep your annotation structure public final Command setSyntaxHelp(String syntaxHelp) { this.syntaxHelp = syntaxHelp; } public final Command setRights(PlayerRights[] rights) { this.rights = rights; return this; } @Override public abstract boolean execute(Player player, String command); }
You could also use a constructor instead of using a "builder pattern" to construct Command, I just HATE having to implement default constructors in all my subclasses (also you assume that constructor args are in same order withCode://not too familiar with this library, idk if its still implementsInterface for //an abstract class if(map.get(c.getName()).implementsInterface("com.elvarg.net.packet.impl.command.Command")) { //^ I assume this makes sure that it must be type command, for safe cast //Get constructor with correct parameters @SuppressWarnings("unchecked") Constructor<Command> constructor = (Constructor<Command>)c.getDeclaredConstructor(); //Don't assume this will be annotation 0, make it certain! CommandInfo annotation = (c.getDeclaredAnnotation(CommandInfo.class)); // THIS will make an instance of command using the annotation variables Command reflectionCommand = constructor.newInstance(). setRights(annotation.rights()).setSyntaxHelp(annotation.syntaxHelp()); // Puts the command in the commands map getInstance().commands.put(annotation.syntax().toLowerCase(), reflectionCommand); ...
Which is fair to assume, but can be annoying if someone decides to flip the order around or something in a subclassCode:c.getDeclaredConstructor(String.class, PlayerRights[].class)
Also, I know map is an interface. It is also a data structure. Every implementation of Map will still implement the methods to effectively do the same things (just in "different ways" - which aren't going to be important 99% of the time and you just default to hashmap).
« Previous Thread | Next Thread » |
Thread Information |
Users Browsing this ThreadThere are currently 1 users browsing this thread. (0 members and 1 guests) |