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

Rename the FuelRegistry.Builder#featureSet field to enabledFeatures #3958

Merged

Conversation

haykam821
Copy link
Contributor

@haykam821 haykam821 commented Aug 23, 2024

This field contains currently enabled features as opposed to being a more generic feature set, so the more specific terminology already used within Yarn should be used. The arguments indirectly passed into the builder's constructor already use the terminology:

// In the ClientPlayNetworkHandler constructor
this.fuelRegistry = FuelRegistry.createDefault(connectionState.receivedRegistries(), this.enabledFeatures);

// In the ClientPlayNetworkHandler#onSynchronizeTags method
this.fuelRegistry = FuelRegistry.createDefault(this.combinedDynamicRegistries, this.enabledFeatures);

// In the MinecraftServer constructor and MinecraftServer#reloadResources method
this.fuelRegistry = FuelRegistry.createDefault(this.combinedDynamicRegistries.getCombinedRegistryManager(), this.saveProperties.getEnabledFeatures());

The names of intermediate parameters have been changed as well.

@haykam821 haykam821 requested a review from a team August 23, 2024 01:54
@haykam821 haykam821 added bug Fixes or discusses a bug within the mappings snapshot A PR that targets a snapshot version of Minecraft labels Aug 23, 2024
@apple502j apple502j merged commit f5b5c74 into FabricMC:24w34a Aug 23, 2024
5 checks passed
@haykam821 haykam821 deleted the fuel-registry-builder-enabled-features branch August 23, 2024 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes or discusses a bug within the mappings snapshot A PR that targets a snapshot version of Minecraft
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants