Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?

Introduction

After much deliberation, our team has decided that it would be in our best interest to start from scratch and develop new code for managing games, and the network as a whole, for Stratus. This document details some of the more technical reasons as to why we felt it was time for a complete rewrite. Some of these reasons include a steep learning curve for new developers, large collection of old code, design principles which no longer make sense, and the fact that the entire project is over-engineered far beyond a point where a Minecraft server needs to go.

Lacking Optimizations

PGM and all of the supporting plugins and infrastructure were written in, and for, an environment that could support large-scale servers with massive amounts of memory and storage capacity. The design principles used throughout all of the OCN plugins, such as the overabundance of dependency injection along with careless memory allocation (more on this later), make them extremely cost-intensive to run and time-consuming to maintain and develop new features for.

Painful for New Developers

There are a large number of extremely complex steps to even set up a testing environment in the first place. A database, messaging server, website, and all the proper configuration options (which aren’t well documented) are needed in order to even join a testing server. A lot of the internal APIs are also poorly documented, if documented at all, which makes life even harder for newer developers and adds a steep learning curve to the already massive project. There are at least 3 generations of APIs and utility layers in the codebase which are tightly bound and extremely difficult to update.

As a whole the code contains 69 instances of the @Deprecated annotation. Code marked as @Deprecated means that it is not meant to be used in any new code and should be removed from any old code whenever it is updated. It is also common practice that a better solution be in place before code is marked as deprecated. This is not the case, however. There are instances of code which should no longer be used which lack an all-encompassing solution that can be used instead.

Large Codebase - Minimal Documentation

The codebase as whole is over 5 years old and contains a lot of old (“legacy”) code. Over time, as the code was updated, and new features were added, it became a lot more complex to maintain and update. When new coding standards were introduced, for example, only parts of the code were updated. This led to even more code being written to support this legacy code so that every single part of the massive project, along with any external libraries, did not have to be updated.

This makes adding features an extremely tedious and time-consuming process. Sometimes in this process even more code has to be written to make it all work with everything else that is so tightly bound together, adding another layer to the growing web of complexity. Our team has spent weeks adding simple functionality because the massive tangled web of old and new code, along with the patches that make the old and new work nicely together, have to be untangled before any feature at the base level (such as chat) can even be modified.

Design Choices: Then

When it was initially created, the OCN codebase was one of a kind in the field of Minecraft game management. As such, some of the concepts and problems it tries to tackle had not been implemented in that environment. Because of the lack of previous implementations, the core developers of the original frameworks (Apple and Anxuiz) had to find creative ways to handle these complications in the bukkit server environment. I say all of this to say that in no way am I blaming them (or anyone who worked on the project, for that matter) for the problems we are facing now. None of them could have imagined how large the codebase would become or how long it would be used.

Over time, as the codebase became increasingly large and countless features were added, some of these core design choices stuck around. Take the concept of a Module and MatchModule, for example. This was designed in an effort to reduce cycle time and to maximize reusability (the same map being played more than once in the same rotation). This design choice was implemented at a time when there were under 100 maps in the entire maps collection, and rotations were quite small. Because of this, the decision was made to parse every map on server start, rather than at actual cycle time. This made sense when the maps collection was quite small, but now we are parsing over 1,000 maps on each server load.

The long load time is not helped by how much extra parsing code was added over the life of the project. As new developers came into the picture, creative new solutions were implemented to tackle some of the pains of developing new features. The problem we're facing now is that while these new styles and utilities were created and used, the old ways are still present because they were too lazy or didn't have enough time to update all of the old parsing code.

Some of these solutions go as far as to let a developer define a class file with method names which match XML tags and have all of the parsing "just work" without them having to write it. This seems nice in principle and does in the end make writing some new features easy, but the backing code to support this functionality is over-engineered to say the least. Great lengths were taken to make this possible, and while it is nice to have something "just work", the performance hit that this causes is not worth it at the end of the day. If it takes us a few more lines to write a parser, but we shave sometimes seconds off of overall parsing time, we'll just have to live without having everything as nice as it is currently. This is just one example of the countless uses of over-engineering to achieve simple goals like making writing parser a bit easier.

Design Choices: Now

Going into the rewrite, we're using the things we've learned from these drawbacks, and also learned to accept that our code won't be as nice as PGM's (it was written by a team of extremely talented people over a span of 4 years). While our team may not have the luxury to write one class and have all of the parsing taken care of in the background, we do have to advantage of not having the overhead that comes with something like that. Take the following block of code from our rewrite, as an example, which shows some of the steps we are taking to make the entire experience more efficient in the end.

/**
 * A specific feature, or container of features, which can be installed into a {@link GameRound}. These are created by {@link
 * FacetParser}s, or can be bound using {@link RoundBinders#bindFacetDirect(Facet)} which will always install the facet regardless
 * of parsing status. These objects are constant and live only as long as a *single* round. Because of this, implementations
 * should not be concerned with immutability or multi-round data.
 * <p>
 * NOTE: This was called a MatchModule in PGM.
 *
 * @implNote PGM had {@code Module}s and {@code MatchModule}s, which were in essence a immutable layer underneath a runtime
 *   layer. While this was nice since it separates configuration from implementation, it had a major side-effect on memory when
 *   a large collection of these module sets were created at once (due to the vastness of our map collection). Because of this,
 *   these two concepts have been combined into a singular object which is created around the same time as the round is loaded,
 *   and only lasts as long as the individual round. While the old modules (and feature definitions) were created at *parse
 *   time* with the match modules (and features) being created at *round load time*, the new system creates them both at the
 *   same time, and simple calls {@link #load()} when everything has been parsed. PGM ran into an issue with longer parse times
 *   due to a variety of design drawbacks, such as reflective parsing and a pile of guice objects to handle legacy code. We have
 *   learned from these mistakes and have made parsing time adequate enough to be used at (or around) cycle time. The only major
 *   drawback of this new system is, however, that parsing implementations must be as light as possible, just with constructor
 *   methods of any sub-class of this one.
 */
public abstract class Facet {

This is just one of the many steps we have taken to make things more efficient in terms of memory usage. We are also heavily focusing on the entire concept of not making processes more complex than they really need to be. We also have the advantage of not having to add support for 4 years' worth of legacy and already inefficient code, which reduces even more overhead.

Conclusion

The OCN codebase has served this community well for a very long time, but has unfortunately become too big to maintain. Features were added on top of each other to make life easier at the time, but have come back to haunt us later. What made sense in 2014 unfortunately no longer makes sense in terms of usability and coding style. Documentation for some of the APIs that were meant to make life easier is so sparse that they have ended up making life harder. In the end, the codebase served its purpose, but it is time to move on and close this chapter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.