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

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 this issue Jan 16, 2016 · 5 comments

Comments

@TealNerd
Copy link

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
Copy link

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
Copy link
Author

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
Copy link

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
Copy link

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
Copy link

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

  • I need a count of loaded chunks for the save progress screen; doing that doesn't show how many chunks are loaded. (Of course, that screen didn't exist in the version of the mod found here).
  • provideChunk doesn't ever return null. Instead, it returns a (shared) EmptyChunk... which can be saved, and will overwrite existing chunks. So that algorithm would overwrite nearby chunks with empty ones - not good. It can be worked around by testing for empty chunks, but I'd rather directly deal with the loaded chunk lists.
    I've actually run into this issue when onChunkNoLongerNeeded is called on a chunk that's already unloaded - it overwrites existing chunks with air ones. This has been fixed in development, but it's still an important thing to note (see Pokechu22/WorldDownloader@03733e5). Also, that empty chunk can be corrupted by invalid writes (yet another issue)...

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

No branches or pull requests

2 participants