[Solved] [1.16.5] FakePlayer and AnimalEntity#getLoveCause leading to NPE

[IceMetalPunk]

I hate having to ask so many questions here in such a short time, but I tried to figure this one out on my own and hit a wall.

I have a FakePlayer that I create in the FMLServerStartingEvent, and I have a block that breeds animals using the FakePlayer as their "love cause" via AnimalEntity#setInLove. This is all working, and the animals are breeding fine. However, when a baby is born, I'm hooking into the BabyEntitySpawnEvent and trying to check whether either of the parents was bred by this FakePlayer, and if so, do some stuff (specifically, spawn a clone of the baby).

This wasn't working; at first, no errors, it just wouldn't spawn the extra baby. So I started debugging with some System.out.printlns, and boom, it started crashing with an NPE when trying to work with the event's getCausedByPlayer() value.

I dug in some more, and after some breakpoints and debugging, I found that while the UUID of the FakePlayer is properly being set as the animal's loveCause value, the getInLoveCause() method is returning null anyway because apparently the FakePlayer is not actually in the world's players list; when I test with just me in a singleplayer world, there is only one player on the list with a different UUID than the fake player (which, I have to assume, is me). Since a player with a matching UUID can't be found on the list, it returns null instead of the correct FakePlayer entity.

I thought the FakePlayer would be added to the world automatically on construction, since the world is passed to the constructor, but no big deal, right? I just added a world.addNewPlayer call in the FMLServerStartingEvent, right after constructing it, to add the FakePlayer to the world list. Except... now, Curios (which my mod supports) crashes with an NPE because apparently it's getting an EntityJoinWorld event for this fake player that doesn't have all the connection fields it's expecting (at least, it's something like that from what I can tell).

So I guess what I'm asking is, "What is the correct way to create a FakePlayer such that it's actually added to the server player list so things like AnimalEntity#getInLoveCause work properly with it?"

