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

Entities saved only when Stop Download clicked #7

Closed johnmlang closed 11 years ago

johnmlang commented 12 years ago

Server: 1.3.2 vanilla Client: 1.3.2 vanilla, MagicLauncher 0.9.9, World Downloader v2 Beta4

Description: Entities from loaded chunks are the only entities saved and only when the Stop Download button is clicked. Entities in other parts of the map explored while World Downloader is saving are not saved.

Steps to reproduce:

  1. Start World Downloader and explore the server map.
  2. Visit entities. Persistent entities such as villagers, golems and pets make this bug easier to verify.
  3. Travel far enough away that the chunks that some of the persistent entities are unloaded.
  4. Click Stop Download.
  5. Load the single player copy and look for the persistent entities.

Expected Result: Persistent entities will be where you left them.

Actual Result: No persistent entities that were in unloaded chunks when the Stop Download button was clicked are present. Only entities in the loaded chunks were saved.

Example: I have two villages several blocks apart. If I visit both villages while World Downloader is saving and clock Stop Download in one of the villages, only the villagers in that village will be saved.

I have also verified that this also occurs with dropped items with dropped items. I expect it affects all entities.

Please let me know if there is any other information I can provide.

dslake commented 12 years ago

I was able to reproduce this with a village. Stopping the download with the village visible saves all of the villagers but moving away until the chunk is unloaded first does not. The village is saved but the entities are only saved when they are visible when the save is stopped. Looking into this now..

dslake commented 12 years ago

This issue is more subtle than thought. I can reproduce the problem with entities not saving, even when the chunk containing the entities is still fully in view. If I go up high toward the block limit and only a few chunks away from a villager, I can still see the houses clearly but the villagers disappear. It does this in a vanilla Minecraft client. This means that the entities are unloaded earlier than the chunk is unloaded at a smaller radius. By the time the chunk is unloaded, there are no entities left in the chunk to save. If I just walk away from the village at ground level while the chunk is still visible and then stop the download, the entities are usually completely saved but it really depends on distance.

I'm finding the routine that removes the downloaded Entities from chunk data so I can store the entities and save them when the chunk is finally unloaded or stop download is clicked.

johnmlang commented 12 years ago

Good catch. It didn't even occur to me that the entities were 'unloading' before the chunk was unloaded. I have noticed that villagers disappear at a smaller radius than iron golems. The entities must only be unloading from the rendering routine or maybe just client-side because my iron farm continues to function when I am far enough away for the villagers to disappear, but close enough for the chunk to be loaded.

Thanks again for continuing the development of this mod.

dslake commented 12 years ago

OK, I found the problem. When connected to a server, entities are created on the client by sending one of the various spawn packets and removed when they are out of tracking range by sending a Packet29DestroyEntity packet. I don't see a way to determine from the client side the reason for the destroy entity. Either it's really not there any more (killed mob, daylight and it disappeared, collected loot, whatever) or you have just gone out of tracking range. The code below is from the vanilla server and the second parameter passed to trackEntity specifies the distance. For most things, it's as small as 64 squares. Once you pass outside of that, the chunk is still loaded but the entities have already been removed. No entities will be saved unless they are within the trackedRange around you when you stop downloading.

Villagers are of type EntityVillager which implements INpc which is derived from IAnimals. So, Villagers are only tracked for 80 blocks in vanilla servers and then they removed by the server from the client.

