Skip to content

Lifecycle and Registry API#8789

Closed
maestro-denery wants to merge 8 commits into
PaperMC:masterfrom
maestro-denery:denery-plugin-work
Closed

Lifecycle and Registry API#8789
maestro-denery wants to merge 8 commits into
PaperMC:masterfrom
maestro-denery:denery-plugin-work

Conversation

@maestro-denery
Copy link
Copy Markdown
Contributor

@maestro-denery maestro-denery commented Jan 14, 2023

This PR depends on #8108. Owen said "Feel free to open a draft PR with all of this", so I did it. This is a draft PR and is highly WIP, but I appreciate any feedback on this.

Lifecycle API reasons

Aside from the early plugin initialization we also need a system to be able to control stages when we mutate various states of a server. Mainly, by "states" I mean various stages of the registry system initialization. Since the 1.19.3 update the whole registry system became even more complex, development in this PR also aims to cover those stages in a nice and understandable way for a plugin developer to interact with, and cover them for the future APIs which need to mutate registries in order to make their stuff work. Porting the bukkit event system in those early stages of the initialization is bad and too hacky. We don't even need a complete event system for such purposes, because we don't want to be able to cancel those stages and we need to keep it simple.

Registry API reasons

Minecraft becomes more data-driven. Nowadays, there are datapacks which allow to register custom biomes, dimension types, dimensions, and all this content synchronizes with clients joining a server. In the 1.19.3 version Mojang made large changes to this system to make feature flags possible. But, unfortunately, Bukkit has never been a platform intended for APIs adding game content, and when Mojang allowed to do such things, Bukkit wasn't ready for such changes and a lot of API functionality was thrown away. So, this API is intended to fill that gap, and provide registry abstractions in a maintainable and fancy to use way. As we can see, Mojang actively develops this system and many new content they make relies on it, and we can no longer ignore its existence.

Lifecycle API main concepts

There is a new io.papermc.paper.lifecycle package where the API is located. The main component is LifecyclePoint which represents some point in a server initialization and allows user to schedule their functionality on them. There are two types of them: SingleEnter and MultiEnter. The first one is executed only once during the whole server runtime and it errors if a user attempts to schedule something after that point of initialization has reached. (i.e. static registries initialization, WorldStem initialization, MinecraftServer ) The second one is execute more than one time. (i.e. Datapack reloading)

Notes

  • All implementations of LifecyclePoint are thread-safe, (using atomic classes) because we mess around with both main and server threads, or user may schedule from other threads.
  • Consumer<C> are stored as one large linked consumer in the each LifecyclePoint (every new passed consumer merges with the global one), because it's the simplest way of storing and entering it. If there are some disadvantages I'll make it use some collection.

TODO

  • Make basic lifecycle event system.
  • Make a system for the elegant NMS to API and vice versa conversion.
  • Implement Registry, WritableRegistry, ResourceKey in the API.
  • Figure out what to do with the Monad-like API design for the lifecycle event subscription / handling.
  • Figure out what to do with org.bukkit.Registry?
  • Implement RegistryAccess, LayeredRegistryAccess in the API, figure out how to deal with RegistryLayers.
  • Figure out how to deal with types / access for registries in the API.
  • Make more LifecyclePoints for the LayeredRegistryAccess initialization in WorldLoader.
  • Figure out what to do with worlds in bukkit API, implement LevelStem registry to the API.
  • Implement more Resource / Datapack stuff in the API.
  • Implement FeatureFlags in the API.
  • Implement Tags and TagKeys system for the Registry API.
  • Implement a smart way to deal with Bukkit's enums for making future APIs.
  • Figure out what to do with an API for allowing modifying values right before their registration. (i.e. Existing entities properties modification)
  • Make LifecyclePoints for more places.
  • Discuss exception handling policy for the lifecycle api.
  • Discuss allowing plugins to have datapacks inside their resources
  • Registry (de-)serialization API?
  • Document everything, make unit tests, final clean up.

@maestro-denery maestro-denery requested a review from a team as a code owner January 14, 2023 20:44
@Owen1212055 Owen1212055 added the status: blocked Issue or Request is waiting on some other issue or change. label Jan 14, 2023
@maestro-denery maestro-denery marked this pull request as draft January 14, 2023 20:49
@maestro-denery
Copy link
Copy Markdown
Contributor Author

