Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a level screen provider registry #4118

Open
wants to merge 1 commit into
base: 1.21.2
Choose a base branch
from

Conversation

haykam821
Copy link
Contributor

@haykam821 haykam821 commented Sep 24, 2024

This pull request adds a registry for level screen providers in the fabric-screen-api-v1 module, which control the screens available from clicking 'Customize' on the create world screen. It can be used in the following way:

Identifier id = Identifier.of("example", "fabricland");
RegistryKey<WorldPreset> key = RegistryKey.of(RegistryKeys.WORLD_PRESET, key);

LevelScreenProviderRegistry.register(FABRICLAND_WORLD_PRESET, (parent, generatorOptionsHolder) -> {
	ChunkGenerator chunkGenerator = generatorOptionsHolder.selectedDimensions().getChunkGenerator();
	// Return a screen here that calls `parent.getWorldCreator().applyModifier(...)` to apply changes
});

To test this registry, a Fabricland chunk generator and world preset has been added to the fabric-screen-api-v1 test mod. The create world screen allows opening a customization screen which allows the background and outline blocks that are generated to be changed:

'World Type: Fabricland' button next to 'Customize' button
Fabricland made of granite and red glazed terracotta

The corresponding names in Yarn mappings may be improved, so the LevelScreenProviderRegistry class name in this API should not be considered final.

Note that the LevelScreenProviderMixin class modifies Map.of rather than replacing the field value as other registry API mixins do; the Map.of method is more brittle, but seemingly is required as LevelScreenProvider is an interface. See this Discord message for more information.

Fixes #3926

@haykam821 haykam821 force-pushed the level-screen-provider-registry branch from 689b272 to 2934b46 Compare September 24, 2024 23:39
Copy link
Member

@modmuss50 modmuss50 left a comment

Choose a reason for hiding this comment

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

Looks good, just a few minor changes.

LevelScreenProvider old = LevelScreenProviderAccessor.fabric_getWorldPresetToScreenProvider().put(key, provider);

if (old != null) {
LOGGER.debug("Replaced old level screen provider mapping from {} to {} with {}", worldPreset, old, provider);
Copy link
Member

Choose a reason for hiding this comment

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

Generally we crash if there is a duplicate, is there a specific usecase for overriding them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The FlattenableBlockRegistry, LandPathNodeTypesRegistry, SculkSensorFrequencyRegistry, StrippableBlockRegistry, VillagerInteractionRegistries, FabricDefaultAttributeRegistry, MinecartComparatorLogicRegistry, and VillagerTypeHelper classes provide registration methods which log replaced values the same way.

/**
* Adds registration hooks for {@link LevelScreenProvider}s.
*/
public final class LevelScreenProviderRegistry {
Copy link
Member

Choose a reason for hiding this comment

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

We generally use interfaces for most public API like this, I would move the logic (with the logger) to an impl class.

@@ -48,6 +60,12 @@ public void onInitializeClient() {
});

ScreenEvents.AFTER_INIT.register(this::afterInitScreen);

Registry.register(Registries.CHUNK_GENERATOR, FABRICLAND_ID, FabriclandChunkGenerator.CODEC);
Copy link
Member

Choose a reason for hiding this comment

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

I assume its fine to register this only on the client? In a real mod this wouldnt be like this, maybe just put a comment on it for people using it as a reference..

@modmuss50 modmuss50 added the enhancement New feature or request label Sep 29, 2024
@Mixin(LevelScreenProvider.class)
public interface LevelScreenProviderMixin {
@WrapOperation(method = "<clinit>", at = @At(value = "INVOKE", target = "Ljava/util/Map;of(Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)Ljava/util/Map;"))
private static Map<Optional<RegistryKey<WorldPreset>>, LevelScreenProvider> makeMutable(Object k1, Object v1, Object k2, Object v2, Operation<Map<Optional<RegistryKey<WorldPreset>>, LevelScreenProvider>> operation) {

Choose a reason for hiding this comment

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

ModifyExpressionValue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Registration helpers for level screen providers should be added
3 participants