DimensionalDevelopment / JustEnoughIDs

Use the 1.13 chunk format in 1.12 to remove the block, item, and biome ID limits
MIT License
31 stars 33 forks source link

Class Transformer collision in ChunkPrimer with mod Vertically Stacked Dimensions #81

Closed CD4017BE closed 5 years ago

CD4017BE commented 5 years ago

I have an issue with my mod when it is installed together with JEID.

My mod does a small modification to the ChunkPrimer class that is essential for its bedrock (or any other block) generation prevention feature. The problem is that - how I understand your code (don't know how that spongepowered mixin stuff you're using exactly works) - JEID completely replaces that class with it's own implementation.

This is the relevant part of the log:

[00:44:41] [Client thread/TRACE] [FML]: Sending event FMLPreInitializationEvent to mod dimstack
[00:44:41] [Client thread/INFO] [dimstack]: Testing ClassTransformer result of class cd4017be.dimstack.asm.ChunkPrimerTransformer for correct functionality ...
[00:44:41] [Client thread/DEBUG] [VSD ASM]: patching ChunkPrimer as net/minecraft/world/chunk/ChunkPrimer ...
[00:44:41] [Client thread/DEBUG] [VSD ASM]: patching method setBlockState() as func_177855_a(IIILnet/minecraft/block/state/IBlockState;)V
[00:44:41] [Client thread/DEBUG] [VSD ASM]: adding field ingnoredBlock
[00:44:41] [Client thread/DEBUG] [mixin]: Mixing MixinChunkPrimer from mixins.jeid.core.json into net.minecraft.world.chunk.ChunkPrimer
[00:44:41] [Client thread/TRACE] [mixin]: Added class metadata for net/minecraft/util/ObjectIntIdentityMap to metadata cache
[00:44:41] [Client thread/INFO] [dimstack]: dryTest passed!
[00:44:41] [Client thread/DEBUG] [VSD ASM]: patching BlockPredicate ...
[00:44:41] [Client thread/DEBUG] [VSD ASM]: patching method disableBlock() as disableBlock(Lnet/minecraft/world/chunk/ChunkPrimer;Lnet/minecraft/block/state/IBlockState;)V
[00:44:41] [Client thread/FATAL] [dimstack]: mainTest failed! Please report this to the mod author!
java.lang.Exception: Blacklisted block was placed: expected minecraft:stone@0 but got minecraft:obsidian@0.

VSD ASM is my class transformer and dimstack is my main mod running its tests.

sam-kirby commented 5 years ago

I think that the best solution to this would be for JEID to stop overwriting setBlockState and instead use a cancellable injection immediately before vanilla's array access/assignment.

I've implemented this and tested it works as expected. Though until JEID's maintainers have time to merge and put out a version on CurseForge I have implemented a slightly less satisfactory fix in JEIDsI.

I was tripped up by the spelling of ingnoredBlock for longer than I care to admit, but I have reimplemented your ASM transformer as a mixin that can be applied after JEID does its thing to ChunkPrimer. It's less than ideal, but I think it works. If you could take a look to ensure I've not mistaken your intent that would be lovely. I'll release JEIDsI on curseforge tomorrow if you don't spot any issues.

https://github.com/robertdroptables-mcmods/JEIDsIntegration/tree/feature/vsd

[01:50:31] [Client thread/INFO] [dimstack]: Testing ClassTransformer result of class cd4017be.dimstack.asm.ChunkPrimerTransformer for correct functionality ...
[01:50:31] [Client thread/DEBUG] [VSD ASM]: patching ChunkPrimer as ayw ...
[01:50:31] [Client thread/DEBUG] [VSD ASM]: patching method setBlockState() as a(IIILawt;)V
[01:50:31] [Client thread/DEBUG] [VSD ASM]: adding field ingnoredBlock
[01:50:31] [Client thread/DEBUG] [mixin]: Mixing mixins.MixinChunkPrimer from mixins.jeidsi.dimstack.json into net.minecraft.world.chunk.ChunkPrimer
[01:50:31] [Client thread/DEBUG] [mixin]: Mixing MixinChunkPrimer from mixins.jeid.core.json into net.minecraft.world.chunk.ChunkPrimer
[01:50:31] [Client thread/TRACE] [mixin]: Added class metadata for net/minecraft/util/ObjectIntIdentityMap to metadata cache
[01:50:31] [Client thread/INFO] [dimstack]: dryTest passed!
[01:50:31] [Client thread/DEBUG] [VSD ASM]: patching BlockPredicate ...
[01:50:31] [Client thread/DEBUG] [VSD ASM]: patching method disableBlock() as disableBlock(Lnet/minecraft/world/chunk/ChunkPrimer;Lnet/minecraft/block/state/IBlockState;)V
[01:50:31] [Client thread/INFO] [dimstack]: mainTest passed!
[01:50:31] [Client thread/INFO] [dimstack]: Testing ClassTransformer result of class cd4017be.dimstack.asm.BlockPortalTransformer for correct functionality ...
[01:50:31] [Client thread/INFO] [dimstack]: portalTest passed!
CD4017BE commented 5 years ago

The replacement code seems correct. And if it wasn't then it would be pretty much guaranteed that it wouldn't pass the test.

I might also look into mixin mow, it seems convenient. Thanks for your fix. And long live the incorrectly spelled variable name.