dslake / WorldDownloader

Clones a Minecraft multiplayer world from a remote server to your singleplayer folder
http://www.minecraftforum.net/topic/1444862-
62 stars 19 forks source link

If anyone feels like updating this (I don't) you don't need to be using reflection to access chunk data #65

Open TealNerd opened 8 years ago

TealNerd commented 8 years ago

it can easily be done like this:

ChunkProviderClient provider = (ChunkProviderClient) mc.theWorld.getChunkProvider();
int viewDist = mc.gameSettings.renderDistanceChunks;
viewDist *= 2;
viewDist += 1;
for(int x = 0; x < viewDist; x++) {
    for(int z = 0; z < viewDist; z++) {
        int playerX = (int)mc.thePlayer.posX >> 4;
        int playerZ = (int)mc.thePlayer.posZ >> 4;
        Chunk chunk = provider.provideChunk(playerX + x, playerZ + z);
        if(chunk != null) {
            saveChunk(chunk);
        }
    }
}
Pokechu22 commented 8 years ago

I'm maintaining the updated version here (forum thread here), for 1.8 / 1.8.9.

I think the benefit of using reflection is that you can be completely sure that all chunks are saved nearby. However, there is also the chunkListing field (a List) which seems to exist in 1.8.

/**
 * This may have been intended to be an iterable version of all currently loaded chunks (MultiplayerChunkCache),
 * with identical contents to chunkMapping's values. However it is never actually added to.
 */
private List chunkListing = Lists.newArrayList();

Despite the comment, it does seem to be added to, so it may be useable. However, the current reflection-based method seems to work well enough so I'm not sure if it should be changed yet.

TealNerd commented 8 years ago

I think the reflection is working off the same data and was probably originally written using reflection due to methods being private or not existing but not using the reflection makes the code shorter and nicer looking imo. Also it's usually better to not use reflection if you don't have to, and I can guarantee the method provided above works just as well

Pokechu22 commented 8 years ago

I looked at it - 1.6.4 required the complex method with LongHashMap.entry, but as of 1.7.10 (and 1.8), the list is perfectly functional. I'll probably stick with reflection, though, but instead use the list rather than messing about with LongHashMap.entry.

It'll probably end up something like this:

WDLMessages.chatMessageTranslated(WDLMessageTypes.SAVING,
        "wdl.messages.saving.savingChunks");

// Get the ChunkProviderClient from WorldClient
ChunkProviderClient chunkProvider = (ChunkProviderClient) worldClient
        .getChunkProvider();

// Get the list of all loaded chunks.
List<?> chunks = ReflectionUtils.stealAndGetField(chunkProvider, List.class);

for (int currentChunk = 0; currentChunk < chunks.size(); currentChunk++) {
    Chunk c = (Chunk) chunks.get(currentChunk);

    progressScreen.setMinorTaskProgress(I18n.format(
            "wdl.saveProgress.chunk.saving", c.xPosition,
            c.zPosition), currentChunk);

    saveChunk(c);
}

WDLMessages.chatMessageTranslated(WDLMessageTypes.SAVING,
        "wdl.messages.saving.chunksSaved");

Far simpler, assuming it works.

Pokechu22 commented 8 years ago

The reason for using reflection is that you can be 100% sure to get all loaded chunks, regardless of player position. That method's likely to get all of them, but I still feel like directly accessing the list through reflection is better. (Though directly using the list is also preferable to trying to iterate through the LongHashMap which is pretty messy).

Pokechu22 commented 8 years ago

I get that this is an old issue, but I feel like making a few other points: