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

Start of a fix for BW's biome-related rituals #153

Merged
merged 15 commits into from
Sep 4, 2020

Conversation

Sunconure11
Copy link
Contributor

@Sunconure11 Sunconure11 commented Aug 17, 2020

The TC code was fairly identical to what I pretty much needed. Still has a few minor things that need to be worked out, but I'm confident you guys can do that.
Works on servers.

What's needed:

Clean up the gradle. The gradle is a byproduct of how I had to bypass some bugs with it, currently, as well as some frustrations.
Biomes changed by shifting seasons need to refresh. The change is registered, but unless one does something like break a block, or log in and out, the color does not change. This does not happen with the blighted lands ritual for some reason. This most likely can be remedied via a class that alters the biome changing packet BW has. I'm not afraid of any hax.
Alter setMultiBiome
Alter setBiome further
Alter the packet for biome changing

If accepted, please improve on this.

https://github.com/Um-Mitternacht/Bewitchment/blob/1.12.2/src/main/java/com/bewitchment/common/network/PacketChangeBiome.java

https://github.com/Um-Mitternacht/Bewitchment/blob/1.12.2/src/main/java/com/bewitchment/common/world/BiomeChangingUtils.java

@Sunconure11
Copy link
Contributor Author

Sunconure11 commented Aug 17, 2020

Oddities noted:

Biomes aren't saved 100% of the time, or even in the right size of a radius.

build.gradle Outdated Show resolved Hide resolved
@Sunconure11
Copy link
Contributor Author

Sunconure11 commented Aug 18, 2020

Alright, this seems to work as intended, but in order for one to see the changes, one must relog from a world, or trigger a block update. The biomes seem to be sticking, which is good.

@Sunconure11
Copy link
Contributor Author

Shifting seasons seems to be fine with the packet on servers. However, blighted lands craps itself.

@Sunconure11
Copy link
Contributor Author

Fixed blighted lands. Though I suppose it would crap itself regardless, when you're looking for a lot of blocks to convert at once.

@Sunconure11
Copy link
Contributor Author

I'm honestly not too fond of this code, but if anyone thinks they can do better, feel free to do so.

Copy link
Member

@BoogieMonster1O1 BoogieMonster1O1 left a comment

Choose a reason for hiding this comment

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

one small change

@Pseudo
@Mixin(RitualBiomeShift.class)
public class MixinRitualBiomeShift {
@Overwrite(remap = false)
Copy link
Member

Choose a reason for hiding this comment

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

are you sure you want to overwrite here? There's definitely a better way.
you could inject at INVOKE, shift to Shift.AFTER and set the target to the descriptor setBiome, and just add

world.getMinecraftServer().getPlayerList().getPlayers().forEach(p -> p.connection.sendPacket(new SPacketChunkData(world.getChunk(effectivePos), 65535)));

This will inject right after setBiome, which is the only place required to inject.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any examples of injection in the code of this mod itself? Fairly new to toying with mixins.

Copy link
Member

Choose a reason for hiding this comment

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

until now I've only seen overwrites(which is probably not a good sign) in jeid.
I do have a cheat sheet about mixins, which you can refer to.
https://github.com/BoogieMonster1O1/mixincs-updated/blob/master/README.md
You'd want to look at @At.

@Runemoro Runemoro merged commit eeaae8c into DimensionalDevelopment:master Sep 4, 2020
@Sunconure11
Copy link
Contributor Author

As per discussion in the discord, this needs a bit more work added on to fully fix it, as I cannot invest more time into this myself.

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.

3 participants