Here's how I'm currently creating the FakePlayers (FAKE_PLAYERS is a static hashmap of worlds to hashmaps of strings to FakePlayers, so I can look up specific players to indicate different blocks have triggered the breeding; I'm creating a set of FakePlayers per world because I believe that's necessary if I want to be able to handle this FakePlayer breeding in every dimension, but correct me if I'm wrong about that):

@SubscribeEvent
public void onServerStarting(FMLServerStartingEvent event) {
	AtomicReference<Integer> i = new AtomicReference<>(0);
	event.getServer().getAllLevels().forEach(world -> {
		int index = i.getAndAccumulate(0, (c, v) -> c + 1);
		HashMap<String, FakePlayer> playerMap = new HashMap<>();
		playerMap.put("Breeder_0", new FakePlayer(world, new GameProfile(UUID.randomUUID(), "Breeder_0_" + index)));
		playerMap.put("Breeder_1", new FakePlayer(world, new GameProfile(UUID.randomUUID(), "Breeder_1_" + index)));
		playerMap.put("Breeder_2", new FakePlayer(world, new GameProfile(UUID.randomUUID(), "Breeder_2_" + index)));
		playerMap.put("Breeder_3", new FakePlayer(world, new GameProfile(UUID.randomUUID(), "Breeder_3_" + index)));
		playerMap.put("Breeder_4", new FakePlayer(world, new GameProfile(UUID.randomUUID(), "Breeder_4_" + index)));
		FAKE_PLAYERS.put(world, playerMap);

		for (FakePlayer player : playerMap.values()) {
			world.addNewPlayer(player);
		}
	});
}

I saw the FakePlayerFactory class, but from what I can tell all that adds is automatic recordkeeping of the fake players created, and it wouldn't help me here, right?

Edited May 21, 2021 by IceMetalPunk
Solved!

[IceMetalPunk]

1 hour ago, diesieben07 said:

It would be a bad idea for a fake player to actually exist in the world I think. It's not designed to be in the world.

You should access the loveCause UUID using reflection and then check if it is one of your fake player IDs (probably best to use a fixed set for those instead of creating a random one every time).

Taking your advice, I refactored it all. I made a FakePlayerHelper class that keeps track of all the fake players, constructing the UUIDs based on the name rather than being random, reusing GameProfiles for the same names when constructing different FakePlayers for different worlds, etc. Then I used reflection in the event handler to pull the UUID from the private field in AnimalEntity directly and look that up via the helper's maps. That solved the NPEs... but now, when a baby sheep is born and triggers the BabyEntitySpawnEvent handler code, the whole game slowly locks up. As in, all entities freeze except the player, no new entities can spawn, and then eventually the player locks up, too, and I have to force quit. No errors thrown, just freezing.

Here's the new code, including all the debug output.

FakePlayerHelper class:

package com.icemetalpunk.psychicaltars.helpers;

import java.util.HashMap;
import java.util.Optional;
import java.util.UUID;

import org.apache.commons.lang3.tuple.Pair;

import com.mojang.authlib.GameProfile;

import net.minecraft.world.server.ServerWorld;
import net.minecraftforge.common.util.FakePlayer;

public class FakePlayerHelper {
	private static HashMap<Pair<ServerWorld, String>, FakePlayer> FAKE_PLAYERS = new HashMap<>();
	private static HashMap<UUID, GameProfile> UUID_PROFILE_MAP = new HashMap<>();
	private static HashMap<Pair<ServerWorld, UUID>, FakePlayer> UUID_PLAYER_MAP = new HashMap<>();

	public static FakePlayer getOrCreate(ServerWorld world, String name) {
		if (FAKE_PLAYERS.containsKey(Pair.of(world, name))) {
			return FAKE_PLAYERS.get(Pair.of(world, name));
		}
		UUID uuid = UUID.nameUUIDFromBytes(name.getBytes());
		GameProfile profile = getGameProfileFromUUID(uuid).orElseGet(() -> {
			return new GameProfile(uuid, name);
		});
		FakePlayer player = new FakePlayer(world, profile);
		FAKE_PLAYERS.put(Pair.of(world, name), player);
		UUID_PROFILE_MAP.put(uuid, profile);
		UUID_PLAYER_MAP.put(Pair.of(world, uuid), player);
		return player;
	}

	// Below is just an alias for better semantics when you don't want to store the result
	public static FakePlayer create(ServerWorld world, String name) {
		return getOrCreate(world, name);
	}

	private static Optional<GameProfile> getGameProfileFromUUID(UUID uuid) {
		return Optional.ofNullable(UUID_PROFILE_MAP.get(uuid));
	}

	public static Optional<FakePlayer> getByUUID(ServerWorld world, UUID uuid) {
		return Optional.ofNullable(UUID_PLAYER_MAP.get(Pair.of(world, uuid)));
	}
}

FMLServerStarting event handler:

@SubscribeEvent
public void onServerStarting(FMLServerStartingEvent event) {
	event.getServer().getAllLevels().forEach(world -> {
		FakePlayerHelper.create(world, "Breeder 0");
		FakePlayerHelper.create(world, "Breeder 1");
		FakePlayerHelper.create(world, "Breeder 2");
		FakePlayerHelper.create(world, "Breeder 3");
		FakePlayerHelper.create(world, "Breeder 4");
	});
}

Event handler class:

package com.icemetalpunk.psychicaltars.events;

import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.util.Optional;
import java.util.UUID;

import com.icemetalpunk.psychicaltars.helpers.FakePlayerHelper;

import net.minecraft.entity.EntityType;
import net.minecraft.entity.MobEntity;
import net.minecraft.entity.passive.AnimalEntity;
import net.minecraft.nbt.CompoundNBT;
import net.minecraft.world.World;
import net.minecraft.world.server.ServerWorld;
import net.minecraftforge.common.util.FakePlayer;
import net.minecraftforge.event.entity.living.BabyEntitySpawnEvent;
import net.minecraftforge.eventbus.api.SubscribeEvent;
import net.minecraftforge.fml.common.ObfuscationReflectionHelper;

public class PAEventHandler {
	private static Field loveCauseField = ObfuscationReflectionHelper.findField(AnimalEntity.class, "loveCause");

	@SubscribeEvent
	public void babySpawnHandler(final BabyEntitySpawnEvent event) {
		System.out.println("In handler!");
		MobEntity parentA = event.getParentA();
		MobEntity parentB = event.getParentB();
		MobEntity child = event.getChild();
		if (!(parentA instanceof AnimalEntity) || !(parentB instanceof AnimalEntity)
				|| !(child instanceof AnimalEntity)) {
			return;
		}
		AnimalEntity animalParentA = (AnimalEntity) parentA;
		AnimalEntity animalParentB = (AnimalEntity) parentB;
		AnimalEntity animalChild = (AnimalEntity) child;
		ServerWorld world = animalParentA.getServer().overworld();
		FakePlayer nullBreeder = FakePlayerHelper.getOrCreate(world, "Breeder 0");
		System.out.println("Is Animal in world " + world);
		UUID loveUUID;
		try {
			loveUUID = (UUID) loveCauseField.get(animalParentA);
		} catch (IllegalArgumentException | IllegalAccessException e) {
			e.printStackTrace();
			return;
		}
		System.out.println("UUID: " + loveUUID);
		Optional<FakePlayer> optionalPlayer = FakePlayerHelper.getByUUID(world, loveUUID);
		if (!optionalPlayer.isPresent()) {
			System.out.println("Not present");
			return;
		}
		FakePlayer player = optionalPlayer.get();
		System.out.println("Player: " + player.getName());
		CompoundNBT nbtTag = new CompoundNBT();
		animalChild.save(nbtTag);
		nbtTag.remove("UUID");
		EntityType<?> type = animalChild.getType();
		for (int i = 1; i <= 4; ++i) {
			if (player == FakePlayerHelper.getOrCreate(world, "Breeder " + i)) {
				System.out.println("Is Breeder " + i);
				animalParentA.setInLove(nullBreeder);
				animalParentA.resetLove();
				animalParentB.setInLove(nullBreeder);
				animalParentB.resetLove();
				for (int j = 0; j < i; ++i) {
					AnimalEntity babyClone;
					try {
						babyClone = animalChild.getClass().getConstructor(EntityType.class, World.class)
								.newInstance(type, world);
						babyClone.load(nbtTag.copy());
						world.addFreshEntity(babyClone);
					} catch (InstantiationException | IllegalAccessException | IllegalArgumentException
							| InvocationTargetException | NoSuchMethodException | SecurityException e) {
						e.printStackTrace();
						return;
					}
				}
				break;
			}
		}
	}
}

The output I'm getting properly recognizes and reaches "Breeder 1" (the first println within the outer 'for' loop in the event handler), but then I get no more output and everything starts locking up. Am I not cloning the baby animal correctly?

Edited May 20, 2021 by IceMetalPunk
Add clarifying comment to code

[IceMetalPunk]

58 minutes ago, diesieben07 said:

Yes, you are. Use the EntityType to create the entity, don't try to use its constructor (it might not even exist). Also, that exception "handling" is not at all appropriate.

 

You should make loveCauseField final, which will improve the reflection performance. Additionally you need to use the SRG name here or your code will not run outside the development environment.

 

Your FakePlayerHelper also has a memory leak. It holds hard references to all worlds, which makes it so that they can never be garbage collected. You have to use a WeakHashMap and ideally you don't want to pre-allocate all those FakePlayers. Instead only allocate them when needed and maybe cache them for a while.

Yeah, I'm aware the error handling is terrible; this code is very much the prototyping, "just trying to make it work first, then I'll go back and clean it up later" kind of code.

I made all of the changes you mentioned, except using the SRG name; how would I find that? I used to use MCPBot, but it seems that hasn't updated past 1.15 and loveCause isn't in those mappings. I checked in the .gradle folder and eventually found joined.tsrg, but that doesn't contain the deobfuscated names, so I'm not sure how to locate this particular field's SRG name in it.

More importantly, I switched over to using EntityType.create instead of calling the entity constructor directly, and it didn't help: the game still locks up as soon as it tries to create the entity. This is the new code in the event handler (I'm fully aware there are one or two now-unused variables); I've tried it with and without the world.addFreshEntity line, and both still lock up (I get the feeling it's freezing on creation of the new entity, before it even gets to that line anyway, but I could be wrong).

