Hey everyone, after a lot of players started complaining about our minecolonies fields, I took a look at the code I wrote almost 3 years ago and I got terrified. So, I decided to refactor the whole code to make it look cleaner.


How it was:

There was 1 Field class which also was the Container.
We used it to store the inventory as well as the information of the owner of the field.
There was 1 tileEntity without any important information.
There was 1 Inventory which was basically a basic ItemHandler and there was 1 fieldView as the client side representation.



This was extremely confusing and due to bad message passing we had a lot of information which didn't reach our GUIs and several other problems.

What I did:

I rewrote the Field code to the following model:

1 simple container class, I moved all the container constants to our inventoryConstants class and cleaned up methods which were not needed.

![](https://i.imgur.com/vUBaPa2.png)

Then, where we had previously stored the field I changed the code to blockPositions.

This way we don't have to serialize and deserialize oder write to buffers the data in several different places.

![](https://i.imgur.com/CNKHk8t.png)

Which should increase the performance of this code.

After that I moved all the management code in the tileEntity.
For example the owner, name, state etc of the field:

![](https://i.imgur.com/XMrPVQx.png)

The field class, since it is a tileEntity automatically writes and reads itself from nbt and also is able with a few adaptations to synch easily from and to NBT.
This way we decentralized the toNbt writing code which makes the fields way less error prone.
Nevertheless, the ScarecrowTileEntity is the biggest field class with over 570 lines (but containing a lot of javadoc as well).

Then in the next step I was able to delete the fieldView and the inventory and field classes (since the tileEntity is taking care of them).

![](https://i.imgur.com/azq7WIB.png)

Now, each time I have to use the field and not only it's position I can get the best instance of it directly from the world and won't, as previously, carry different instances of it around. This might be slightly more performance heavy but we have the most recent version of the entity, guaranteed.


Then I verified if the refactoring did not only fix the issues we previously had but also was efficient:

![](https://i.imgur.com/uPvvSi8.png)

And found that here in my pull request.
Definitely worth it, I removed around 240 lines keeping the same features and increasing the stability of the program.


I hope you liked my short refactoring trip.

Until the next time.

<br /><hr/><em>Posted on <a href="https://utopian.io/utopian-io/@raycoms/minecolonies-and-field-refactoring">Utopian.io -  Rewarding Open Source Contributors</a></em><hr/>