I'm thinking about ways to fix this. Maybe Nairol has an idea too.

    public void trackEntity(Entity par1Entity)
    {
        if (par1Entity instanceof EntityPlayerMP)
        {
            this.trackEntity(par1Entity, 512, 2);
            EntityPlayerMP var2 = (EntityPlayerMP)par1Entity;
            Iterator var3 = this.trackedEntitySet.iterator();

            while (var3.hasNext())
            {
                EntityTrackerEntry var4 = (EntityTrackerEntry)var3.next();

                if (var4.trackedEntity != var2)
                {
                    var4.updatePlayerEntity(var2);
                }
            }
        }
        else if (par1Entity instanceof EntityFishHook)
        {
            this.trackEntity(par1Entity, 64, 5, true);
        }
        else if (par1Entity instanceof EntityArrow)
        {
            this.trackEntity(par1Entity, 64, 20, false);
        }
        else if (par1Entity instanceof EntitySmallFireball)
        {
            this.trackEntity(par1Entity, 64, 10, false);
        }
        else if (par1Entity instanceof EntityFireball)
        {
            this.trackEntity(par1Entity, 64, 10, false);
        }
        else if (par1Entity instanceof EntitySnowball)
        {
            this.trackEntity(par1Entity, 64, 10, true);
        }
        else if (par1Entity instanceof EntityEnderPearl)
        {
            this.trackEntity(par1Entity, 64, 10, true);
        }
        else if (par1Entity instanceof EntityEnderEye)
        {
            this.trackEntity(par1Entity, 64, 4, true);
        }
        else if (par1Entity instanceof EntityEgg)
        {
            this.trackEntity(par1Entity, 64, 10, true);
        }
        else if (par1Entity instanceof EntityPotion)
        {
            this.trackEntity(par1Entity, 64, 10, true);
        }
        else if (par1Entity instanceof EntityExpBottle)
        {
            this.trackEntity(par1Entity, 64, 10, true);
        }
        else if (par1Entity instanceof EntityItem)
        {
            this.trackEntity(par1Entity, 64, 20, true);
        }
        else if (par1Entity instanceof EntityMinecart)
        {
            this.trackEntity(par1Entity, 80, 3, true);
        }
        else if (par1Entity instanceof EntityBoat)
        {
            this.trackEntity(par1Entity, 80, 3, true);
        }
        else if (par1Entity instanceof EntitySquid)
        {
            this.trackEntity(par1Entity, 64, 3, true);
        }
        else if (par1Entity instanceof IAnimals)
        {
            this.trackEntity(par1Entity, 80, 3, true);
        }
        else if (par1Entity instanceof EntityDragon)
        {
            this.trackEntity(par1Entity, 160, 3, true);
        }
        else if (par1Entity instanceof EntityTNTPrimed)
        {
            this.trackEntity(par1Entity, 160, 10, true);
        }
        else if (par1Entity instanceof EntityFallingSand)
        {
            this.trackEntity(par1Entity, 160, 20, true);
        }
        else if (par1Entity instanceof EntityPainting)
        {
            this.trackEntity(par1Entity, 160, Integer.MAX_VALUE, false);
        }
        else if (par1Entity instanceof EntityXPOrb)
        {
            this.trackEntity(par1Entity, 160, 20, true);
        }
        else if (par1Entity instanceof EntityEnderCrystal)
        {
            this.trackEntity(par1Entity, 256, Integer.MAX_VALUE, false);
        }
    }
nairol commented 12 years ago

I would probably try to catch the entity destruction events with a hook in some method in WorldClient that is called when the DestroyEntity packet comes in. Then I'd check if the distance is the same as in the code above (+- 3 blocks or so) and if it is, add the Entity to a HashMap in WDL:

HashMap<Chunk, Entity[]> entitiesOutOfRange;

(or something like that)

Then when saving a chunk it should look if there are any entities for this chunk in the HashMap and add them to the chunk (and delete them from the HashMap of course).

It will probably mess up redstone machines that rely on the movement of some entities like minecarts, arrows or boats.

EDIT: Oh and there is also a problem if the player moved away from some entities far enough for them to disappear but not far enough that they would be saved and then move towards the entities again. They would be added twice to the HashMap. (Maybe the HashMap isn't a good idea, or maybe we could add them to the chunk right after receiving the destroy packet?)

This is what I'd do, but I haven't looked at the source and have no time to do that at the moment, so I have no idea if this is possible.

dslake commented 12 years ago

That's probably all that's possible here is to try and guess based on distance why each entity was destroyed. In the worst case, it will just mean that some entities are saved that really should have been destroyed or visa versa. I'm out of town this weekend through next Tuesday but I'll work on it some tonight.

On Thursday, September 13, 2012, Florian wrote:

I would probably try to catch the entity destruction events with a hook in some method in WorldClient that is called when the DestroyEntity packet comes in. Then I'd check if the distance is the same as in the code above (+- 3 blocks or so) and if it is, add the Entity to a HashMap in WDL:

HashMap<Chunk, Entity[]> entitiesOutOfRange;

(or something like that)

Then when saving a chunk it should look if there are any entities for this chunk in the HashMap and add them to the chunk (and delete them from the HashMap of course).

It will probably mess up redstone machines that rely on the movement of some entities like minecarts, arrows or boats.

This is what I'd do, but I haven't looked at the source and have no time to do that at the moment, so I have no idea if this is possible.

— Reply to this email directly or view it on GitHubhttps://github.com/dslake/WorldDownloader/issues/7#issuecomment-8528322.

danthedad

johnmlang commented 12 years ago

This is a reasonable compromise. Thanks for finding a solution.

dslake commented 12 years ago

This is a video I made while debugging this. Thought you might find it interesting. http://www.youtube.com/watch?v=G-fNra3Qe5A

johnmlang commented 11 years ago

Indeed, that is some interesting work you've done there. Unfortunately, the server I play has been updated to the latest snapshot and I won't be able to help test any changes. :(

dslake commented 11 years ago

I'll post a snapshot version of the mod if I can get a version of MCP that will deobfuscate it. In the mean time, I've tried not deleting entities when they go out of range but it's been a disaster. When the same entity comes back in range, the server adds it back in but with a different identifier so there doesn't seem to be a way to match them up. The result is many copies of every entity as you walk around the world and entities go in and out of range.

johnmlang commented 11 years ago

I've upgraded to 1.4.2 so I'm able to test this mod again! :) It seems like deleting entities as they go out of range is the best solution for now. Entity duplication sounds more problematic than none at all.

Maybe add this to the list of limitations and make it a feature request.

dslake commented 11 years ago

I'll document it for now in the limitations section. Thanks for helping to test out World Downloader!