maestro-denery commented Jan 15, 2023

I've just made a LifecyclePointContext. This is how LifecyclePoint scheduling looks now:

    @Override
    public void bootstrap(@NotNull PluginBootstrapContext context) {
        final LifecyclePointContext lifecyclePointContext = context.getLifecyclePointContext();
        lifecyclePointContext.schedule(Lifecycles.REGISTRIES_INITIALIZED, registryInitializationContext -> {
            RegistryAccess.getInstance().registerSomethingInStatic(); // regsiters new MemoryModuleType.
        }).schedule(Lifecycles.REGISTRIES_FROZEN, registryFrozenContext -> {
            RegistryAccess.getInstance().obtainAndSOUTSomethingRegistered(); // obtains the registered MemoryModuleType from a registry after it's frozen.
        });
    }

I don't like that users could schedule lifecycles in a plain JavaPlugin class using the API, which of course wouldn't work and they would be confused, so now it's done only using LifecyclePointContext which is provided by PluginBootstrapContext. If a user wants to schedule their functionality in other places they need explicitly pass the LifecyclePointContext to that place.

Maybe I'll do even more restrictions on that, but for now I think I'll further analyze the registry initialization process and make more lifecycle points to see how future APIs for various things should be done.

@maestro-denery maestro-denery changed the title Paper Plugins PR + Lifecycle API Paper Plugins PR + Lifecycle, Registry API Jan 25, 2023
@maestro-denery
Copy link
Copy Markdown
Contributor Author

maestro-denery commented Jan 25, 2023

Changed the name of the PR, made a large TODO list in the description. I implemented registry functionality in the ExtendedRegistry<T extends Keyed> and WritebleRegistry<T extends Keyed> and made an interesting NMS to API and vice versa conversion system, see io.papermc.paper.registry.Converters, it makes conversion lazy and cached, looks better than PaperRegistry conversion system made by MachineMaker. If you want to see an example using the biome registry, see tests. Also LifecyclePoint scheduling looks like this:

    @Override
    public void bootstrap(@NotNull PluginBootstrapContext context) {
        context.getLifecyclePointContext().schedule(Lifecycles.REGISTRIES_INITIALIZED, registryInitializationContext -> {
            ResourceKey<? extends ExtendedRegistry<Biome>> resourceKey = ResourceKey.<Biome>create(Key.key("testplugin", "something"));
        }).schedule(Lifecycles.REGISTRIES_FROZEN, registryFrozenContext -> {
            // Doing something
        }).build(context.getConfiguration());
    }

As you can see now you have to provide PluginMeta to register provided functionality, it's made for the better exception handling. Once the provided LifecyclePointScheduler is built you can't schedule more functionality in it.

@maestro-denery
Copy link
Copy Markdown
Contributor Author

I made huge additions, and I'd like to hear a feedback on them to know whether there is something to refactor or not before I start to make more stuff. So TestPluginBootstrap#bootstrap() now looks like this:

    @Override
    public void bootstrap(@NotNull PluginBootstrapContext context) {

        context.getLifecyclePointScheduler().schedule(ServerLifecyclePoints.WORLDGEN_REGISTRIES_INITIALIZED, registryInitializationContext -> {
            final WritableRegistry<Biome> biomeRegistry = registryInitializationContext.writableRegistryAccess()
                .registry(ExtendedRegistry.BIOME_REGISTRY_KEY);

            biomeRegistry.register(
                ResourceKey.create(biomeRegistry.resourceKey(), Key.key("test", "mybiome")),
                Biome.CUSTOM // What to do with enums?
            );

        }).schedule(ServerLifecyclePoints.WORLDGEN_REGISTRIES_FROZEN, worldgenRegistryFrozenContext -> {
            final ExtendedRegistry<Biome> biomeRegistry = worldgenRegistryFrozenContext.registryAccess()
                .registry(ExtendedRegistry.BIOME_REGISTRY_KEY).orElseThrow();

            final ResourceKey<Biome> biomeResourceKey = biomeRegistry.resourceKey(Biome.BAMBOO_JUNGLE).orElseThrow();
            System.out.println("resource key: " + biomeResourceKey);
        }).build(context.getConfiguration());
    }

