Common anti-patterns in Minecraft modding
An anti-pattern is a common response to a recurring problem that is usually ineffective and risks being highly counterproductive. So, here are some anti-patterns specific to Minecraft modding. I will be focusing on things that can cripple your productivity, and the stability and performance of your mod.
You should be free to do whatever you want however you want. However, there are some architectural anti-patterns specific to Minecraft modding that you should avoid.
Carelessly copying vanilla Minecraft code
Sometimes you have to imitate vanilla Minecraft behavior. Say you want to make your own fence block. You could simply copy the code of the vanilla fence block into your own class.
If you didn’t thorougly inspect that code and rename things as you see fit, you don’t know exactly what you are doing. You could be doing things you never intended to do and it will be hard to make any sense of this in the future.
Carelessly copying vanilla Minecraft code can cause unexpected behavior, confusion and makes the mod harder to maintain.
You should be looking for opportunities to reuse code by invoking it instead of copying it. In the case of the fence block, you should be using, not copying the vanilla fence block class.
When CTRL-C CTRL-V is the only option, take your time to understand and clarify the code by renaming things. The vanilla code is obfuscated, naturally it takes time and effort to understand it. You might discover parts you can omit or change, that you would otherwise not find. Your future self will thank you for doing it now instead of later.
Designing your mod around base classes for ALL Items and Blocks
The motivation behind these base classes is great: Generalization. All Items and Blocks typically have registry names and item models. What better way to apply this to all your Items and Blocks than base classes?
It makes a lot of sense at first glance: When you need a new class, you just extend these and it handles all the common stuff. However, this has a major weakness: What do you do when you want to reuse other Item and Block classes?
You have no choice but to copy most of that vanilla class into your implementation, or break your pattern. Sometimes you simply cannot do what you want because parts of Minecraft are riddled with instanceof checks.
Using base classes for ALL Items and Blocks makes reusing vanilla Minecraft code harder. It thus cripples productivity and adds more code to maintain and understand.
It is the result of an overobsession with putting everything in Block/Item classes. What you should do instead? One solution is to add reusable methods outside your Item/Block classes to apply the common stuff. The following is an excerpt from Leatherworks that avoids the anti-pattern:
Automatically creating ItemBlocks for ALL Blocks
Most modders tend to automate registration in some way. Seeing as you have to manually create items for your blocks, why not shove all of your blocks into a list and create an ordinary ItemBlock for each of them?
It turns out not every block should have an ordinary ItemBlock. Here’s a list of a few vanilla blocks that have special ItemBlocks:
Here’s a list of a few vanilla blocks that don’t have ItemBlocks at all:
Automatically creating ItemBlocks for ALL Blocks builds upon a false assumption that every Block should have an ordinary ItemBlock.
In most cases non-ItemBlock Items are not an exception, but a rule, and should be treated as such. You should instead separate Block and Item registration and handle each item explicitly.
Client/server architecture anti-patterns
Minecraft is designed entirely with a client/server architecture. Your mod has to respect this.
Lack of server side validation
Never trust the client. Minecraft is no exception to this rule. Disobedience of this rule can cause unexpected behavior and create attack vectors for people with malicious intent. Your incompetence could cause servers to go down because of an undetectable security issue.
Here are some things that could easily go wrong:
- Blindly accepting item manipulation from the client. This can allow creation of arbitrary items or duping.
- Not checking a BlockPos coming from the client with isBlockLoaded. This can cause arbitrary chunk generation.
Synchronising excessive/useless data for TileEntities
It is up to you to implement synchronisation of tile entity state. Minecraft has built-in mechanisms for this; you could trigger a block update, or write the TileEntity to NBT and send it in a packet.
If you have a tile entity that has a lot of state and requires frequent synchronisation, these built-in mechanisms can cause too much overhead.
Synchronising excessive/useless data for TileEntities can lead to network congestion and (sometimes) unexpected negative side effects for the client.
You should optimize your network usage by sending only the information the client needs when the client needs it. For instance:
- You are making a furnace-like device. There is no need to send any information to the client about the smelting until the player opens the GUI, and that is exactly what the vanilla furnace does.
- You are making a device containing energy. Assuming the client cannot see or interact with the energy level, there is no need for the client to know the energy level at all.
If the tile entity state is large, you should consider sending only the part of the state that changed instead of the whole thing as NBT.
NOTE: Triggering a block update to synchronise a TileEntity can supposedly force the client to re-render a chunk even if nothing changed visually. That is highly unnecessary if done in excess.
Some things you do incorrectly can cause rare errors that you might never encounter personally.
Processing packets on the network thread
Minecraft uses Netty, therefore the networking is asynchronous and happens on another thread, the network thread.
Sometimes it seems you can get away with processing packets on the network thread, but in most cases you will cause a ConcurrentModificationException without even realizing.
CMEs are some of the hardest errors to debug, therefore you should always offload your packet processing to the logic thread. This is descibed in the MinecraftForge documentation.
There is but one way to send a packet to the server. This is because there is only ever one server, and only one way to…mcforge.readthedocs.io
Whether you’re downloading things from the internet or reading local files, there are two things you could be doing wrong.
Unsafe I/O is when you expect an InputStream/OutputStream to act in a specific way when there is no guarantee that it will. For instance, you could be trying to read a string of JSON over HTTP. There are endless scenarios where this would not produce a valid string of JSON.
Unsafe I/O can cause frustrating errors that you will never realize you are not handling.
You should handle every edge case properly, such as 404s, I/O exceptions and unexpected input.
Blocking I/O is when you halt a process to wait for a stream to complete its operation.
Blocking I/O can cripple load times and cause huge lag spikes in-game.
The process has to wait for the I/O to complete or time out, which can take several seconds. You should therefore offload I/O to separate threads or subroutines to avoid halting the process.
If you’re doing update checks, did you know MinecraftForge has an awesome built-in protocol for just that? It provides a standardized non-intrusive method of notifying users about updates.
Not using I18n (internationalization)
Minecraft is available in many languages, and your mod can be too. You should use the I18n system provided by Minecraft.
Not using I18n makes translation of your mod to other languages highly inconvenient.
In many cases, trying to use raw strings instead of I18n will cause more friction than using it. There are ways to use it incorrectly, though.
Localizing on the server
A key principle of Minecraft’s multiplayer is that you can connect to a server where you, the server, and any other player can have different language settings. This implies that all localization should happen on the side that displays the text.
Localizing on the server disrespects the client’s language settings. A dedicated server will always localize in “en_us”.
If you are using the deprecated net.minecraft.util.text.translation.I18n class, chances are you are doing something very wrong. The class is deprecated for a reason.
The following is a bad command implementation. It exhibits the two anti-patterns mentioned above.
- getCommandUsage returns a raw string. There is no feasible way to translate it.
- Everything is localized on the server. A spanish client should see
“El servidor dice pong”, but sees “The server says pong” instead.
The following is the same command avoiding the anti-patterns.
Identifying players by username
This was valid some years ago, but not anymore. Players can change their usernames, therefore you cannot identify players by their username.
If you need to store information about a specific player, use their UUID as the key.