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

Fix 1.18 issues #98

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Fix 1.18 issues #98

wants to merge 10 commits into from

Conversation

0dinD
Copy link

@0dinD 0dinD commented Jan 17, 2022

Fixes #94
Fixes #95

The menus were broken because AnvilGui wasn't included on the runtime classpath. I also made sure it's shaded now to avoid conflicts with other plugins using AnvilGui.

There was also an issue where the plugin version was written as a number in the YAML, so the plugin tried to auto-update itself from what it thought was version 1.2, when it should be 1.20.

Lastly, the code for setting the MOTD using NMS/reflection doesn't work anymore (so I removed it) since Spigot has dropped most of their mappings (meaning most methods/fields are obfuscated now). See the "NMS" section at https://www.spigotmc.org/threads/spigot-bungeecord-1-17-1-17-1.510208/ for more details. For setting the MOTD in particular, NMS is not needed anyways since the PingListener class already handles it using ServerListPingEvent. The only reason I can see why the NMS was there in the first place is 4ec5815: "Needed for plugins using queries to retrieve the motd". But the way I see it, those plugins (if they still exist) should just use ServerListPingEvent instead, there's really no need to use NMS.


There are also a few errors like these in the console, which I don't think were there in 1.17:

[19:47:44 ERROR]: Adding duplicate key 'ResourceKey[minecraft:worldgen/biome / minecraft:plains]' to registry
[19:47:44 ERROR]: Adding duplicate key 'ResourceKey[minecraft:worldgen/biome / minecraft:forest]' to registry
[19:47:44 ERROR]: Adding duplicate key 'ResourceKey[minecraft:worldgen/biome / minecraft:plains]' to registry
[19:47:44 ERROR]: Adding duplicate key 'ResourceKey[minecraft:worldgen/biome / minecraft:forest]' to registry
[19:47:44 ERROR]: Adding duplicate key 'ResourceKey[minecraft:worldgen/biome / minecraft:plains]' to registry
[19:47:44 ERROR]: Adding duplicate key 'ResourceKey[minecraft:worldgen/biome / minecraft:forest]' to registry
[19:47:44 ERROR]: Adding duplicate key 'ResourceKey[minecraft:worldgen/biome / minecraft:plains]' to registry
[19:47:44 ERROR]: Adding duplicate key 'ResourceKey[minecraft:worldgen/biome / minecraft:forest]' to registry
[19:47:44 ERROR]: Adding duplicate key 'ResourceKey[minecraft:worldgen/biome / minecraft:plains]' to registry

I'm pretty sure the problem was always there (what BiomeMapping is doing is a bit hacky), but the warning is new. Either way it doesn't cause any major issues from what I can tell.

The real problem is that replacing biomes won't work in 1.18 (if the goal is to remove oceans), because as it turns out, they changed a few details about world generation, to say the least. It used to be that biomes were selected first, and then terrain generated based on the biome. Now it's the other way around, the terrain is generated first (according to a bunch of new parameters), and then the biomes are selected based on said parameters. So even if the ocean biome is replaced, oceans (as in large bodies of water) will still generate.

I'm working on a solution to that problem, and I think it's working based on my initial testing. It requires a bit of NMS because there's no API yet for tweaking those worldgen parameters, so I'm thinking of making a library that we can use in this plugin. Or I could contribute it to BiomeMapping, but it doesn't really have anything to do with biomes.

If I have time to spare, I might be able to submit a PR for fixing replace-ocean-biomes in 1.18 later this week. If everything else works well I guess you could release an initial version for 1.18 and drop a patch when I've fixed replace-ocean-biomes.

ProtocolLib 4.4.0 depends on a broken artifact (BukkitExecutors),
which is now part of ProtocolLib itself as of version 4.6.0.
See dmulloy2/ProtocolLib@7ac4ac6

AnvilGui needs to be shaded into the JAR.
MinecraftServer#setMotd was obfuscated in Spigot for 1.18, and
maintaining an NMS solution using VersionUtils seems like more trouble
than it's worth when we can just use PingListener to change the MOTD.
@MadeByIToncek
Copy link

@0dinD respect

@0dinD
Copy link
Author

0dinD commented Jan 18, 2022

I'm pretty sure the problem was always there (what BiomeMapping is doing is a bit hacky), but the warning is new.

