ServerEntityEvents.ALLOW_FRESH_LOAD and AFTER_FRESH_LOAD event

[bleudev]

Add two simple server-side events which calls before/after a living entity was added to level (spawned). It is useful for developers who wants do something before/after entity spawn.

Sample:

// Allow fresh load sample
ServerEntityEvents.ALLOW_FRESH_LOAD.register((entity, level) -> {
	if (entity instanceof Creeper creeper) {
		// Spawn with 25% chance zombie instead of creeper
		if (level.getRandom().nextFloat() <= 0.25f) {
			var zombie = EntityType.ZOMBIE.create(level, EntitySpawnReason.EVENT);
			if (zombie != null) {
				zombie.copyPosition(creeper);
				level.addFreshEntity(zombie);
				return false;
			}
		}
	}
	return true;
});

[modmuss50]

What is the difference between this and ServerEntityEvents.ENTITY_LOAD?

[bleudev]

What is the difference between this and ServerEntityEvents.ENTITY_LOAD?

The entity load event is called when an entity is loaded upon creation, as well as when the server starts once the entity has been added. My event is called only when an entity is added to the server.

[modmuss50]

Could you use data attachments to keep track of if its the first time the entity was added to the world? E.g store a marker to indicate that your mod has done what it needs to the entity?

[cassiancc]

I'm personally happier with this than having to needlessly keep track of things myself - I've had to mixin in order to do this instead of having an equivalent event to the one on NeoForge. It'd save on unnecessary calls to mod logic by only running when needed instead of every time the entity is loaded/unloaded.

[modmuss50]

At the very least please fix the build issues, and explain the documentation what its for, why it differs from the other and when its fired.

[Patbox]

Would it maybe make more sense to run the event before entity is actually added to world rather than after? I think for cases where you want to modify it in some way it would be probably cleaner (and maybe allow canceling that within the event).

Maybe rename would be good, since just "adding" an entity feels little bit unspecific, maybe calling it "ON_FRESHLY_ADDED" would be better (since it's more descriptive). Probably would make sense to document it too that it's called when ServerLevel#addFreshEntity runs

[DennisOchulor]

Could this be expanded to cover all entities? I'm thinking something like ServerEntityEvents.BEFORE_FRESH_ENTITY_LOAD that's cancellable.

Then ENTITY_LOAD would need some indicator of whether the entity was freshly added or not, similar to #5112 for chunks.
This is for use cases where you want to do some action only if the entity is definetly being spawned, which may not be the case in the cancellable event.

[bleudev]

Could this be expanded to cover all entities? I'm thinking something like ServerEntityEvents.BEFORE_FRESH_ENTITY_LOAD that's cancellable.

Then ENTITY_LOAD would need some indicator of whether the entity was freshly added or not, similar to #5112 for chunks. This is for use cases where you want to do some action only if the entity is definetly being spawned, which may not be the case in the cancellable event.

Great idea, I thought about it before.

But parameters (similar to chuck load event) - i think no. It will be unnecessary breaking change

[bleudev]

Could you use data attachments to keep track of if its the first time the entity was added to the world? E.g store a marker to indicate that your mod has done what it needs to the entity?

What do you mean? Sorry for my misunderstanding)

[bleudev]

Would it maybe make more sense to run the event before entity is actually added to world rather than after? I think for cases where you want to modify it in some way it would be probably cleaner (and maybe allow canceling that within the event).

Maybe rename would be good, since just "adding" an entity feels little bit unspecific, maybe calling it "ON_FRESHLY_ADDED" would be better (since it's more descriptive). Probably would make sense to document it too that it's called when ServerLevel#addFreshEntity runs

Maybe i will do something like BEFORE_FRESH_LOAD and AFTER_FRESH_LOAD?

[bleudev]

Done some recommendations. Can anyone review?

[bleudev]

I can't check build issues, workflow awaiting approval

[DennisOchulor]

Then ENTITY_LOAD would need some indicator of whether the entity was freshly added or not, similar to #5112 for chunks.

I still think it would be better to add to ENTITY_LOAD instead of a new AFTER_FRESH_LOAD event.

Yes it's a breaking change but if #5112 was considered fine I don't see why this would be unacceptable.

[bleudev]

Yes it's a breaking change but if #5112 was considered fine I don't see why this would be unacceptable.

Maybe you're right. Probably, we can do breaking changes cause it's snapshots. But I'm waiting for @modmuss50. What he will say about it?

[bleudev]

I've fixed all issues from checkStyleMain task. Can anyone approve workflow again?

[bleudev]

Will be this pull request reviewed?

[modmuss50]

This still doesnt explain when this is invoked and why its different from the other entity events.

Im a little bit concerned about this injection point as it doenst really do anything special: https://mcsrc.dev/1/26.1-snapshot-11/net/minecraft/server/level/ServerLevel#L951-953 and is possibly going to miss certain types of spawns.

[modmuss50]

I do wonder if the regular ENTITY_LOAD event could do with some sort of context instead? E.g being able to target only spawn eggs, or natural spawns. It would be a lot more robust than just another event that is fired within a questionable method.

[modmuss50]

Been thinking some more, could this event just be replaced by:

	public static ScopedValue<LoadContext> LOAD_CONTEXT = ScopedValue.newInstance();

	public enum LoadContext {
		SPAWN_EGG,
		NATURAL_SPAWN,
		/// Others here
	}

in ServerEntityEvents? And then you would just need some trivial mixins in the specific places you care about to set the context around the addEntity calls?

[bleudev]

Been thinking some more, could this event just be replaced by:

	public static ScopedValue<LoadContext> LOAD_CONTEXT = ScopedValue.newInstance();

	public enum LoadContext {
		SPAWN_EGG,
		NATURAL_SPAWN,
		/// Others here
	}

in ServerEntityEvents? And then you would just need some trivial mixins in the specific places you care about to set the context around the addEntity calls?

Okay, it's great idea. I will try to do that

[bleudev]

Been thinking some more, could this event just be replaced by:

	public static ScopedValue<LoadContext> LOAD_CONTEXT = ScopedValue.newInstance();

	public enum LoadContext {
		SPAWN_EGG,
		NATURAL_SPAWN,
		/// Others here
	}

in ServerEntityEvents? And then you would just need some trivial mixins in the specific places you care about to set the context around the addEntity calls?

I have a question. Do you suggest to remove FRESH_LOAD events and use LOAD_CONTEXT in code? If you do, i suggest to rename ALLOW_FRESH_LOAD to ALLOW_ADD and do not remove it. It's simple way to discard entity add before adding.

[bleudev]

I've probably did it, but not sure (I can't run Fabric Gradle project for some reason)

[bleudev]

I've used EntitySpawnReason cause it contains all need reasons and it is used in EntityType.spawn() method