package com.icemetalpunk.psychicaltars.events;

import java.lang.reflect.Field;
import java.util.Optional;
import java.util.UUID;

import com.icemetalpunk.psychicaltars.helpers.FakePlayerHelper;

import net.minecraft.entity.Entity;
import net.minecraft.entity.EntityType;
import net.minecraft.entity.MobEntity;
import net.minecraft.entity.passive.AnimalEntity;
import net.minecraft.nbt.CompoundNBT;
import net.minecraft.world.server.ServerWorld;
import net.minecraftforge.common.util.FakePlayer;
import net.minecraftforge.event.entity.living.BabyEntitySpawnEvent;
import net.minecraftforge.eventbus.api.SubscribeEvent;
import net.minecraftforge.fml.common.ObfuscationReflectionHelper;

public class PAEventHandler {
	private static final Field loveCauseField = ObfuscationReflectionHelper.findField(AnimalEntity.class, "loveCause");

	@SubscribeEvent
	public void babySpawnHandler(final BabyEntitySpawnEvent event) {
		System.out.println("In handler!");
		MobEntity parentA = event.getParentA();
		MobEntity parentB = event.getParentB();
		MobEntity child = event.getChild();
		if (!(parentA instanceof AnimalEntity) || !(parentB instanceof AnimalEntity)
				|| !(child instanceof AnimalEntity)) {
			return;
		}
		AnimalEntity animalParentA = (AnimalEntity) parentA;
		AnimalEntity animalParentB = (AnimalEntity) parentB;
		AnimalEntity animalChild = (AnimalEntity) child;
		ServerWorld world = animalParentA.getServer().overworld();
		FakePlayer nullBreeder = FakePlayerHelper.getOrCreate(world, "Breeder 0");
		System.out.println("Is Animal in world " + world);
		UUID loveUUID;
		try {
			loveUUID = (UUID) loveCauseField.get(animalParentA);
		} catch (IllegalArgumentException | IllegalAccessException e) {
			e.printStackTrace();
			return;
		}
		System.out.println("UUID: " + loveUUID);
		Optional<FakePlayer> optionalPlayer = FakePlayerHelper.getByUUID(world, loveUUID);
		if (!optionalPlayer.isPresent()) {
			System.out.println("Not present");
			return;
		}
		FakePlayer player = optionalPlayer.get();
		System.out.println("Player: " + player.getName());
		CompoundNBT nbtTag = new CompoundNBT();
		animalChild.save(nbtTag);
		nbtTag.remove("UUID");
		EntityType<?> type = animalChild.getType();
		for (int i = 1; i <= 4; ++i) {
			if (player == FakePlayerHelper.getOrCreate(world, "Breeder " + i)) {
				System.out.println("Is Breeder " + i);
				animalParentA.setInLove(nullBreeder);
				animalParentA.resetLove();
				animalParentB.setInLove(nullBreeder);
				animalParentB.resetLove();
				for (int j = 0; j < i; ++i) {
					Optional<Entity> babyClone = EntityType.create(nbtTag.copy(), world);
					babyClone.ifPresent(baby -> world.addFreshEntity(baby));
				}
				break;
			}
		}
	}
}

 

