Sponge module refactoring and fixes#2697
Conversation
wizjany
left a comment
There was a problem hiding this comment.
In general, I'm unsure that we actually want all the changes from MC to Sponge classes/methods. Given that most of the Sponge API is just implemented via mixin, the only thing this really accomplishes and pushing the we-sponge code base even further from we-fabric/we-neoforge (making it harder to maintain), and there's several places where sponge classes are just cast back to MC classes anyway.
If this PR just had the block/biome fixes and maybe even the event listener refactor (though see my comments), it would be a lot easier to pull, but right now I feel like there's too much that isn't necessarily a benefit.
worldedit-sponge/src/main/java/com/sk89q/worldedit/sponge/SpongeAdapter.java
Outdated
Show resolved
Hide resolved
worldedit-sponge/src/main/java/com/sk89q/worldedit/sponge/SpongeAdapter.java
Outdated
Show resolved
Hide resolved
worldedit-sponge/src/main/java/com/sk89q/worldedit/sponge/SpongeWorldEdit.java
Outdated
Show resolved
Hide resolved
In my opinion Sponge API is more "stable" than MC. |
|
given we're working with multiple platforms that are Minecraft code, from a maintainability perspective it's often better. Any potential benefits are immediately removed by all (from what i can tell) spots where you've swapped over to Sponge code requiring casting back to MC native code anyway. I agree with wiz in that splitting out the biome & block fixes would be significantly better than this PR as-is. it's doing too much at once, and they're not all changes that make sense for us to merge. |
4fb1fd6 to
a5d6e3a
Compare
Fix items drop when making block changes.
Implement sendBiomeUpdates.
Move listeners into own class.