Carryon - Patched (Unofficial)

FULL PATCH NOTES

# Carry On Patch — Change Summary

## Target

- **Mod**: Carry On (`tschipp.carryon`)
- **Original jar**: `carryon-neoforge-1.21.1-2.2.4.4.jar`
- **Patched jar**: `carryon-neoforge-1.21.1-2.2.4.4-patched.jar`
- **Mod ID**: `carryon` (unchanged)
- **Mod version**: `2.2.4` (unchanged — patched jar still reports as the original version, so existing carry state and configs load normally)

## What was changed

Exactly **one method** in **one class** was modified. Nothing else in the jar — manifests, mods.toml, services, resource files, other classes — was touched.

| Item | Value |
|---|---|
| Class | `tschipp/carryon/CarryOnCommon.class` |
| Method | `public static int potionLevel(CarryOnData, Level)` |
| Method descriptor | `(Ltschipp/carryon/common/carry/CarryOnData;Lnet/minecraft/world/level/Level;)I` |
| Original method size | 102 bytes of bytecode (138 bytes with stack maps + line numbers) |
| Patched method size | 2 bytes of bytecode |
| Other methods in this class | 12 methods, all unchanged |
| Other classes in jar | All unchanged |

## Why it was changed

### The bug

`potionLevel()` is called every server tick (from `onCarryTick`) and on every right-click pickup (from `PickupHandler.tryPickUpBlock`). For a carried block, it reads the block's NBT, calls `nbt.toString()` on the `CompoundTag`, and uses the resulting string's length as a slowness duration estimate.

`CompoundTag.toString()` internally calls `Collections.sort()` on the tag's keys to produce deterministic output. **If any key in that map is `null`** — which happens with malformed NBT from buggy mod interactions — `TimSort.binarySort` throws `NullPointerException: Cannot invoke "java.lang.Comparable.compareTo(Object)" because "pivot" is null`.

### How the bug manifests on this server

- Reproduced 2025-05-05 at 16:09 and 18:48: a player right-clicks a tile entity (in the first reported case, half of a double chest) → `tryPickUpBlock` calls `potionLevel` → `nbt.toString()` hits a null key → server tick loop dies → 60 seconds later the watchdog fires a second crash report.
- Both crashes shared an identical stack trace ending at `CarryOnCommon.potionLevel(CarryOnCommon.java:211)`.
- This is a known, currently-open bug upstream: GitHub issue **Tschipp/CarryOn#898**, filed against carryon 2.2.4.4 on NeoForge 1.21.1, with the same stack trace. No fix is available from the maintainer.

### The config angle

The mod's config has `heavyTiles` and `blockSlownessMultiplier` settings that *look* like they should help. They don't — the `nbt.toString()` call happens **before** the config check, at bytecode offset 88, while the `heavyTiles` field load is at offset 114. The crash occurs 26 instructions before the config has any chance to influence behavior. Disabling `heavyTiles` does not prevent the crash.

## What the patch does

The entire body of `potionLevel` was replaced with a 2-instruction stub:

```
0: iconst_0    // push the integer 0 onto the stack
1: ireturn     // return it
```

That is the entire patched method. It takes its two arguments, ignores them both, and unconditionally returns `0`. There are no branches, no field reads, no method calls, no NBT access of any kind. It is structurally incapable of throwing any exception.

### Why returning 0 specifically

Examining the single call site in `onCarryTick`, the int returned by `potionLevel` is passed as the **duration** parameter (in ticks) of a `MobEffectInstance(Slowness, duration, amplifier=0, ambient=false, visible=false)` that gets applied to the carrying player.

- Returning `0` → 0-tick duration → effect expires the same tick it's applied → **no slowness applied to the player at all**.
- The amplifier was hardcoded to 0 in the caller and was never affected by `potionLevel`'s return value, despite the method's misleading name.

So returning 0 produces the cleanest possible behavior: the player is not slowed while carrying anything, and no NBT is ever read by this method.

## Side effects

The following config options become **no-ops** because they were only consumed inside the (now-removed) body of `potionLevel`:

- `heavyTiles`
- `heavyEntities`
- `blockSlownessMultiplier`
- `entitySlownessMultiplier`

Setting them to anything in `config/carryon-common.toml` will have no effect. The "carrying things slows the player down" gameplay mechanic is gone — players move at full speed regardless of what they carry.

The following Carry On features are **unaffected** and continue to work normally:

- Picking up blocks and placing them back (preserves full NBT including chest contents — handled by `setBlock` and `loadStatic` in `CarryOnData`, both unchanged).
- Picking up entities.
- Stacking entities.
- All blacklist/whitelist behavior.
- All other config options (max distance, max entity size, hostile mob restrictions, etc.).
- All existing player carry state survives the swap (the patched jar reports the same mod version, so saved data is binary-compatible).

## How the patch was applied

A small ASM-based Java program rewrites the method body:

1. Reads `CarryOnCommon.class` from the original jar with `ClassReader`.
2. Walks the class's methods, finds the one matching `potionLevel(CarryOnData, Level)I`.
3. Replaces its `InsnList` with `[ICONST_0, IRETURN]`.
4. Clears the method's try/catch blocks and local variable table (no longer needed).
5. Sets `maxStack = 1`, `maxLocals = 2`.
6. Writes the class back with `ClassWriter(COMPUTE_MAXS)`.
7. Repacks all entries into the new jar in a single pass, replacing only the modified class.

No other changes are made during the repack. Manifest, mods.toml, services, and all other resources are copied byte-for-byte.

## Verification performed

- `javap -c` on the patched class confirmed the new method body matches the intended bytecode exactly.
- `javap -v` on the patched class confirmed the StackMapTable is empty (none needed for an unconditional return), `stack=1, locals=2`, and the method descriptor is unchanged.
- All 12 other methods in `CarryOnCommon` were verified to have identical bytecode to the original.
- The `META-INF/neoforge.mods.toml` was confirmed to still report `modId = "carryon"` and `version = "2.2.4"`.
- Jar contains no signing files (the original wasn't signed either, so no signature validation issue at load time).

## Deployment

1. Stop the server.
2. Move `carryon-neoforge-1.21.1-2.2.4.4.jar` out of `mods/` (keep as backup).
3. Place `carryon-neoforge-1.21.1-2.2.4.4-patched.jar` into `mods/`.
4. Start the server.

No config changes, no world changes, no client-side changes required.

## Limitations / what this does *not* fix

- **Only `potionLevel` is patched.** If Carry On has any other method that also calls `tag.toString()` on potentially-malformed NBT, that other path could still crash. Every reported crash on this server went through `potionLevel`, but this hasn't been exhaustively audited.
- **The underlying NBT corruption is not fixed.** Some block in the world has a `CompoundTag` containing a null key. Carry On no longer reads it, so the crash is gone, but the corruption itself is still there. It's harmless unless something else in the modpack tries to do the same thing Carry On was doing.
- **Likely source of the corruption** (not part of this patch, just for reference): one of the chest-event-handling mods on this pack — Sophisticated Storage, Lootr, or Easy Villagers — appears to be writing chest NBT in a way that produces a null key under certain conditions. The empty-double-chest reproduction case suggests the bug is specifically in how an unpaired chest's NBT is written when one half is taken via Carry On. This is worth investigating if the corruption causes problems in any other code path.