[IceMetalPunk]

13 hours ago, diesieben07 said:

forge-bot on the Forge discord.

Thank you! That will be very useful! I've updated the reflection name to the SRG name and that part seems to still be working.

13 hours ago, diesieben07 said:

Please post a Git repo so I can debug this locally.

OK, I've just pushed up a repo: https://github.com/IceMetalPunk/psychic-altars

When testing, you'll need to create a "telepathic altar" to trigger the breeding code; which is a multiblock listed in the Patchouli handbook already. It's a Telepathic Altar block on top of any inventory, with the inventory surrounded by 8 omen-matched blocks (that is, omenstone or one of the upgrade omen types). At least one of the cardinal direction omens should be an Efficiency omen, as that's what initiates the cloning behavior. Then put a couple sheep within 1 block of the central altar block, fill the inventory with wheat, and wait for it to breed. You can right-click the altar block to validate the multiblock faster (it'll turn red when it's active). I'd suggest the other 3 upgrade omens should be speed omens, as the base speed is intentionally a slow 60 seconds per operation; with 3 speed omens, you should only need to wait about 15 seconds for it to trigger. The freezing happens once the animals breed from the altar and try to spawn a baby.

Edited May 21, 2021 by IceMetalPunk

[IceMetalPunk]

9 minutes ago, diesieben07 said:

I got the thing to work, but the altar never finds any animals, I don't know why, your AABB code seems to be wrong (at least the AABB never has any entities within it).

Also note that you store a lot of mutable data in your block class. You cannot do this. There is only one instance of your block class and it is shared even between client and server in single player. If you need to store complex mutable data you must use a tile entity.

