-
Notifications
You must be signed in to change notification settings - Fork 417
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
base: 1.21.2
Are you sure you want to change the base?
Add a level screen provider registry #4118
Conversation
689b272
to
2934b46
Compare
There was a problem hiding this 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); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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..
@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) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ModifyExpressionValue
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: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: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 modifiesMap.of
rather than replacing the field value as other registry API mixins do; theMap.of
method is more brittle, but seemingly is required asLevelScreenProvider
is an interface. See this Discord message for more information.Fixes #3926