[1.20.4/5] Create BonusLootDropsEvent

[AnonymousHacker1279]

Create a BonusLootDropsEvent which is fired before dropping items via the ApplyBonusCount loot function (for example, blocks with Fortune modifiers). It allows enchantment levels to be modified or a specific drop count to be set.

All of the variables/parameters available in ApplyBonusCount.run() are exposed through the event.

Example Usages:

@SubscribeEvent
public static void bonusLootDropsEvent(BonusLootDropsEvent event) {
	// Add two extra enchantment levels when specific conditions are met
	if (event.getEntity() instanceof Player player) {
		if (playerMeetsSpecificConditions()) {
			event.setEnchantmentLevel(event.getEnchantmentLevel() + 2);
		}
	}
}

@SubscribeEvent
public static void bonusLootDropsEvent(BonusLootDropsEvent event) {
	// Always set the drops to three when specific conditions are met
	if (event.getEntity() instanceof Player player) {
		if (playerMeetsSpecificConditions()) {
			event.setDropCount(3);
		}
	}
}

[CLAassistant]

All committers have signed the CLA.

[neoforged-pr-publishing[bot]]

• [ ] Publish PR to GitHub Packages

[Shadows-of-Fire]

Not 100% convinced this is a great solution, especially because changing the enchantment level is totally superseded by someone else modifying the drop count.

[AnonymousHacker1279]

The drop count method intentionally acts as a final say-so in case a mod decides it needs to force it for any reason. I can change this if desired though.

[AnonymousHacker1279]

My apologies, I did not catch the merge failure when updating my branch through GitHub Desktop. I have force-pushed this branch from a fresh one.

[marchermans]

Okey that merge disaster is indeed fixed, now to the content.

Would a GLM not work here?

[AnonymousHacker1279]

The problems with GLMs for this particular use scenario is that they run after loot is generated, and Fortune-style modifiers run before.

There was a more extended discussion on Discord regarding this.

[KnightMiner]

In the past, the forge recommendation for how to deal with this issue is to use a GLM to delete all generated loot then create a modified context to rerun the loot table. Obviously this is a horrible solution as it means other GLMs trying a similar approach are clobbered, but this is why this has never been addressed.

Not sure if the silk touch GLM doing that is still around, that case at least is long since handled by item enchantment overrides.

---

This PR to me seems like it has notable overlap #503 which was recently merged. It really needs a javadoc on both events explaining when this should be used over that and vice versa.

The only usecase for the enchantment side of this event I can think of is if you need to set enchantments conditioned on the details of the loot context (e.g. you increase fortune conditioned on the block being mined) as the enchantment event is better for other cases as it lets non-loot table contexts properly query the enchantment. I'd normally say you should add an example of why you need that functionality, but in this case its functionality I use in one of my mods (fortune conditioned on only a subset of blocks that can be mined)

[AnonymousHacker1279]

I was referred to the GetEnchantmentLevelEvent introduced in #503 however my particular use case was being able to adjust Fortune levels based on an armor set bonus, meaning I needed a player context of some sort. It was mentioned in Discord that ideally the GetEnchantmentLevelEvent would contain this context, but the patch surface was too high to be feasible.

Later this evening I can improve the JavaDocs on this event to explain when it should be used over the other event.

[sciwhiz12]

Hello! Do you intend to continue with this PR? If so, please fix the merge conflicts.

[AnonymousHacker1279]

I have no intentions of continuing the PR at this point.