I fully understand how blocks work. This mod started as an exercise in making functional blocks without adding tile entities. It may look like the block is storing mutable state, but it's not *really* doing that; any variables that seem like they could differ between blocks in the world are only ever used by the same piece of code that sets them, and they're always reset/re-evaluated at the beginning of any such code. I understand that internally, this means the same instance is overwriting its values for every block that evaluates them, but as that was the motivation for making this mod, I'm happy with it as long as it works. I know that it's an unusual way to handle things, and I'm okay with that.

4 minutes ago, diesieben07 said:

Your AABB code is wrong, you should be using inflate, not expandTowards.

Fixed it and then found your infinite loop: https://github.com/IceMetalPunk/psychic-altars/blob/949a1d0757e9e0cb306da9079c6bcd24816c2bf4/src/main/java/com/icemetalpunk/psychicaltars/events/PAEventHandler.java#L65. i keeps increasing, j always stays at 0. Hence j will always be lower than i and the loop never finishes.

...of course it *would* be a single-character copypasta typo at the root of all this 😂 Thank you so much! And thank you for mentioning inflate; I remember thinking the method name was a bit strange, but I couldn't find one called "expand" which I thought would be the name for the opposite of the "contract" method, and "expandTowards" was the closest, so I went with that (and it did seem to work for me in my tests somehow; I must have coincidentally set things up in the perfect position for the numbers to work out 😂).

After fixing the i -> j typo, the game no longer locks up, which is good, but... it still also doesn't work It *says* it's successfully cloning the baby sheep, but I only see one. However, if I use a command like /say or /kill, it reports the total number of sheep correctly, including clones. I though maybe there was a client/server sync piece I was missing, so I looked into how the SummonCommand works to spawn entities, and saw it's done very differently from how I was doing it. So I reworked the spawning bit using the same method SummonCommand does (where applicable), and now... same result: no visible clones, but /say and /kill report the correct number of sheep including clones.

I've pushed up the changes to the repo, but here's the new event handler if you don't feel like functionally testing it; I hope you can spot what I'm doing wrong that makes SummonCommand work but my spawning fail:

package com.icemetalpunk.psychicaltars.events;

import java.lang.reflect.Field;
import java.util.Optional;
import java.util.UUID;

import com.icemetalpunk.psychicaltars.helpers.FakePlayerHelper;

import net.minecraft.entity.Entity;
import net.minecraft.entity.EntityType;
import net.minecraft.entity.MobEntity;
import net.minecraft.entity.SpawnReason;
import net.minecraft.entity.passive.AnimalEntity;
import net.minecraft.nbt.CompoundNBT;
import net.minecraft.world.DifficultyInstance;
import net.minecraft.world.server.ServerWorld;
import net.minecraftforge.common.util.FakePlayer;
import net.minecraftforge.event.entity.living.BabyEntitySpawnEvent;
import net.minecraftforge.eventbus.api.SubscribeEvent;
import net.minecraftforge.fml.common.ObfuscationReflectionHelper;

public class PAEventHandler {
	private static final Field loveCauseField = ObfuscationReflectionHelper.findField(AnimalEntity.class,
			"field_146084_br");

	@SubscribeEvent
	public void babySpawnHandler(final BabyEntitySpawnEvent event) {
		MobEntity parentA = event.getParentA();
		MobEntity parentB = event.getParentB();
		MobEntity child = event.getChild();
		if (!(parentA instanceof AnimalEntity) || !(parentB instanceof AnimalEntity)
				|| !(child instanceof AnimalEntity)) {
			return;
		}
		AnimalEntity animalParentA = (AnimalEntity) parentA;
		AnimalEntity animalParentB = (AnimalEntity) parentB;
		AnimalEntity animalChild = (AnimalEntity) child;
		ServerWorld server = animalParentA.getServer().overworld();
		FakePlayer nullBreeder = FakePlayerHelper.getOrCreate(server, "Breeder 0");
		UUID loveUUID;
		try {
			loveUUID = (UUID) loveCauseField.get(animalParentA);
		} catch (IllegalArgumentException | IllegalAccessException e) {
			e.printStackTrace();
			return;
		}
		Optional<FakePlayer> optionalPlayer = FakePlayerHelper.getByUUID(server, loveUUID);
		if (!optionalPlayer.isPresent()) {
			return;
		}
		FakePlayer player = optionalPlayer.get();
		CompoundNBT nbtTag = new CompoundNBT();
		animalChild.save(nbtTag);
		nbtTag.remove("UUID");
		for (int i = 1; i <= 4; ++i) {
			if (player == FakePlayerHelper.getOrCreate(server, "Breeder " + i)) {
				animalParentA.setInLove(nullBreeder);
				animalParentA.resetLove();
				animalParentB.setInLove(nullBreeder);
				animalParentB.resetLove();
				for (int j = 0; j < i; ++j) {
					Entity babyClone = EntityType.loadEntityRecursive(nbtTag, server, (baby) -> {
						return baby;
					});
					if (babyClone != null) {
						DifficultyInstance diff = server.getCurrentDifficultyAt(animalChild.blockPosition());
						((AnimalEntity) babyClone).finalizeSpawn(server, diff, SpawnReason.BREEDING, null, null);
						if (server.tryAddFreshEntityWithPassengers(babyClone)) {
							System.out.println("Spawned clone " + j + "!");
						} else {
							System.out.println("Failed to tryAdd clone " + j + "!");
						}
					} else {
						System.out.println("Failed to spawn clone " + j + "!");
					}
				}
				break;
			}
		}
	}
}

 

