Repository navigation
Conversation
Mixins that shadow the field (Lumi) need the Forge descriptor. The Set view speaks vanilla chunkXZ2Int keys; Crucible keeps the unboxed LongHash path through the new unloadQueue field, the same object. Fixes #189
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Fixes #189. Crucible's
ChunkProviderServerpatch changedchunksToUnload(field_73248_b) fromjava.util.Setto CraftBukkit'sLongHashSet. Any mod that expects Forge's type breaks. Lumi, for example, crashes at boot withInvalidMixinException: @Shadow field field_73248_b was not located, because a mixin@Shadowmatches the field by name and descriptor (Ljava/util/Set;).LongHashSetcan't just implementSet: itstoArray()returnslong[], which clashes withSet.toArray(). Its keys are also different. Forge keys the set byChunkCoordIntPair.chunkXZ2Int(x, z), whileLongHashSetusesLongHash.toLong(x, z). So a mod callingcontains(chunkXZ2Int(x, z))would get the wrong answer even if the type matched.Change
io.github.crucible.util.ChunkKeySet, anAbstractSet<Long>backed by the sameLongHashSetas before. TheSet<Long>methods (add,contains,remove,iterator, includingIterator.remove) take and return vanillachunkXZ2Intkeys and convert them toLongHashinternally. Objects that aren't aLongare ignored, like anySet<Long>does.(x, z)overloads andpopFirst()stay unboxed and inLongHashencoding. That's the path Crucible's own code uses, so the unload loop performs the same as before.ChunkProviderServerdeclarespublic Set chunksToUnloadagain. A newpublic final ChunkKeySet unloadQueuefield points at the same object, and Crucible's callers (ChunkProviderServer,WorldServer,CauldronHooks,CraftWorld) use it.Code compiled against the old
LongHashSet chunksToUnloadfield needs to switch tounloadQueue. Nothing in this repo still references the old type.Testing
A minimal coremod with a mixin in Lumi's shape (
@Shadow private Set chunksToUnload), loaded through UniMixins 0.3.1. On the firstunloadQueuedChunksit queues a far chunk throughunloadChunksIfNotNearSpawnand then exercises the field only throughjava.util.Setwith vanilla keys:InvalidMixinException: @Shadow field field_73248_b was not located in the target class, the same as the issue's reportcontains, the iterator yields the vanilla key,remove/addby key,Iterator.remove,size, and a non-Longargument is ignoredLumi itself was not run.