Actually I take that back, the problem is simply that the 1.18 wrapper registers the newBiome to the newBiomeResourceKey:

https://github.com/Mezy/BiomeMapping/blob/76519df5ab618c777fb262a1bf52da0702ce71fb/1_18_R1/src/main/java/com/pieterdebot/biomemapping/version/Wrapper_1_18_R1.java#L27

Hence, the warning about registering duplicates on the same resource key (UhcCore uses a mix of forest/plains as replacement).

It used to be that the biomes were registered by ID from oldBiome, but the BiomeRegistry class no longer works with raw IDs. Not sure why the biomes weren't registered by the resource key for the old biome anyways? Either way, it seems to work in 1.17 and below, and for 1.18 replacing the biomes won't actually affect things like ocean generation (apart from decorations like coral reefs and structures), so a different solution is needed.

Replacing ocean biomes no longer works with the new terrain generation
system, so it's better to log a warning.

This also avoids a plugin crash in 1.18.2, for which there is no
BiomeMapping release yet (and it's unclear if there will ever be one).
@0dinD
Copy link
Author

0dinD commented Mar 4, 2022

PR updated to support Minecraft 1.18.2, release can be found here: https://github.com/0dinD/UhcCore/releases/tag/v1.20-pre2

The benefit of VersionAdapter over VersionUtils is that each adapter
implementation is built against its corresponding Paper/Spigot version,
meaning it's easier to catch errors and less reflection has to be used.

However, VersionUtils is tightly coupled with the rest of the codebase
which makes it hard to migrate some methods without substantial
refactoring. That'll be something to look at in the future.
This makes it easier to test the plugin on different server versions.
Previously it was applied to all projects, which is unnecessary and
slows the build down.

BiomeMapping is now shaded to avoid conflicts with other plugins.
@0dinD
Copy link
Author

0dinD commented Mar 10, 2022

I've implemented a replacement for BiomeMapping that works on 1.18+ now, to support the replace-ocean-biomes setting. It works by introducing an abstract VersionAdapter class which can handle the removeOceans() operation. It's similar to VersionUtils, but each implementation is actually built against the corresponding Paper/Spigot server, which is nice because many errors can be caught when compiling and reflection doesn't have to be used (other than to access private fields/methods). Paperweight is used for the 1.18 VersionAdapter implementations, to compile against Mojang mappings and then reobfuscate when the plugin is built.

If you're curious about the actual implementation for how to remove oceans in 1.18+, here goes:

Previously, BiomeMapping could be used to replace the ocean biomes with plains/forest in the biome registry. In 1.18+, this no longer has any effect, because biomes are not responsible for shaping the terrain anymore. There's not much official documentation on how the new terrain generation system works yet, but you can dig around in the Mojang mapped source code from paperweight. I'd also recommend this video by Henrik Kniberg, a Mojang developer: https://www.youtube.com/watch?v=CSa5O6knuwI. It doesn't go into all implementation details, but gives a really good overview of how the new system works.

One of the main takeaways is that ocean generation is now dictated by the terrain shaper, which uses a 2D Perlin noise called "continentalness" to decide whether to generate oceans or inland terrain. A negative continentalness value is interpreted as ocean terrain, and a positive value as inland terrain. Thus, to remove oceans, we just need to make sure that this noise is always positive.

A simple solution is to replace the continentalness noise with a 0 amplitude (constant 0) noise, which is done in the 1.18/1.18.1 implementation. This should work fine, but perhaps the terrain will be flatter since high positive continentalness values are interpreted as "far inland", i.e. mountains.

A better solution can be implemented in 1.18.2, which introduced "noise routers" and "density functions". The continentalness noise is now wrapped in a "continents" density function. With that, we can simply replace the continents density function with one that takes the absolute value of the continentalness noise. This means that the qualities of the original noise are preserved, while making sure that the value is never negative, to stop ocean generation.

A prebuilt JAR for this version of the PR can be found here: https://github.com/0dinD/UhcCore/releases/tag/v1.20-pre3

@0dinD 0dinD mentioned this pull request Mar 10, 2022
The issue was that task the configuration avoidance API has to be used
explicitly in the Groovy DSL, see PaperMC/paperweight#143.
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.

1.18.1 support Building with gradlew.bat doesn't work
2 participants