[Draco18s]

7 hours ago, IceMetalPunk said:

any variables that seem like they could differ between blocks in the world are only ever used by the same piece of code that sets them, and they're always reset/re-evaluated at the beginning of any such code.

False:
https://github.com/IceMetalPunk/psychic-altars/blob/949a1d0757e9e0cb306da9079c6bcd24816c2bf4/src/main/java/com/icemetalpunk/psychicaltars/blocks/altar/PAAltarBlock.java#L216
https://github.com/IceMetalPunk/psychic-altars/blob/949a1d0757e9e0cb306da9079c6bcd24816c2bf4/src/main/java/com/icemetalpunk/psychicaltars/blocks/altar/PAAltarBlock.java#L218
https://github.com/IceMetalPunk/psychic-altars/blob/949a1d0757e9e0cb306da9079c6bcd24816c2bf4/src/main/java/com/icemetalpunk/psychicaltars/blocks/altar/PAAltarBlock.java#L160

 

Unrelated, this is just bad code design.
https://github.com/IceMetalPunk/psychic-altars/blob/949a1d0757e9e0cb306da9079c6bcd24816c2bf4/src/main/java/com/icemetalpunk/psychicaltars/blocks/PACraftContainerBlock.java#L52-L57
https://github.com/IceMetalPunk/psychic-altars/blob/949a1d0757e9e0cb306da9079c6bcd24816c2bf4/src/main/java/com/icemetalpunk/psychicaltars/blocks/PABlock.java
1) that default getRenderLayer is terrible because it's objectively wrong. That should never return null.
2) you shouldn't be using the Registry events any more and this loop through thing is dumb. instead of calling myBlock.register(event) you could just do event.register(myBlock) and not need these methods at all. Ditto your items.
3) block items should be aware of their blocks. Blocks should not be aware of their block items. Items are registered after blocks for a reason.

The fuck is this used for and why.
https://github.com/IceMetalPunk/psychic-altars/blob/949a1d0757e9e0cb306da9079c6bcd24816c2bf4/src/main/java/com/icemetalpunk/psychicaltars/blocks/OmenBlock.java#L12

 

[IceMetalPunk]

3 hours ago, Draco18s said:

False:
https://github.com/IceMetalPunk/psychic-altars/blob/949a1d0757e9e0cb306da9079c6bcd24816c2bf4/src/main/java/com/icemetalpunk/psychicaltars/blocks/altar/PAAltarBlock.java#L216
https://github.com/IceMetalPunk/psychic-altars/blob/949a1d0757e9e0cb306da9079c6bcd24816c2bf4/src/main/java/com/icemetalpunk/psychicaltars/blocks/altar/PAAltarBlock.java#L218
https://github.com/IceMetalPunk/psychic-altars/blob/949a1d0757e9e0cb306da9079c6bcd24816c2bf4/src/main/java/com/icemetalpunk/psychicaltars/blocks/altar/PAAltarBlock.java#L160

 

Unrelated, this is just bad code design.
https://github.com/IceMetalPunk/psychic-altars/blob/949a1d0757e9e0cb306da9079c6bcd24816c2bf4/src/main/java/com/icemetalpunk/psychicaltars/blocks/PACraftContainerBlock.java#L52-L57
https://github.com/IceMetalPunk/psychic-altars/blob/949a1d0757e9e0cb306da9079c6bcd24816c2bf4/src/main/java/com/icemetalpunk/psychicaltars/blocks/PABlock.java
1) that default getRenderLayer is terrible because it's objectively wrong. That should never return null.
2) you shouldn't be using the Registry events any more and this loop through thing is dumb. instead of calling myBlock.register(event) you could just do event.register(myBlock) and not need these methods at all. Ditto your items.
3) block items should be aware of their blocks. Blocks should not be aware of their block items. Items are registered after blocks for a reason.

The fuck is this used for and why.
https://github.com/IceMetalPunk/psychic-altars/blob/949a1d0757e9e0cb306da9079c6bcd24816c2bf4/src/main/java/com/icemetalpunk/psychicaltars/blocks/OmenBlock.java#L12

 

this.isValid and this.tier are (re)set in the validate() method, which is called right before they're used, so they're both definitely re-evaluated immediately before use. getRangeBB is only being used by operate() method overrides, which is called also in the code immediately following a (re)set in validate(). None of those variables are ever being used without first being re-evaluated

getRenderLayer is only meant to indicate when a block doesn't use the default layer so it can be set accordingly. Is RenderType.SOLID the default? If so, I can make that the default return value instead.

You're right about the block registry; I thought I was going to have those PABlock#register methods do a bit more when I first structured it, but then ended up not, so it's an artifact of that. I can easily change that over to remove that method, thanks.

The thing about BlockItems, though, is that I designed the system so I could just add blocks and they'd automatically add their own BlockItems without me having to do twice as much work for every block I add. I suppose I can refactor such that the ItemBlock is created locally in the block's registerBlockItem method instead of stored in a class field, but that's nearly the same code -- what exactly is the benefit of rejecting any reference to the ItemBlock in the block class and manually doubling the amount of new classes I make?

The allOmens list in OmenBlock is used for the Patchouli rendering of the multiblock structures. Since each omen is a separate block rather than a block state property variant (which was a choice that I'm not passionate about nor passionate against), to have the render cycle among all the omen types I needed a way to get a list of all those omen blocks, so I just created that list.

I hope this answers your questions

[IceMetalPunk]

4 hours ago, diesieben07 said:

They really shouldn't be fields in the block class then and instead method parameters.

 

The reason your cloned children immediately get removed is because you spawn them at 0, 0, 0. When BabyEntitySpawnEvent is triggered the baby animal is not at the correct position yet.

I suppose I could have the validate() method return either a triple or an optional pair to indicate isValid, tier, and range, rather than storing it in the block class's fields. That's definitely something I'll look into, thanks for the suggestion!

I didn't realize the BabyEntitySpawnEvent fired before the baby was moved to the correct position, but looking at the breeding method now with that in mind, I see it. After adding a moveTo for the clones in the event handler, everything is working perfectly now!

Thank you for all your help with everything! ❤️

[Draco18s]

15 minutes ago, IceMetalPunk said:

this.isValid and this.tier are (re)set in the validate() method, which is called right before they're used, so they're both definitely re-evaluated immediately before use.

Learn about the out keyword.

15 minutes ago, IceMetalPunk said:

Is RenderType.SOLID the default? 

Yes. I still don't think you really need this method/interface, but not a huge deal. 

  

15 minutes ago, IceMetalPunk said:

and they'd automatically add their own BlockItems without me having to do twice as much work for every block I add.

foreach(Block block : MyBlocks) {
    BlockItem bi = new BlockItem(block)
    event.register(bi);
}

If you want a custom block item, you'll have to treat it special anyway.

Edited May 21, 2021 by Draco18s

[Choonster]

1 minute ago, Draco18s said:

Learn about the out keyword.

I think you're mixing up C# and Java.

[Draco18s]

Just now, Choonster said:

I think you're mixing up C# and Java.

Touche. You can simulate it with the ref keyword, or by returning a Tuple.

[Choonster]

2 minutes ago, Draco18s said:

Touche. You can simulate it with the ref keyword, or by returning a Tuple.

Java doesn't have ref either, but does have tuples (from Minecraft and Apache Commons Lang).

[Draco18s]

I swear I've done ref things in my modding work before.

Eh. Oh well.