The next version of Bok’s Banging Butterflies will introduce villagers that can interact with butterflies. It’s still in the design stages, but I’m starting out by fixing a few bugs and working on what I can until I get my flow back again.
Ten Percent
When I was working at Codemasters on Operation Flashpoint: Red River I managed to reduce the networking resource requirements for the game to around 10%. The problem was that whenever a game started, there would be a spike in both the bandwidth and memory required that was up to ten times what it would use for the rest of the game.
I was set up to investigate this bug, and was determined to figure out why this was happening. After some investigation it turned out to be the way that the game was synchronising player inventories.
When a player joined a game it would send the inventory to the server. The server would then send all inventories of all players to all players. This was already bad enough, since players in the game only needed to know about the new player’s inventory – they already know about players already in the game.
But to make this worse, the players would then send back the inventories to the server, and the server would then send all inventories to all players again. Thankfully that was where it ended.
Knowing this I was able to fix it. I changed the code so that players would only ever send their own inventory to the server once and only once. I also modified it so that the server would only send all inventories to new players, and just the new inventory to players already in game.
By doing this I was able to reduce the memory budget requirement for networking to 10% of what we already had, which is important because that memory could now be used by (e.g.) the rendering systems to make the game look nicer, or the gameplay to allow more objects to be in a level.
Flashbacks
Soon after I released v5.0.0 of Bok’s Banging Butterflies, I got a bug report about players joining the game.
After I joined this mod, when I was connected to another player, I would get an in: Internal Exception: io.nettyhandler codec, EncoderExceptionio.netty,utilIllegalfeferencecountException: refcnt: 0.
I asked for a bit more information then set out to get a reproduction. It didn’t happen in 1.20.2, but when I switched to 1.20.1 I got it around 90% of the time. Checking the logs I found the culprit on the server side.
[01:12:11] [Render thread/WARN] [minecraft/ClientPacketListener]: Unknown custom packet identifier: butterflies:butterfly_data
This was starting to sound familiar. And then I realised, this was the same bug I found all those years ago on Red River. The clients were sending a message to the server they weren’t supposed to. This message was supposed to be the server syncing its version of the Butterfly Data to the clients. The server doesn’t care what data the clients have.
Obviously I needed to fix this, so I looked up the documentation for OnDatapackSyncEvent to see what I was doing wrong. This was the key – I had misunderstood how this event actually worked.
I asked for a bit more information then set out to get a reproduction. It didn’t happen in 1.20.2, but when I switched to 1.20.1 I got it around 90% of the time. Checking the logs I found the culprit on the server side.
In my implementation, I was always syncing to all players all the time, which in a LAN game would include the listen server on the host. That was the reason for the error. I was only supposed to sync to the player from getPlayer(), unless that player was null. The fix is only four characters long.
- if (event.getPlayerList() != null) {
+ else if (event.getPlayerList() != null) {
I tested this to confirm that the bug was fixed and pushed out a new release. One of these days I’ll make a release that works out of the box…
Biome Modifier Generation
In older versions of Minecraft there was a BiomeLoadingEvent that you could listen for in order to add new spawns. When adding new butterflies to the mod, it involved a lot of copying, pasting, editing, a lot of grunt work. Work that was prone to mistakes.
I thought if I could do this programmatically then it would be one less step and make adding future moths and butterflies easier. Unfortunately BiomeLoadingEvent was removed in 1.19, so I can’t do it this way anymore.
Then I realised that I could generate the spawn modifiers using the data generation script. To do this I would first need some templates, so I created a bunch by copying the biome modifiers and removing all the spawners. As an example this is the template for a forest biome modifier.
{
"biomes": "#minecraft:is_forest",
"spawners": [],
"type": "forge:add_spawns"
}
With these templates I was able to start writing a script that could generate real biome modifiers based on the butterfly data. The first thing to do is to copy the templates and overwrite the biome modifiers we already have.
# Generate biome modifiers based on rarity and habitat
def generate_biome_modifiers():
print("Generating biome modifiers...")
# Overwrite the biome modifiers with empty templates
for (_, _, files) in os.walk(BIOME_MODIFIER_TEMPLATES):
for file_ in files:
src_file = os.path.join(BIOME_MODIFIER_TEMPLATES, file_)
dst_file = os.path.join(BIOME_MODIFIERS, file_)
if os.path.exists(dst_file):
# in case of the src and dst are the same file
if os.path.samefile(src_file, dst_file):
continue
os.remove(dst_file)
shutil.copy(src_file, BIOME_MODIFIERS)
# Iterate over butterflies
for i in BUTTERFLIES + MOTHS + SPECIAL:
addSpawns(i, False)
for i in MALE_BUTTERFLIES + MALE_MOTHS:
addSpawns(i, True)
The above snippet overwrites all the biome modifiers, then calls methods to add spawns to each biome. Male butterflies/moths are separated since we don’t want to add eggs/caterpillars/chrysalises spawns for these.
The addSpawn() method reads in the butterfly data and determines what needs to be added to the modifiers based on a butterfly’s rarity and habitat. This adds the advantage that what the butterfly data says will always match what the modifiers actually do. There is no room for error anymore (provided my Python code is up to snuff).
# Add a group of spawns for a single butterfly.
def addSpawns(species, is_male):
# Open Butterfly data
with open(BUTTERFLY_DATA + species + ".json", 'r', encoding="utf8") as input_file:
json_data = json.load(input_file)
rarity = json_data["rarity"]
habitat = json_data["habitat"]
# Generate weights/min/max
if (rarity == "common"):
weight = 4
maximum = 4
elif (rarity == "uncommon"):
weight = 2
maximum = 3
else:
weight = 1
maximum = 2
# Open relevant files and add butterfly spawns
if "forests" in habitat:
addSingleSpawn("cherry_grove", species, weight, maximum, is_male)
addSingleSpawn("dense", species, weight, maximum, is_male)
addSingleSpawn("forest", species, weight, maximum, is_male)
addSingleSpawn("lush", species, weight, maximum, is_male)
addSingleSpawn("taiga", species, weight, maximum, is_male)
if "hills" in habitat:
addSingleSpawn("hill", species, weight, maximum, is_male)
if "jungles" in habitat:
addSingleSpawn("jungle", species, weight, maximum, is_male)
if "plains" in habitat:
addSingleSpawn("plains", species, weight, maximum, is_male)
if "plateaus" in habitat:
addSingleSpawn("lush", species, weight, maximum, is_male)
addSingleSpawn("plateau", species, weight, maximum, is_male)
if "savannas" in habitat:
addSingleSpawn("savanna", species, weight, maximum, is_male)
if "villages" in habitat:
addSingleSpawn("village_desert", species, weight, maximum, is_male)
addSingleSpawn("village_plains", species, weight, maximum, is_male)
addSingleSpawn("village_savanna", species, weight, maximum, is_male)
addSingleSpawn("village_snowy", species, weight, maximum, is_male)
addSingleSpawn("village_taiga", species, weight, maximum, is_male)
if "wetlands" in habitat:
addSingleSpawn("mushroom", species, weight, maximum, is_male)
addSingleSpawn("river", species, weight, maximum, is_male)
addSingleSpawn("swamp", species, weight, maximum, is_male)
To support this, I’ve added a few more habitats to the ButterflyData enumeration, as well as some localisation strings to go along with it.
The final method actually opens a biome modifier and adds the spawns to it. In the case of male butterflies, it will skip the chrysalis/caterpillar/egg spawns for male butterflies.
# Add a single spawn for a butterfly.
def addSingleSpawn(tag, species, weight, maximum, is_male):
target_file = BIOME_MODIFIERS + tag + "_butterflies.json"
with open(target_file, 'r', encoding="utf8") as input_file:
json_data = json.load(input_file)
json_data["spawners"].append({
"type" : "butterflies:" + species,
"weight" : weight,
"minCount" : 1,
"maxCount" : maximum
})
if not is_male:
json_data["spawners"].append({
"type" : "butterflies:" + species + "_caterpillar",
"weight" : weight,
"minCount" : 1,
"maxCount" : maximum
})
json_data["spawners"].append({
"type" : "butterflies:" + species + "_chrysalis",
"weight" : weight,
"minCount" : 1,
"maxCount" : maximum
})
json_data["spawners"].append({
"type" : "butterflies:" + species + "_egg",
"weight" : weight,
"minCount" : 1,
"maxCount" : maximum
})
with open(target_file, 'w', encoding="utf8") as output_file:
output_file.write(json.dumps(json_data,
default=lambda o: o.__dict__,
sort_keys=True,
indent=2))
Now there is one less step I need to think about when I add a butterfly. This means less mistakes when I add something new, and has the bonus of ensuring that what the biome modifiers do matches what the butterfly data says. It also means that if I want to add a new biome/group of biomes to the list of habitats I can do so quickly and easily.
Refactoring Code
Every project grows given enough time. Eventually you will end up with a beast that could easily become unmaintainable if you don’t trim the spaghetti. Since I’m moving on to a new feature set, I wanted to take some time to clean up the code I’d written so far.
The code I had makes heavy use of global variables and objects. Polluting the global namespace like this is always a bad code smell, so I worked on altering the structure of things so that they would rely less on global objects and be better encapsulated.
These changes were mainly in the registries and the event listeners. For the registries I replaced the statically initialised global with an object that gets created on construction.
/**
* This class registers all the entities we use with Forge's Entity Type Registry
*/
public class EntityTypeRegistry {
// An instance of a deferred registry we use to register our entity types.
private final DeferredRegister<EntityType<?>> deferredRegister;
/**
* Construction
* @param modEventBus The event bus to register with.
*/
public EntityTypeRegistry(IEventBus modEventBus) {
this.deferredRegister = DeferredRegister.create(ForgeRegistries.ENTITY_TYPES, ButterfliesMod.MOD_ID);
this.deferredRegister.register(modEventBus);
}
}
Some registries require other registries to be created, and there are some that depend on each other. So the registry objects aren’t final anymore as well as non-static. Each registry now has an initialise() method that will take any dependencies and create the registry objects we need.
public class EntityTypeRegistry {
// The block registry.
private BlockRegistry blockRegistry;
// The Butterfly Scroll entity.
private RegistryObject<EntityType<ButterflyScroll>> butterflyScroll;
// The butterfly and moth entities.
private List<RegistryObject<EntityType<? extends Butterfly>>> butterflies;
// Other entity types here
/**
* Register the entity types.
* @param blockRegistry The block registry.
*/
public void initialise(BlockRegistry blockRegistry) {
this.blockRegistry = blockRegistry;
this.butterflyScroll =
this.deferredRegister.register(ButterflyScroll.NAME, () -> EntityType.Builder.of(ButterflyScroll::create, MobCategory.MISC)
.sized(1.0f, 1.0f)
.build(ButterflyScroll.NAME));
this.butterflies = new ArrayList<>() {
{
for (int i = 0; i < ButterflySpeciesList.SPECIES.length; ++i) {
add(registerButterfly(i));
}
}
};
// Create other registry objects here.
}
}
I also added accessors to the registry objects so they can be used elsewhere in the code base.
For event listeners, I removed all the EventBusSubscriber and SubscribeEvents, replacing them with a constructor that would register the callbacks.
/**
* Holds event listeners for entities.
*/
public class EntityEventListener {
// The entity type registry.
private final EntityTypeRegistry entityTypeRegistry;
/**
* Construction
* @param forgeEventBus The event bus to register with.
*/
public EntityEventListener(IEventBus forgeEventBus,
IEventBus modEventBus,
EntityTypeRegistry entityTypeRegistry) {
forgeEventBus.register(this);
forgeEventBus.addListener(this::onJoinWorld);
modEventBus.register(this);
modEventBus.addListener(this::onEntityAttributeCreation);
modEventBus.addListener(this::onSpawnPlacementRegister);
this.entityTypeRegistry = entityTypeRegistry;
}
/**
* Register the attributes for living entities
*/
private void onEntityAttributeCreation(EntityAttributeCreationEvent event) {
// <snip>
}
/**
* On joining the world modify entities' goals so butterflies have predators.
* @param event Information for the event.
*/
private void onJoinWorld(EntityJoinLevelEvent event) {
// <snip>
}
}
I added more event listener classes and moved some other things around so it was better organised and I ended up with code that doesn’t rely on as many global objects as it used to. Now I can update the mod initialisation so that it explicitly passes any needed dependencies where they need to go.
/**
* The main entry point for the mod.
*/
@Mod(ButterfliesMod.MOD_ID)
public class ButterfliesMod
{
// Define mod id in a common place for everything to reference
public static final String MOD_ID = "butterflies";
/**
* Constructor.
*/
public ButterfliesMod() {
final IEventBus modEventBus = FMLJavaModLoadingContext.get().getModEventBus();
final IEventBus forgeEventBus = MinecraftForge.EVENT_BUS;
// Create the registries.
BlockEntityTypeRegistry blockEntityTypeRegistry = new BlockEntityTypeRegistry(modEventBus);
BlockRegistry blockRegistry = new BlockRegistry(modEventBus);
EntityTypeRegistry entityTypeRegistry = new EntityTypeRegistry(modEventBus);
ItemRegistry itemRegistry = new ItemRegistry(modEventBus);
LootModifierRegistry lootModifierRegistry = new LootModifierRegistry(modEventBus);
MenuTypeRegistry menuTypeRegistry = new MenuTypeRegistry(modEventBus);
// Initialise the registries. Do this here because (e.g.)
// blockEntityTypeRegistry requires blockRegistry to be created and
// vice-versa.
blockEntityTypeRegistry.initialise(blockRegistry, menuTypeRegistry);
blockRegistry.initialise(blockEntityTypeRegistry, menuTypeRegistry);
entityTypeRegistry.initialise(blockRegistry);
itemRegistry.initialise(blockRegistry, entityTypeRegistry);
lootModifierRegistry.initialise(itemRegistry);
menuTypeRegistry.initialise();
// Create the Mod event listeners
new ClientEventListener(modEventBus, blockEntityTypeRegistry, entityTypeRegistry);
new LifecycleEventListener(modEventBus, itemRegistry,menuTypeRegistry);
new ModEventListener(modEventBus, itemRegistry);
// Create the Forge event listeners.
new EntityEventListener(forgeEventBus, modEventBus, entityTypeRegistry);
new LevelEventListener(forgeEventBus);
new NetworkEventListener(forgeEventBus);
new PlayerEventListener(forgeEventBus);
// Mod Config Settings
ModLoadingContext.get().registerConfig(ModConfig.Type.SERVER, ButterfliesConfig.SERVER_CONFIG);
}
}
The downside is we need to pass the registries to any objects that need to use them manually, but it’s a small price to pay for a safer code base.