In it I do an example "registration" of custom biome when worldgen registries initialized, but not yet froze. (which fails, because for now we don't have a flexible abstraction for biomes to make new ones), and when worldgen registries are frozen, I get a ResourceKey<T> of a bamboo jungle biome in print it, and it works:

[18:36:10 INFO]: [STDOUT]: resource key: [ minecraft:worldgen/biome / minecraft:bamboo_jungle ]

Now let's see how we get access for writable registries in various stages of initialization. I made a RegistryKey<L, E> API which makes it flexible and totally safe. First of all, we don't have any "static" instances of registries in the API, we have only keys for them, which we can use in lifecycle points of their initialization / freezing. Let's see how the BIOME_REGISTRY_KEY is defined in the ExtendedRegistry interface:

RegistryKey.MutableRegistryKey<RegistryKey.RegistryLayerType.WORLDGEN, Biome> BIOME_REGISTRY_KEY = API.registryKeyFactory.createMutable(Biome.class);

First of all, as we can see the type itself is MutableRegistryKey, which means that a user can safely mutate the registry obtainable by this key. Second of all, we see that it has RegistryKey.RegistryLayerType.WORLDGEN it means that the registry this key refers to is a worldgen one. This generic is used in WritableRegistryAccess<L>, it restricts which types of WritableRegistry are allowed to be obtained and later are safe to mutate in the given WritableRegistryAccess<L> instance. So, we can get WritableRegistry<T> only and only if it's key type is MutableRegistryKey and the generic of WritableRegistryAccess<L> matches the generic of the given key. That's how I made the system safe. But now I wanna hear a feedback on some questions:

  1. Which exception handling policy should we use if some exception is thrown in LifecyclePoint#schedule() method? (Imo we should simply shut down the server)
  2. How to deal with namespaced / bukkit keys in this API? (the generic in ExtendedRegistry<T> extends org.bukkit.Keyed interface, but I'd like it either to have Kyori's one, or even not to have any, but in this case I don't know what to do with org.bukkit.Registry)
  3. Should we allow API users to make their own registries for better cross plugin compatibility like it's done in modding APIs? (I think we should)
  4. Should we allow serialization / deserialization of registries in the API? If so, how would it be implemented in the API? (a half of 1.19.3 registry system is made purely for (de-)serialization purposes, because mojang made the vanilla datapack)
  5. How to deal with bukkit enums like Biome? (for now I see so a few good solutions for allowing registering custom things if there are already bukkit enums for them. We can make an interface like FlexibleBiome, make bukkit's Biome extend it and spread this interface across the API, but this is also a shoddy solution...)
  6. Should we really make an API for property modification of already registered things like Item or EntityType right before they're registered? (i.e. allow changing a hitbox of entities, allow changing resistance of blocks.)
  7. Should we allow defining a datapack in the plugin's resources/ like mods do? (And also make some API for that in the PluginBootstrapContext)

@maestro-denery maestro-denery changed the title Paper Plugins PR + Lifecycle, Registry API Lifecycle and Registry API Feb 19, 2023
@maestro-denery maestro-denery marked this pull request as ready for review February 19, 2023 21:54
@maestro-denery
Copy link
Copy Markdown
Contributor Author

Rebased, no longer draft.

Rename to paper meta

Fix provided plugins causing plugins to be loaded multiple times

Combine config api

rebase

Cleanup, documentation, fixes

Update logic

Update some debugging utilities

Bootstrap debugging + wtf config file delete moment

Move entrypoint above bootstrap logic

Move library resolution condition check outside of method

Cherrypick "Also load resources from LibraryLoader"

Renames, annotations, service api bytecode provider

Rebase

Unit tests + build fixing

Permission fixes + misc plugin loading changes

List patches to be squashed

Fix dependency issues + don't allow dupe plugins + switch legacy loading setting + fix constructors

Remove ConfigurationSerializable, remove unused classes

Dependency handling for plugins + bootstrap

Move papertest plugin package, fix package check

Better api support, deprecate PluginLoader for removal

Painfully adding support for api, this prolly doesn't work don't even bother

rename

Javadocs, bug fixes, platform classloaders, move a bunch of stuff to server, annotation test changes, fix plugin loading with plugins that have no dependencies, cleanup classloaders, support to create your own javaplugins

Configurate improvements + misc changes

Move loggers + cleanup provider loading

Dependency handling + dependency fixes

Plugin loader and plugin boostrapper

Fail if initializer fails, remove plugin initializers

New soft utilizes

New classloader groups, remove utilizes, open classloader config option

Add more backwards compatability for DummyBukkitPluginLoader

Synchronize groups and fix resolving dependencies

Rebase

Boostrapper dependencies

Draft Lifecycle API

Fix Merge conflict in patches

Introduce LifecyclePointContext

Fix annotations

Fix server test

Better LifecyclePoint scheduling, more exception checks

Fix LifecyclePointContextImpl

Draft Registry API

tiny fixes

Make RegistryAccess, RegistryKey types, worldgen registries LifecyclePoints

fix tests

Enum workaround

Draft Biome API

rebase
@Owen1212055 Owen1212055 removed the status: blocked Issue or Request is waiting on some other issue or change. label Feb 23, 2023
Comment on lines +122 to +124
+ map.put(RegistryKey.RegistryLayerType.STATIC.class, ServerLifecyclePoints.STATIC_REGISTRIES_INITIALIZED);
+ map.put(RegistryKey.RegistryLayerType.WORLDGEN.class, ServerLifecyclePoints.WORLDGEN_REGISTRIES_INITIALIZED);
+ }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess i am trying to wrap my head around these registry layer types. Specifically, why do these types need to be denoted separately?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These types are used in RegistryKey in a generic as a compile-time check, so users in WritableRegistryAccess can get WritableRegistry only if the RegistryKey's LayerType generic matches the generic of WritableRegistryAccess, so people won't misuse lifecycle api, i.e. try to get a worldgen registry from the static registry access.

Comment on lines +1115 to +1120
+ public <API extends Enum<API> & Keyed, NMS> EnumWritableRegistry<API> getOrCreateRegistry(RegistryKey.EnumMutableRegistryKey<? extends RegistryKey.RegistryLayerType, API> registryKey, net.minecraft.core.Registry<NMS> registry) {
+ Converters.Converter<ExtendedRegistry<API>, net.minecraft.core.Registry<NMS>> converter = (Converters.Converter<ExtendedRegistry<API>, net.minecraft.core.Registry<NMS>>) this.keyConverterMap.get(registryKey);
+ EnumWritableRegistry<API> apiRegistry = (EnumWritableRegistry<API>) converter.convertNMS(registry);
+ this.apiRegistryPerRegistryKey.put(registryKey, apiRegistry);
+ return apiRegistry;
+ }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding this, is this only used in order to dynamically add new enum types?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lines you marked there are used to get or create API EnumWritableRegistry from the NMS one. EnumWritableRegistry is an API interface which is used to specifically register Enums types by users, I made it because Bukkit enums implement the Keyed interface and have NamespacedKey as a field, we don't need to have Key or ResourceKey param in the register method in the WritableRegistry, because the Enum already has the namespaced key when the user created it, so EnumWritableRegistry#register() method contains only E extends Enum<E> & Keyed as a param, and gets the key from it to register it.

@maestro-denery
Copy link
Copy Markdown
Contributor Author

I think this PR is done. I don't think it's worth to make it even bigger, TODOs I described but didn't finished I'll comment on the paper plugin future planning issue and make other PRs for them. This PR also includes basic Biome creation API which allows users to create new Biome enum instances with custom climate and special effects settings, other features such as mob spawn and generation will be added in the future. This API also allows getting instances of custom / datapack biomes using new methods in RegionAccessor using the same system. See TestPlugin#onPlayerJoin.
So, the enum construction and its registration looks like this:

    @Override
    public void bootstrap(@NotNull PluginProviderContext context) {
        final Biome biome = Biome.builder()
            .climateSettings(ClimateSettings.builder().build())
            .specialEffects(BiomeSpecialEffects.builder().grassColor(Color.BLACK).skyColor(Color.BLACK).build())
            .build(Key.key("test", "example"))
            .register();
    }        

the #build(Key) method returns RegsitryInstance<Biome> which a user either can #register() and get a value, or get a #value() without the registration. Note that #register doesn't exactly registers, it schedules registration to the point when the registry is modifiable, it means that this whole builder can be placed as a field in the TestPluginBootstrap and the value registers automatically. Note that the LifecyclePoint system isn't gone, it gives users more functionality, but as you can see the API doesn't force to use it for simple cases, due to its complexity. That's all, waiting for reviews and minor fixes I guess.

@Machine-Maker
Copy link
Copy Markdown
Member

I really think that this PR is over engineered for what we need. I don’t understand why we need to replicate the registry layers, especially when 2 of them really don’t matter to us. DIMENSIONS doesn’t matter to us, that’s just all the worlds, those don’t have to be a registry in the API. The RELOADABLE also doesn’t really matter, as those can be changed at any point theoretically.

Also, a lot of how this API should be designed will be based on how we want to expose changing some of the types. Like for example, GameEvent. That’s a built in registry, but there is no api for changing or adding new game events. This sounds like it would be useful to have, but the GameEvent ctor is private, there’s no setRange method. I don’t think the solution is to add those two things, cause most of the time they should be considered “immutable”.

And ^ will depend quite a bit on upstream’s enum removal PR. I don’t think this should happpen before that is merged (or before we just decide to do it ourselves).

@maestro-denery
Copy link
Copy Markdown
Contributor Author

Responding to MM, I agree that DIMENSION and RELOADABLE aren't needed, but I don't think we should remove a distinction between static and worldgen ones, as I said in the review to Owen, for users it's a compile time check used in WritableRegistryAccess (Registry access which gives WritableRegistry), nothing more.

Speaking of an API for the content modification, I can think of some property system, maybe even using the same abstractions Owen made in the Property API. I can think that maybe Reference<Something> should have some kind of generic based property modification system, and it allows to modify them only if that Reference is unbound to a registry yet.

Speaking of the enum removal PR, I think it won't make any API breaks as long as users don't use things marked with @ApiStatus.Internal which they ofc shouldn't, if Biome suddenly becomes an object we'll make it use new Biome() instead of EnumCreators in the #build(Key) method, the builder will stay valid anyway.

There are also things I'd like to have an opinion on, the first one is naming, especially ExtendedRegistry, WritableRegistry, RegistryAccess and WritableRegistryAccess, I think WritableRegistry can also be called MutableRegistry, RegistryAccess can be named RegistryProvider, and I have no better names for ExtendedRegistry. ExtendedRegistry is called that way because it's like bukkit's Registry but has more functionality.

The second thing is do we really need support for numerical keys in the ExtendedRegistry and WritableRegistry? I think that in WritableRegistry it is totally unneeded, but I think that in the ExtendedRegistry needs numerical keys getters by value / keys, just in case someone needs them for serialization purposes.

The third thing is, does WritableRegistry really needs to inherit ExtendedRegistry? The problem with that is that since 1.19.3 mojang doesn't allow to get anything from a Registry if they're not frozen, it means that people will get exceptions if they try to use these methods in the SOMETHING_REGISTRIES_INITIALIZED LifecyclePoint.

@Machine-Maker
Copy link
Copy Markdown
Member

You don’t have to remove a distinction between static and worldgen registries, just have two methods in the bootstrap interface to modify them. Removes a ton of unneeded complexity imo. RegistryAccess can stay, with one instance for static, passed into the fist method, and another for worldgen.

We don’t need ResourceKey, registry layers, def don’t need network ID api, and a ton of other stuff on registry. We can continue to use org.Bukkit.Registry

@InkerBot
Copy link
Copy Markdown

InkerBot commented Apr 7, 2023

btw, net.kyori.adventure.key.KeyImpl and org.bukkit.NamespacedKey with different equals and hashCode

@Machine-Maker
Copy link
Copy Markdown
Member

Machine-Maker commented Apr 7, 2023

@InkerBot you sure about that? We changed the hash code and equals of NamespacedKey so it would work with KeyImpl.

@InkerBot
Copy link
Copy Markdown

@InkerBot you sure about that? We changed the hash code and equals of NamespacedKey so it would work with KeyImpl.

Sorry for outdate version. It have been fixed in recently version

@Owen1212055
Copy link
Copy Markdown
Member

In general, this PR has been superseded by a number of smaller PRs which build the ecosystem of which this exact PR targeted.

Thank you however for making some of this original draft work, as it helped shape Paper Plugins and made it a more feasible system for possible api like this in the future.

I highly encourage that when the biome instances are moved away from enums, you migrate some of the biome api that this introduces into the new registry modification api.

In general, see:
#8920
#9233
https://github.com/PaperMC/Paper/tree/feature/lifecycle-event-system

These PRs mostly split some of the principals that this PR shares.

Again, thank you very much. 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants