Registry API - Tag events seems broken in 1.21.2/3

[MiraculixxT]

Expected behavior

Creating a new typed key and registering it via the registerEventHandler should allow using those keys inside all further postFlatteners of the same type.

Observed/Actual behavior

Using such typed key after registering still throws startup errors on usage, that the key does not exist.

Error: https://pastes.dev/WNd63g7agt

Steps/models to reproduce

Using the following code flow triggers the error for KEY1 on enchantment example.

final var KEY1 = TypedKey.create(RegistryKey.ENCHANTMENT, Key.key("namespace:key1"))

LifecycleEventManager<BootstrapContext> manager = context.getLifecycleManager();
manager.registerEventHandler(RegistryEvents.ENCHANTMENT.freeze().newHandler(event -> 
    event.registry().register(KEY1, builder -> { ... })
)));

manager.registerEventHandler(LifecycleEvents.TAGS.postFlatten(RegistryKey.ENCHANTMENT), event -> {
    final PostFlattenTagRegistrar<Enchantment> registrar = event.registrar();
    registrar.addToTag(EnchantmentTagKeys.TRADEABLE, Set.of(KEY1)); // IllegalArgumentException thrown on startup
});

Plugin and Datapack List

None

Paper version

This server is running Paper version 1.21.3-12-master@c6aa61e (2024-11-04T17:51:59Z) (Implementing API version 1.21.3-R0.1-SNAPSHOT)
You are running the latest version

Other

Using the preFlattener seems to work as a workaround for now

[lynxplay]

Caused by mojang now flattening the tags and binding them before freezing the registries as tags are part of the freezing logic in 1.21.2+.
As such the freeze event is called after the post flatten event, which leads to this explosion.
I am unsure what our move forward here is, especially for future proofing this API to avoid something like this.
Will get some more input.

[Machine-Maker]

I think the solution is to just move where we call the freeze event up a bit, and then decide on a better name for that. It was never really about the "freezing", more about the ability to add more values to the registry, so some name separate from freezing would be good. Can deprecate for removal the old event and remove in a version (since its still experimental)

[MiraculixxT]

Would this new event be a pure rename or also add changes to the behavior and usage?

[Machine-Maker]

Nah, just a rename really. Looking back, tying the name to "freeze" wasn't a good idea, it's just an event to add new stuff, has nothing to do with the freezing of the registries really. But this is what experimental is for. We'd keep the old event for compat for like a version or 2 while deprecated and then remove it.

[Machine-Maker]

@MiraculixxT in a few minutes, try downloading the build provided here and running your plugin on it to make sure it fixes the issue you had.

[MiraculixxT]

This fixes the issue! No errors on startup and all registries and modifications seems to be applied. Thank you very much for the quick fix!
(Sorry for the delayed response)