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

[1.16.3] Productive Bees's ConfiguredFeatures may need to be registered #94

Closed
TelepathicGrunt opened this issue Oct 14, 2020 · 10 comments

Comments

@TelepathicGrunt
Copy link
Contributor

TelepathicGrunt commented Oct 14, 2020

Hey there! I was testing productive bees with World Blender and I seemed to have found that productive bee's configuredfeatures are not registered. This can be an issue for mod compatibility as under certain conditions, unregistered ConfiguredFeatures can basically prevent other mod's registered ConfiguredFeatures from spawning if in the same generation stage.

By that I mean, if mod A adds an unregistered CF to the ore generation stage and the biome's codec reaches it first, it will choke and basically nuke mob B's registered CFs afterwards. Here's a case where BetterCaves forgot to register their CF and caused several CFs from Oh The Biomes You'll Go to stop spawning in the world: YUNG-GANG/YUNGs-Better-Caves#75

Here's a more detailed explanation of why this happens in the biome's codec:
image

Specifically, when you call .withConfiguration on a Feature, you create a ConfiguredFeature. This is what should be registered to the WorldgenRegisties at mod init (you can do it in FMLCommonSetupEvent so you have your config ready too if it is needed).

Anyway here's an example from my mod RepurposedStructures of me registering all my ConfiguredFeatures if you want to see one way of doing this.
https://github.com/TelepathicGrunt/RepurposedStructures/blob/a4e3365e3867b8510952ebf658c415de6e412927/src/main/java/com/telepathicgrunt/repurposedstructures/RSConfiguredFeatures.java#L184-L185

I hope this helps! Let me know if you need some help with this and I could try and assist when I have time.

Here's the list of configuredfeatures that seems to be unregistered

productivebees:acacia_wood_nest
productivebees:birch_wood_nest
productivebees:coarse_dirt_nest
productivebees:dark_oak_wood_nest
productivebees:end_nest
productivebees:end_stone_nest
productivebees:glowstone_nest
productivebees:gravel_nest
productivebees:jungle_wood_nest
productivebees:nether_brick_nest
productivebees:nether_fortress_nest
productivebees:nether_quartz_nest
productivebees:nether_quartz_nest_high
productivebees:oak_wood_nest
productivebees:obsidian_nest
productivebees:sand_nest
productivebees:slimy_nest
productivebees:snow_nest
productivebees:soul_sand_nest
productivebees:spruce_wood_nest
productivebees:stone_nest
productivebees:sugar_cane_nest

@JaisDK
Copy link
Collaborator

JaisDK commented Oct 14, 2020

Thank you, I had no idea they need additional registering. I'm almost ready with another release, so I'll put it in there.

@bessiq
Copy link

bessiq commented Oct 17, 2020

Hey there!

I'm not sure if you intended to include this fix in your latest release (productivebees-1.16.3-0.4.1.1.jar) or an upcoming one, but as of this release I am still seeing these features as possibly unregistered:

productivebees:acacia_wood_nest
productivebees:acacia_wood_nest_feature
productivebees:birch_wood_nest
productivebees:birch_wood_nest_feature
productivebees:coarse_dirt_nest
productivebees:dark_oak_wood_nest
productivebees:dark_oak_wood_nest_feature
productivebees:end_nest
productivebees:end_stone_nest
productivebees:glowstone_nest
productivebees:gravel_nest
productivebees:jungle_wood_nest
productivebees:jungle_wood_nest_feature
productivebees:nether_brick_nest
productivebees:nether_fortress_nest
productivebees:nether_quartz_nest
productivebees:nether_quartz_nest_high
productivebees:oak_wood_nest
productivebees:oak_wood_nest_feature
productivebees:obsidian_nest
productivebees:obsidian_pillar_nest
productivebees:sand_nest
productivebees:slimy_nest
productivebees:snow_nest
productivebees:soul_sand_nest
productivebees:spruce_wood_nest
productivebees:spruce_wood_nest_feature
productivebees:stone_nest
productivebees:sugar_cane_nest

