Di3mex/ExtraHardMode — pull request #1 by Mitsugaru (2013-02-19 07:04:17+00:00)
Title: Redesign & Restructuring

Alright, so, here's my first PR for EHM. Hopefully you'll welcome the changes I've made.
What's new?
------------------
- Convert the project to use Maven.
- Moved classes into appropriate packages.
- Added useful service classes, such as:
- Modules
- Command Handlers
- Modular Configuration (YAML) handlers
- Centralized PermissionNodes
- Documentation (however, not entirely complete)
- Other minor cleanup

Maven
---------
Maven is incredibly useful for project management, such as keeping track of dependencies and for continuous integration. In fact, I have my fork always build on my personal Jenkins whenever I commit something.

To make use of this, just have your IDE recognize that it is a Maven project.

You will not have to specify the Bukkit jar in the project's build path as a library reference since Maven will do that for you.

However, it is important to note that you will have to use Maven to build the resulting jar, which is placed in the generated ```target``` directory in the project's base folder.

The reason for this is because the [plugin.yml](https://github.com/Mitsugaru/ExtraHardMode/blob/master/plugin.yml#L1-L5) has been edited to make use of Maven's filtering. You'll notice that the info in plugin.yml has variables. Maven will fill those in when the project is built.

However, building a project as Maven is really easy. The typical goals to run are `clean package`. If you need help setting up with Maven, I can help over Skype / TeamViewer.

Packages
--------------
Before, all the source files were under a [single package](https://github.com/Di3mex/ExtraHardMode/tree/master/src/me/ryanhamshire/ExtraHardMode). While this might not be such a big issue for smaller plugins, EHM is hardly "small". Not to mention, having tasks mixed with event listeners mixed with configuration handlers is kind of unorganized.

So I've gone ahead and restructured the class files to be [organized in packages](https://github.com/Mitsugaru/ExtraHardMode/tree/master/src/me/ryanhamshire/ExtraHardMode).

Modules
------------
Modules are basically a way to separate the logic and storage methods that need to be accessed by other parts of the plugin.

For example, some of the methods in the old EntityEventHandler were used in other parts of the code. Specifically, ```markLootLess(LivingEntity entity)``` is used in both the [EntityEventHandler](https://github.com/Di3mex/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/EntityEventHandler.java#L191) and was statically called in the [RespawnZombieTask](https://github.com/Di3mex/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/RespawnZombieTask.java#L46). By moving reusable code into Module classes, anything that has a reference to the original plugin instance can access a module class and use its methods. This removes messy static methods.

Getting an existing module instance is really easy:
```java
EntityModule module = plugin.getModuleForClass(EntityModule.class);
```
No casting necessary, its automatically done for you via Generics. Whatever Module class you ask for, you receive the registered instance of it, if it has been registered. If it hasn't been, then the returned module would be null... however, you're guaranteed the current existing modules in the code will be registered since they're the first thing to happen on the plugin enable.

Now, you might be thinking that Modules are overkill. I mean, this could easily be accomplished by just having the plugin have a reference to the instance:
```java
public class ExtraHardMode extends JavaPlugin {
private final EntityModule entityModule= new EntityModule ();

public EntityModule getEntityModule() {
return entityModule;
}
}
```
Except, modules allows us to easily expand available classes without having to require that method. Instead of having a method per module, we have a single unified method that lets you get the module instance you ask for. It makes the main plugin class look really clean in comparison.

Plus, it also allows for people to extend ExtraHardMode by having secondary plugins register their own modules. AKA, maybe a plugin called EHMExtensions?

Registering a module is easy:
```java
registerModule(MessageConfig.class, new MessageConfig(this));
```
On register, the starting() method of the module is called. Examples that make use of that method are the configuration handlers (which are also modules now) [RootConfig](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/config/RootConfig.java) and [MessageConfig](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/config/messages/MessageConfig.java), where they load their defaults, if any are missing, and load the actual data of the config to memory. As well as [BlockModule](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/module/BlockModule.java#L145) where it gets the block materials that have physics enabled.

Creating a Module class is as simple as either extending [EHMModule](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/service/EHMModule.java) or implementing the [IModule](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/service/IModule.java) interface. The abstract class EHMModule is there only to insure that the module always has a reference to the original plugin instance.

Commands & Command Handlers
-------------------------------------------------
I've also added two more files, [CommandHandler](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/service/CommandHandler.java) and [ICommand](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/service/ICommand.java). These have been immensely helpful in creating modular command trees.

Essentially, think of a CommandHandler as a menu with various options and ICommand classes are those terminating commands.

CommandHandlers implement [CommandExecutor](http://jd.bukkit.org/rb/apidocs/org/bukkit/command/CommandExecutor.html), so you can [attach them to commands](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/ExtraHardMode.java#L87) _or_ [chain them to existing](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/service/CommandHandler.java#L83) CommandHandlers to make menus within menus (Menu-ception).

Classes that implement the ICommand interface, such as the [ReloadCommand](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/command/ReloadCommand.java), are the terminating commands.

For instance, I added a few basic commands under the [root command](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/command/Commander.java#L15-L20) ```/ehm```.

Configuration
------------------
If there was any one thing that absolutely needed to be fixed in EHM, it was configuration handling.

If the DataStore class was [supposed to](https://github.com/Di3mex/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/ExtraHardMode.java#L48) handle configuration files ([and at the same time not supposed to?](https://github.com/Di3mex/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/DataStore.java#L27) :\).... then [**all** of the configuration values](https://github.com/Di3mex/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/ExtraHardMode.java#L57-L149) should **not** be in main plugin class **nor** be [handled in the onEnable()](https://github.com/Di3mex/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/ExtraHardMode.java#L166-L405).

So, to clean that up, I added the [ModularConfig](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/service/ModularConfig.java) class to handle configuration files and [ConfigNode](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/service/ConfigNode.java) to represent all possible configuration node paths, what type of variable to expect from the path, and the default value it should be set to.

Also, should the configuration's internal structure change, it means minimal / reduced changes to references in the code.

The best example of using it would be in the [RootConfig](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/config/RootConfig.java) class, which handles the default config.yml file. Associated with this class are the nodes from the [RootNode](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/config/RootNode.java) enumeration. For each entry, you can easily see the path in the config, the type, and the default value that it will set to if the entry is missing.

The defaults and current configuration are loaded by simply [iterating over the enumeration](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/config/RootConfig.java#L59-L73).

Using the config is easy. Simply get the variable you want by giving it the enumeration value you want:
```java
// FEATURE: charged creeper spawns
if(entityType == EntityType.CREEPER) {
if(plugin.random(config.getInt(RootNode.CHARGED_CREEPER_SPAWN_PERCENT))) {
((Creeper) entity).setPowered(true);
}
}

// FEATURE: extra monster spawns underground
int maxY = config.getInt(RootNode.MORE_MONSTERS_MAX_Y);
int multiplier = config.getInt(RootNode.MORE_MONSTERS_MULTIPLIER);

// FEATURE: always-angry pig zombies
if(config.getBoolean(RootNode.ALWAYS_ANGRY_PIG_ZOMBIES)) {
if(entity instanceof PigZombie) {
PigZombie pigZombie = (PigZombie) entity;
pigZombie.setAnger(Integer.MAX_VALUE);
}
}
```
Found in [EntityEventHandler](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/event/EntityEventHandler.java#L424).

Also, ModularConfig is not restricted to just the config.yml. Thus, I gave a specific handler for the messages.yml through the [MessageConfig](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/config/messages/MessageConfig.java) with its own [MessageNode](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/config/messages/MessageNode.java) enumeration values. Because of this change, it replaces the [CustomizableMessage](https://github.com/Di3mex/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/CustomizableMessage.java) class, [TextMode](https://github.com/Di3mex/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/TextMode.java) static variables and [Messages](https://github.com/Di3mex/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/Messages.java) enumeration. Chat colors are parse-able from the config itself and it [automatically translates](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/config/messages/MessageConfig.java#L101) the standard ```&``` symbol.

Permissions
------------------
1) I noticed that only the [bypass](https://github.com/Di3mex/ExtraHardMode/blob/master/plugin.yml#L8) permission node was in the plugin.yml.
2) I found hardcoded permission nodes [strewn](https://github.com/Di3mex/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/BlockEventHandler.java#L79) [about](https://github.com/Di3mex/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/BlockEventHandler.java#L223).
3) Certain [permission](https://github.com/Di3mex/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/BlockEventHandler.java#L261) [nodes](https://github.com/Di3mex/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/BlockEventHandler.java#L271) are [ambiguous](https://github.com/Di3mex/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/BlockEventHandler.java#L284).

If permission nodes are not recorded somewhere, such as a wiki page on the BukkitDev project, then certain server operators / administrators are smart enough to look in the plugin.yml for the available permission nodes... but if they're not recorded in there either, then, they can't possibly know about the permission nodes in the code itself.

Enter the [PermissionNode](https://github.com/Mitsugaru/ExtraHardMode/blob/master/src/me/ryanhamshire/ExtraHardMode/service/PermissionNode.java) enumeration. A central place to store and reference registered permission nodes. This way, there is no question to what permission nodes exist. Also, should a permission node path change, the code does not have to change.

I understand and acknowledge the case for on-demand, generated permission nodes... however, the project is not at that point... nor did I see any uses for it.

Also, put all of the permission nodes into the [plugin.yml](https://github.com/Mitsugaru/ExtraHardMode/blob/master/plugin.yml#L12-L53) as well.

Documentation
----------------------
While there were some helpful in-line comments, I added JavaDocs to whatever I could. Albeit, there are probably some parts that I may not have completely filled in the info... and the fact that majority of the info is simply whatever comment was already there.

Other minor cleanup
-----------------------------
### Static
Completely removed all unnecessary static keywords, particularly for methods.
### Public static reference to plugin instance
```java
//for convenience, a reference to the instance of this plugin
public static ExtraHardMode instance;
```
It may appear to be "convienent", but it is **FAR** from it.
If there is one programming habit of some Bukkit developers that should be broken, its **THIS**.
This should **never** be done. **Ever**. Please... just no.

If a part of your plugin needs a reference to the main plugin instance... it isn't so much extra to pass that reference to its constructor and have the object hold onto it.
### Logger of Minecraft subsystem
```java
//for logging to the console and log file
private static Logger log = Logger.getLogger("Minecraft");
```
Bukkit gave each plugin their own Logger object for a reason... and I've switched any reference to this old log object to the plugin's one.
### Formatting
Standardized the formatting for all classes.

Finally
---------
I've loaded it on my test server and it initialized / enabled just fine. However, I haven't had the chance to do extensive testing.

What worked:
- Block physics, tested by breaking a tree from the bottom. Logs fell as normal.
- Mining ore rules, surrounding stone turned to cobble.

What hasn't been tested:
- Everything else.

The thing to keep in mind is that my edits were not to edit the logic of the plugin as much as possible. So, everything should still work, in theory.