# I found some bugs in Create mod and checked them in Minecraft
This article covers bugs in Create, one of the most popular Minecraft mods\. Let's see how they impact the game and then submit pull requests with the necessary fixes\.
## Introduction
[Create](https://modrinth.com/mod/create) is a Minecraft mod that adds shafts, gears, and other mechanical components\. You can combine them to build machines powered by rotational force, from a simple windmill to a fully automated [cake factory](https://www.youtube.com/watch?v=rR8W-f9YhYA)\.
The mod is hugely popular: it has more than 206 million [downloads on CurseForge](https://www.curseforge.com/minecraft/mc-mods/create) alone\. That doesn't even include [Modrinth](https://modrinth.com/mod/create), and the mod keeps getting updates for new versions of the game\. I'm pretty sure almost everyone who opened the article has played this mod\.
## Let's check the project
Some important info
* To check the project, I used PVS\-Studio static analyzer\. The article author is one of its developers\.
* You'll see code examples as you read\. Most of them have been shortened to avoid overwhelming the reader\. I've also put an ellipsis to mark the shortened code: "\.\.\.\."\.
* At the time of the check, the latest revision was the [87b3c6a](https://github.com/Creators-of-Create/Create/tree/87b3c6a65fd00c023a07b37b0353144bc7e6a5bf) commit\. I checked it using the static analyzer\.
* All analyzed source files and any conclusions based on other source files, include a permanent link that you can use to find them\.
* The article includes only the bugs that the author found interesting \(yes, it's a matter of personal taste\)\. If you want to see the rest, you can always download the analyzer and check the project yourself\.
### Ctrl \+ V doesn't work
The [SchematicEditScreen\.java\(130\)](https://github.com/Creators-of-Create/Create/blob/0924e93639ad5f61cfc39a221d909e16f2893df1/src/main/java/com/simibubi/create/content/schematics/client/SchematicEditScreen.java#L130) file
Let's take a look at the menu for pasting a schematic\.
What's a schematic?
A schematic is a part of the game world saved as a separate file\. This lets you move your house from one world to another, for example\.
```cpp
public boolean keyPressed(int code, ....) {
if (isPaste(code)) {
String coords = minecraft.keyboardHandler.getClipboard();
if (coords != null && !coords.isEmpty()) {
coords.replaceAll(" ", ""); // <=
String[] split = coords.split(",");
if (split.length == 3) {
boolean valid = true;
for (String s : split) {
try {
Integer.parseInt(s);
} catch (NumberFormatException e) {
valid = false;
}
}
if (valid) {
xInput.setValue(split[0]);
yInput.setValue(split[1]);
zInput.setValue(split[2]);
return true;
}
}
}
}
....
}
```
The PVS\-Studio warning:
[V6010](https://pvs-studio.com/en/docs/warnings/v6010/) The return value of function 'replaceAll' is required to be utilized\. SchematicEditScreen\.java 130
The idea is simple: the player copies the coordinates, presses Ctrl \+ V in a dedicated window, and the fields fill in automatically\.
But the `replaceAll` call marked `// <=` serves no purpose\. Strings in Java are immutable, so the method doesn't modify the original string; it returns a new one instead\. The new string is immediately discarded, leaving the spaces instead\.
The code then splits the string by commas, leaving a space at the beginning of the second and third elements, and `Integer.parseInt(" 67")` throws a [`NumberFormatException`](https://docs.oracle.com/javase/8/docs/api/java/lang/NumberFormatException.html)\. As a result, the fields remain empty\.
After opening the menu, I saw the expected behavior:
* `-156, 67, 204` doesn't work;
* `-156,67,204` works\.
The fix fits on a single line\. We just add an assignment before the `replaceAll` operation:
```cpp
coords = coords.replaceAll(" ", "");
```
### Unnecessary maximum
The [ScheduleScreen\.java\(557\)](https://github.com/Creators-of-Create/Create/blob/0924e93639ad5f61cfc39a221d909e16f2893df1/src/main/java/com/simibubi/create/content/trains/schedule/ScheduleScreen.java#L557) file
```cpp
for (List list : entry.conditions) {
int maxWidth = getConditionColumnWidth(list);
for (int i = 0; i < list.size(); i++) {
ScheduleWaitCondition scheduleWaitCondition = list.get(i);
Math.max(maxWidth, renderInput(....)); // <=
scheduleWaitCondition.renderSpecialIcon(....);
}
AllGuiTextures.SCHEDULE_CONDITION_APPEND.render(
graphics,
xOffset + (maxWidth - 10) / 2,
29 + list.size() * 18
);
xOffset += maxWidth + 10;
}
```
The PVS\-Studio warning:
[V6010](https://pvs-studio.com/en/docs/warnings/v6010/) The return value of function 'max' is required to be utilized\. ScheduleScreen\.java 557
The analyzer warning clearly explains the error: the result of the `Math.max(....)` call isn't written anywhere\.
I looked through the commit history, expecting to find some traces of refactoring\. I thought the developers might have moved the width calculation to a separate method and removed the assignment to `maxWidth` but left the `Math#max` call untouched\. There was nothing like that, though\. The file first appeared on February 1, 2022, in a [single 941\-line commit](https://github.com/Creators-of-Create/Create/commit/576d00d3a0e502d418488ef463ad7eb4237560ab#diff-59b447844d14ec067c0449bccbecf6179277ee5603f60cc4cb682911967f0bc8), and the `Math#max` call without an assignment had been there all along\. The original initialization of `maxWidth` with a call to `getConditionColumnWidth()` was there too\.
What would change if the devs added `maxWidth = Math.max(....)`? Nothing\. The column width where the text appears \(see the image below\) is calculated in advance with some extra room: `getConditionColumnWidth()` goes through the entire column and takes the maximum width, so no line can extend beyond it\. I checked this using the debugger: in every case I was able to test, it returned the same width\. So, the code is searching for the maximum of two identical numbers\.
The line didn't break over time, nor was it affected by any refactoring\. From day one, it had done nothing—for four years and through several updates to newer Minecraft versions\.
I'd suggest removing the unnecessary wrapper and leaving `renderInput` on its own\.
### Ignore error handling
The [MechanicalCrafterBlockEntity\.java\(535\)](https://github.com/Creators-of-Create/Create/blob/0924e93639ad5f61cfc39a221d909e16f2893df1/src/main/java/com/simibubi/create/content/kinetics/crafter/MechanicalCrafterBlockEntity.java#L535) file
First of all, let me show you what the mechanical crafter looks like:
Image explanation
In vanilla Minecraft, players craft items at a crafting table by manually placing ingredients in specific slots\. A mechanical crafter does the same thing automatically\. You build a grid from several blocks, with each block corresponding to one slot in the recipe, and place the required ingredient in each one\. The screenshot shows the recipe for an iron pickaxe: three ingots in the top row and two sticks down the middle\.
The arrows on the front indicate the route\. The crafters pass items from one to another along the chain until everything reaches the final block\. That's where the recipe gets assembled, and the finished item moves on to a conveyor belt or into a chest\.
The whole system runs on rotational power, so it needs a drive connected to the side\. In the screenshot, a creative motor powers the grid\.
Let's take a look at a method in its logic:
```cpp
protected void continueIfAllPrecedingFinished() {
List preceding =
RecipeGridHandler.getPrecedingCrafters(this);
if (preceding == null) { // <=
ejectWholeGrid();
return;
}
for (MechanicalCrafterBlockEntity blockEntity : preceding)
if (blockEntity.phase != Phase.WAITING)
return;
phase = Phase.ASSEMBLING;
countDown = 1;
}
```
The PVS\-Studio warning:
[V6007](https://pvs-studio.com/en/docs/warnings/v6007/) Expression 'preceding \=\= null' is always false\. New returns not\-null reference\. MechanicalCrafterBlockEntity\.java 535
The `getPrecedingCrafters` method returns the crafters that come earlier in the chain\. In other words, the ones that feed items into the current crafter\. It can never return `null`\. A list is created at the start, and both method exits return it\. Even if the block isn't a crafter, the method returns an empty list:
```cpp
public static List getPrecedingCrafters(
MechanicalCrafterBlockEntity crafter
) {
BlockPos pos = crafter.getBlockPos();
Level world = crafter.getLevel();
List crafters = new ArrayList<>();
BlockState blockState = crafter.getBlockState();
if (!isCrafter(blockState))
return crafters;
....
return crafters;
}
```
This means the branch with `ejectWholeGrid()` is unreachable\. The `preceding` list can never be `null`, but it can be empty, and often is\. What if we replace the check with `isEmpty()` and test it in the game? Let's see what happens\.
Before the fix:
After the fix with `isEmpty`:
The video shows that, before the fix, items entering the loop were passed from one crafter to another indefinitely\. That could potentially cause trouble and break someone's factory :\)
However, after the check was replaced with `isEmpty()`, the loop didn't even have a chance to form\. The method determined that there was nowhere else to expect items from, so it ejected them\.
However, as soon as I added two more crafters to the chain, the fix stopped working:
Two options are possible here\. Either the mechanical crafter needs to detect loops differently, then simply replacing the check with `isEmpty()` isn't enough\. Or this looping behavior isn't a bug at all\. In that case, we can just remove the `null` check\.
### The logging crash
The [TrainRelocationPacket\.java\(52\)](https://github.com/Creators-of-Create/Create/blob/0924e93639ad5f61cfc39a221d909e16f2893df1/src/main/java/com/simibubi/create/content/trains/entity/TrainRelocationPacket.java#L52) file
Here's the client\-server train movement package handler:
```cpp
public void handle(ServerPlayer sender) {
Train train = Create.RAILWAYS.trains.get(trainId);
Entity entity = sender.level().getEntity(entityId);
String messagePrefix = sender.getName()
.getString() + " could not relocate Train ";
if (
train == null || // <=
!(entity instanceof CarriageContraptionEntity cce)
) {
Create.LOGGER.warn(messagePrefix + train.id.toString()
.substring(0, 5) + ": not present on server");
return;
}
if (!train.id.equals(cce.trainId))
return;
....
}
```
The PVS\-Studio warning:
[V6008](https://pvs-studio.com/en/docs/warnings/v6008/) Potential null dereference of 'train'\. TrainRelocationPacket\.java 52
The condition triggers in two cases: either the train can't be found, or the entity is of the wrong class\. The second case is fine: it logs a proper warning\. However, in the first case, the code accesses `train.id` when `train` is `null`\.
The funny part is that the message we're trying to log ends with `not present on server`\. The branch was created for a missing train, but it fails because of it\.
The packet comes from the client, meaning it chooses the train ID\. A modified client build can send requests to transfer nonexistent trains and make the server fill its log with stack traces\.
The server won't crash: Create registers its packets so they execute safely, and even if an exception occurs, it gets logged, while the packet simply goes unhandled\.
We can fix this by including the identifier in the package:
```cpp
messagePrefix + trainId.toString().substring(0, 5)
```
### The two\-state theory
The [MechanicalMixerBlockEntity\.java\(238\)](https://github.com/Creators-of-Create/Create/blob/0924e93639ad5f61cfc39a221d909e16f2893df1/src/main/java/com/simibubi/create/content/kinetics/mixer/MechanicalMixerBlockEntity.java#L238) file
```cpp
protected List> getMatchingRecipes() {
List> matchingRecipes = super.getMatchingRecipes();
if (!AllConfigs.server().recipes.allowBrewingInMixer.get())
return matchingRecipes;
Optional basin = getBasin();
if (!basin.isPresent())
return matchingRecipes;
BasinBlockEntity basinBlockEntity = basin.get();
if (basin.isEmpty()) // <=
return matchingRecipes;
IItemHandler availableItems = level.getCapability(
Capabilities.ItemHandler.BLOCK,
basinBlockEntity.getBlockPos(), null
);
if (availableItems == null)
return matchingRecipes;
for (int i = 0; i < availableItems.getSlots(); i++) {
....
}
return matchingRecipes;
}
```
The PVS\-Studio warning:
[V6007](https://pvs-studio.com/en/docs/warnings/v6007/) Expression 'basin\.isEmpty\(\)' is always true\. MechanicalMixerBlockEntity\.java 238
Here, `basin.isEmpty()` can never return `true`, so `return` in this branch is unreachable\. Most likely, the developers meant to check `basinBlockEntity` rather than `Optional`\.
This doesn't affect gameplay in any way\. Further down, the code has other checks that work with the same block, and if the basin is empty, either the loop doesn't run or `availableItems` is `null`\.
But a dead `if` is more dangerous than it looks\. It's a safeguard that doesn't really protect against anything\. Rewriting the checks below leaves a hole that no one will think to look for because the check is kind of there\.
### Method contracts must be followed
The [Contraption\.java\(230\)](https://github.com/Creators-of-Create/Create/blob/0924e93639ad5f61cfc39a221d909e16f2893df1/src/main/java/com/simibubi/create/content/contraptions/Contraption.java#L230) file
```cpp
public static Contraption fromNBT(Level world, CompoundTag nbt,
boolean spawnData) {
String type = nbt.getString("Type");
Contraption contraption = ContraptionType.fromType(type);
contraption.readNBT(world, nbt, spawnData);
....
}
```
Can you see the error? What if we take a look inside `ContraptionType#fromType`?
```cpp
/**
* Lookup the ContraptionType with the given ID,
* and create a new Contraption from it if present.
* If it doesn't exist, returns null.
*/
@Nullable
public static Contraption fromType(String typeId) {
ContraptionType legacy = AllContraptionTypes.BY_LEGACY_NAME
.get(typeId);
if (legacy != null) {
return legacy.factory.get();
}
ResourceLocation id = ResourceLocation.tryParse(typeId);
ContraptionType type = CreateBuiltInRegistries.CONTRAPTION_TYPE.get(id);
return type == null ? null : type.factory.get();
}
```
Now it's clear\. The method explicitly warns about `null` twice: with an annotation and a comment\. However, the caller code doesn't check anything\.
The PVS\-Studio warning:
[V6008](https://pvs-studio.com/en/docs/warnings/v6008/) Potential null dereference of 'contraption'\. Contraption\.java 230
It's easier to break than it seems\. The very first line calls `nbt.getString("Type")`, and if the key doesn't exist, an empty string is returned\. This type isn't in the registry either, so the code returns `null` again\.
We can verify this pretty easily by giving ourselves a minecart with a nonexistent contraption type:
```cpp
/give @p
create:minecart_contraption[create:minecart_contraption_data=
{Type:"create:does_not_exist",InitialOrientation:"north"}]
```
What is a contraption?
A contraption is a structure that the mod detaches from the world and turns into a single movable object: a platform on a minecart, a piston carrying a load, or a rotating bridge\. The blocks inside stop being part of the world and are stored as data belonging to a single entity instead\.
The command won't break anything, and the item will go into the inventory without a problem\. But once we put the minecart on the rails, `MinecartContraptionItem` will try to build a contraption from this data, and that's where we'll get a [stack trace with `NullPointerException`](https://gist.github.com/TheLivan/ad251e6a4357230f1181eb59ae06d7c3)\. Minecraft will replace the player's contraption with a vanilla minecart\.
We can easily add the check:
```cpp
Contraption contraption = ContraptionType.fromType(type);
if (contraption == null) {
Create.LOGGER.warn("Unknown contraption type: {}", type);
return null;
}
```
But then `null` will need to be handled in both code snippets that call it\. That raises the question of what to do with a minecart whose contents the mod can no longer understand\. Should we leave it as an empty minecart, or simply not load the entity at all? I think it's worth asking the mod developers\. They clearly know better than anyone else what to do here\.
### Interruption
The [ServerLagger\.java\(14\)](https://github.com/Creators-of-Create/Create/blob/0924e93639ad5f61cfc39a221d909e16f2893df1/src/main/java/com/simibubi/create/infrastructure/command/ServerLagger.java#L14) file
```cpp
public void tick() {
if (!isLagging || tickTime <= 0)
return;
try {
Thread.sleep(tickTime);
} catch (InterruptedException e) {
e.printStackTrace();
}
}
```
The PVS\-Studio warning:
[V6103](https://pvs-studio.com/en/docs/warnings/v6103/) Ignored InterruptedException could lead to delayed thread shutdown\. ServerLagger\.java 14
`ServerLagger` is a helper class for the `killtps start ` debug command that puts the server to sleep for the specified number of milliseconds on every tick, allowing to see how the mod behaves when the TPS drops\. It's available only in debug builds and only to operators \(that's what server administrators are called\), so there's no need to worry\. However, the code is still incorrect, and it's worth explaining why\.
When the thread is interrupted, `sleep` immediately throws an `InterruptedException` and also clears the interrupt flag\. In other words, after `catch`, it seems like nothing's left: no flag, no exception, just a line in `System.err`\. The method returns control without issue, and the server continues running as if it had never been called\.
An [interrupt](https://docs.oracle.com/javase/tutorial/essential/concurrency/interrupt.html) is a request for the entire thread to stop, not just the part where you catch it\. By interrupting the thread and doing nothing, you take that request and throw it away\. The [Javadoc](https://docs.oracle.com/en/java/javase/26/docs/api/java.base/java/lang/InterruptedException.html) explicitly points this out\. One line is enough to pass the interrupt:
```cpp
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
```
## Conclusion
That's all for today\. I collected all the bugs both in this article and in pull requests for the project's developers \([click](https://github.com/Creators-of-Create/Create/pull/10696), [click](https://github.com/Creators-of-Create/Create/pull/10721)\)\.
Want to try static analysis too? You can always integrate it into your own project with PVS\-Studio\. You can download a trial version [here](https://pvs-studio.com/en/pvs-studio/try-free/)\. The license is [free for open\-source projects](https://pvs-studio.com/en/order/open-source-license/)\.