Just wanted to let you know. Thanks!

@JaisDK
Copy link
Collaborator

JaisDK commented Oct 17, 2020

I did intend to have it in. If they are not registered now, what on earth do I need to do more?

@TelepathicGrunt
Copy link
Contributor Author

I can clone the repo later tonight or tomorrow to see what's up. The configuredfeature registry should be enough unless it's being registered in the BiomeLoadEvent? If it's being registered in that event, then that might be more tricky as WorldGenRegisteries was already copied by DynamicRegistries at that point and won't ever copy from WoldGenRegistries again. You might want to try registering to DynamicRegistries directly if registering inside BiomeLoadEvent but idk if this will work.

but yeah I'll take a look soon

@JaisDK
Copy link
Collaborator

JaisDK commented Oct 17, 2020

I am registering in RegistryEvent.Register<Feature<?>> and adding the features to biomes in BiomeLoadEvent

@TelepathicGrunt
Copy link
Contributor Author

the 1.16.3 branch looks to be correct to me. The only thing I see is you can remove the ModFeatures registering of the base features in this method as you already call "register" in the static fields (I too kind of accidentally did this but it turns out the forge registry is just a wrapper around the vanilla one so we just need to register to one or the other. Doing both will produce log spam saying the stuff was registered twice. yeah my bad lol)

public static void registerFeatures(RegistryEvent.Register<Feature<?>> event) {

The ModConfiguredFeature side does look correct which is weird as to why it seems to say the stuff isn't registered. Do you have the gradle.properties file you can share? I cant seem to run the dev environment as the build.gradle references a lot of stuff from the properties file which is omitted from github. We can talk more in discord if you don't want to share that file on github.

@JaisDK
Copy link
Collaborator

JaisDK commented Oct 18, 2020

org.gradle.jvmargs=-Xmx3G
org.gradle.daemon=false

version=1.16.3-0.4.1.3
mcversion=1.16.3
forgeversion=1.16.3-34.1.7
mcp_mappings=20200916-1.16.2

jei_version=1.16.3:7.5.0.42
patchouli_version=1.16-43-SNAPSHOT
hwyla_version=1.10.9-B76_1.16.1
top_version=1.16:1.16-3.0.4-beta-7

@JaisDK
Copy link
Collaborator

JaisDK commented Oct 18, 2020

I don't have it on github because I don't want to deal with merge conflicts all the time :)

What about 1.15.2, should I remove registerFeatures there as well?

@TelepathicGrunt
Copy link
Contributor Author

In 1.15.2, it is fine to not register Features but it might make it harder for some other mods that relies on registry names of features. One example I know of is World Blender allows users to use a blacklist of feature's registry names to prevent the features for being imported into the dimension.

But anyway, I'm getting off topic now lol. Thank you for the properties stuff! I'll check the project out right now

@TelepathicGrunt
Copy link
Contributor Author

Alright I did some digging and I think I found a clue. So for Blame, here is what it is seeing. The ConfiguredFeature instance in the biome is not the same object as in any of the registries for the sugar cane nest.
image

So I placed a breakpoint in Productive Bee's registration of the sugar cane nest and the line that adds it to the biome. What I found is the ModConfiguredFeatures.SUGAR_CANE_NEST_FEATURE.get() is returning a different instance of the object when called in different places? Super weird but that seems to be what is happening.
image

So I did a test of calling .get() and storing the configuredfeature to see the instances it returns. I also called vanilla's bamboo configuredfeature as a reference. What I found is that vanilla's always returns the same instance of the configuredfeature which is the needed behavior. The supplier setup for Productive Bees is returning a new instance of the configuredfeature every time which is why the registries acts like it isn't registered.
image

I hope this helps! I'll try and make a PR rn to remove the suppliers just for the configuredfeatures and see if that fixes the issue

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

No branches or pull requests

3 participants