Matti Ruohonen

I just thought about this very briefly (for a few minutes), ie. how I could implement the chunk saving, before fully reading your comments. And I came up with basically what you have in your latest comment.

So in short:

• There would be a Map of queued chunks to be unloaded and saved

• The Write I/O thread has a separate Map or List or just a single Pair of ChunkPos -> Chunk of the chunk it's currently writing to disk (probably just zero to one entries at a time)

• When a chunk needs to get loaded, it would first be checked in the queued chunks map, as those are always the most recently updated version of the chunk, waiting to be written to disk

• If the chunk isn't in the queue map, then it would be checked in the map or whatever the write I/O thread uses, ie. what is currently being written, as that's the "second most recent place" a chunk can be in

• If it isn't in there either, then it can be loaded from disk via RegionFileCache, as it would have been completely written to disk already

Now, the critical place is to always use a common synchronization/lock object when the queue map is accessed from the server thread to see if a chunk is there, and when the write I/O thread takes a chunk from the queue map, and moves it to the write map, before starting to write it to the disk, and finally when the write i/o thread is done writing the chunk data, and removes the chunk from the write map. That lock shouldn't cause much slow-downs, as the synchronized code section would only be the time it takes to check and grab the chunk from the queue map in the server thread, or the time it takes to grab the chunk from the queue map and put it into the write map in the write thread. The write thread's actual write operation would obviously be outside that synchronized block.

So basically when a Chunk gets unloaded, a reference to it (or its data) would move like so: queue -> writemap -> file

So when unloading a chunk, it would be safe to freely override a chunk reference in the queue map, as there is never any newer version of the chunk anywhere else, and the write thread grabbing chunks from that queue map would be synchronized. Even if the write thread is currently writing a chunk, and it gets modified and put to the queue again, it's no problem, it will just get written again later. And if a chunk needs to re-load before getting written, the latest version of it would be waiting in that queue map. If it's not there yet, then check the write map in case it's currently being written, and if it's not there either, then it can be read from the file again.

Anyway, this is my current thinking of this issue, after a very short amount of time thinking about it.

@Takuto Lehr, It doesn't do a /kill per-se, because it does the removal directly from the chunk NBT data. The end result is essentially the same - only one entity of each UUID will be left. Since UUIDs are supposed to be extremely unique, a case where two different entities would have the same UUID is almost non-existing, so removing all but one entity of each UUID should always result in leaving one copy of each what is supposed to be one entity. But there is no control over which one is left. Thats should really only make a difference with things like villagers, if they duplicate before all the trades are unlocked, or with mules that contain items in their inventories etc. But the same issue also exist while normally in-game - depending on in which chunks the entities are and in which order the chunks load, the entity that will "exist in the world" is also non-deterministic in a way (because only the first one can be spawned in the world, the rest print out that console warning and won't be spawned in the world, but they will still exist in the chunk data, and thus they won't disappear on their own). But that is what you end up with when dealing with a result of a bug that duplicates data...

While you need to have Forge installed to use the mod, you can still keep the world entirely vanilla. All this particular command does, is remove the NBT tags containing the duplicated entities from the chunk's entity list. So to use the mod, you can either temporarily install Forge and the mod to run the command on the server (which is more or less how I did it), and then remove Forge again, or you could have a local Minecraft instance with Forge and the mod, and copy over the world from the server, and then copy it back after. There is also a MCEdit filter to do the same operation (I think it was linked earlier in the comments here?), but I haven't personally used that, so I can't comment on how that one works.

If you need more help with the mod, I'd suggest we take the conversation to the CurseForge comments of the mod (or some other place like Discord), to not ping everyone else in this issue.

@Takuto Lehr, If you want to fix the current continuous log spam (ie. clean up the current state of the world), there is a Forge mod that allows you to do it with relative ease, here: https://minecraft.curseforge.com/projects/world-utils. I have done the clean-up operation twice thus far for my vanilla server's world.

The problem here is, that because of the entity handling issues that are currently in Minecraft, the entities can over time duplicate again and then the log spam can start to happen again. So unfortunately you might have to do the clean-up/duplicate removal operation periodically, until hopefuly some day the entity handling issue itself gets fixed. This issue report is just a symptom of that entity handling bug.

One way to mitigate the problem, is to modify things like animal farms (which often have large numbers on entities) so that the animals can't move across chunk boundaries (F3 + G helps with that). Because afaik, the entity duplicating issue happens when entities move from one chunk to another in the same tick (or very close) to when a chunk unloads or becomes a non-entity-processing chunk (ie. when a player moves away from the area).

This is still an issue even in the latest 1.12 snapshot, 17w06a.

But I think this issue is exclusive to chickens (or any other mobs that have their eye height right at the top of their bounding box).
I think this happens because when they jump up against the ceiling, since their eye height is at the top of their hitbox, their head is then considered to be inside the ceiling block and thus they take suffocation damage. You can achieve the same for example by pushing them against a ceiling with a piston or a shulker box.

http://i.imgur.com/qcJWbmU.png

I don't know if it matters in any way, but this can properly be marked as resolved. I haven't seen this issue at all probably since somewhere around 1.8. At least in 1.10 and 1.11 things seem to work perfectly fine regarding this issue.

I have only recently updated my vanilla server from 1.8.9 to 1.10.2. During this update I ran the world in a modded instance and used a mod I made to remove all the duplicate entities from the world. The first times running the actual vanilla server after this, everything was fine and I got no warnings in the server console.

However I just now looked at the server log, and at the moment there are 4334 lines of this warning message, 4331 of those lines are for Sheep entities. (There are small sheep farms near our server spawn). I started the server and immeadiately got a few warnings of duplicate sheep entities (so they must be in the spawn chunks). This seems to become a real nuisance because of the warning messages constantly polluting the server log...

But the real issue that should be fixed is the fact that entity saving is currently, and has been for a long time, broken. Sometimes when entities cross chunk boundaries (probably near to a time when the chunk unloads?), they can either duplicate or vanish. I saw a youtube video recently where the person said that the entity may get saved to both chunks, or neither, which would make sense. My Mojira-search-fu is failing me, does someone know if there is an open issue for this entity saving bug (vanishing/duplicating entities)? In my opinion it is the most serious and game breaking bug currently in the game, and it has existed from personal experience AT LEAST since 1.7.10, probably even a lot longer (always?).

Yep this is because of a lazy non-fix for the issue MC-101325.

Instead of changing the Village class to use the player's UUID as a key in the playerReputation map, they made a lazy non-fix by just adding null checks for the world reference to the readVillageFromNBT() method to fix the crash that MC-101325 was about. Which means that due to the reasons I explained in a comment in that issue, the game still won't read the player reputation from NBT by the actual player name because that part of the code now doesn't run because of the null checks, and the world still being null at that point. So the player reputation is instead read from a tag called "Name" which doesn't exist. Which then leads to the GameProfile being created for a player called "" (empty string), and thus this exception and console spam.

Please fix this issue by using the player UUID instead of the player name as a key when storing player reputation in a village and get rid of the currently broken GameProfile stuff, which would need a world reference in the NBT read/write methods, which currently is not available at those times when the methods are called. Using the name also means that the reputation for a player (account) in a village will be lost if they change their username.

An easy way to test this (and how I've been testing it), is to create a superflat world and find a village. Then punch any villager even once so that you get a reputation in that village, maybe wait a little bit and then save and quit the world, and then load it again. Upon loading the world, you should see a NullPointerException in the console.

Also, confirmed for 1.9.4 still.

I just tested it, yes it still happens in 1.9.3-pre3.

I'm using MCP names from a Forge modding environment for 1.9 in this explanation.

This bug seems to occur because Village#worldObj is null when the Village#readVillageDataFromNBT() is called.
The readVillageDataFromNBT() method then tries to do this.worldObj.getMinecraftServer().getPlayerProfileCache() which is where the NPE comes from.
The problem is that VillageCollection#readFromNBT() creates a new instance of Village by doing "new Village();" ie. a constructor without the World argument.
But then again that VillageCollection itself is also created without a reference to World in MapStorage#loadData() by getting a constructor with just a String argument.

So to get the GameProfile for a player this way, the Village object would need to have a reference to world at this point.

Edit: So the real question then might be, why is the player reputation in the Village class even stored using the player's name as a key, why not just use the UUID directly? Is this just another example of legacy code not properly updated? It is stored as UUID in NBT already, so changing this run-time storage to also use UUID shouldn't cause any issues that I can see.

Aww crap. I didn't realize this is (intended?) behaviour on Peaceful mode... I was testing something else in a superflat world, and to get rid of the annoying slime invasion I turned the difficulty to Peaceful. It seems I can't close the issue myself, so could a mod close this as invalid please? And sorry about creating an invalid issue :/

This is still in 1.9-pre2. I can't get a minecart to pick up any entities from a level surface. The only way I could make it pick up entities is if it's moving down a slope at the time that it collides with the entity (even going up doesn't seem to work).

Here is a short video demonstrating this (and another minecart/mobs related issue), watch from about midway onwards: https://youtu.be/d0EQvFNb9bQ

From my understanding, this bug occurs due to the following change in the client Minecraft class:

In 1.7.10 the keyboard input is read like so (using MCP names):
public void func_152348_aa()
{
int i = Keyboard.getEventKey();
...
}

Whereas in 1.8 it is read like:
public void dispatchKeypresses()
{
int i = Keyboard.getEventKey() == 0 ? Keyboard.getEventCharacter() : Keyboard.getEventKey();
...
}

This means that if the result from getEventKey() is 0, then the result from getEventCharacter() is used instead.
From a simple debug print (using a FI (= SWE?) keyboard/layout):

When pressing (and releasing) F2:
[06:12:57] [Client thread/INFO] [STDOUT]: eventCharacter: eventKey: 60 state: true
[06:12:57] [Client thread/INFO] [STDOUT]: eventCharacter: eventKey: 60 state: false

When pressing (and releasing) '<':
[06:13:19] [Client thread/INFO] [STDOUT]: eventCharacter: < eventKey: 0 state: true
[06:13:19] [Client thread/INFO] [STDOUT]: eventCharacter: eventKey: 0 state: false

So this means that when a user is pressing '<', the value of getEventKey() is zero, so the result of getEventCharacter() is used instead.
Now if you look at the ASCII table (for example at http://www.asciitable.com), you can see that the ASCII code for '<' is 60, which is the getEventKey() value for F2.
There may also be other collisions, where a "missing" getEventKey() value results in using the getEventCharacter() value of another key.

So the problem here is that the getEventKey() and getEventCharacter() values are not aligned, they should not be used in the same context like this.

Could the reporter or a mod edit the description, because this bug affects ALL sounds (afaik). It is not just the jukebox.
For example block breaking and placing and walking sounds and chest sounds loop when you quickly open and close your inventory after the sound started playing. It gets really confusing at times.

I have just a few days ago finally updated my server from 1.7.10 to 1.8.1. For this update, I made a custom Python program to convert the stats files from 1.7.10 format to 1.8.x format. I know this might be too late for anyone else at this point, but in any case, it is available at: https://github.com/maruohon/mcstatsconverter

Do note that the stats need to be from 1.7[.10], if you have already loaded the world in 1.8, then the stats are gone from that file. Then you would need to get the old stats from a backup and convert them. And if you already had played in 1.8 also, then you would have to manually merge them, or make another program to do it automatically...

Confirmed for 1.8.2-pre1.

Seems to be working pretty good in 1.7.10, don't know about 1.8.x yet, since I haven't updated my servers yet because of other issues in them. I'll report back if the issue returns in 1.8.x once I feel comfortable updating, but for now it can remain as resolved from my part, based on 1.7.10.

This is still an issue in 14w30c.

This happens when chickens or baby cows (at least) jump up from non-full blocks such as carpets, redstone repeaters, daylight sensors and a few sheets thick snow into a one block high gap. They take suffocation damage from the ceiling. It might be because their eye level seems to be just above their collision box❓ (F3+B). It doesn't affect baby pigs, probably since their eye level is noticeably lower. Here is an example of an automatic chicken suicide booth: http://imgur.com/zvTBnje

I was really hoping this would have gotten fixed in 1.7 still, because I will be playing 1.7.x for a long time to come because of mods.
I just tried it in the 1.7.10-pre2, and it is still present :/
The problem based on in-game experiences is, that sounds play an extra time when the player opens his inventory while a sound is still playing, and then closes the inventory again pretty quickly. This often happens when browsing chests and opening and closing the inventory/chests a lot, and it gets really confusing and annoying quickly.
I recorded a short video demonstration in the just released 1.7.10-pre2: http://youtu.be/7f6X4cuPRGA

Yes, the number of people doesn't really matter, it also happens with just one user. Recently it has mostly been just one user playing on the server anyway. The number of region files touched per play session is usually around 5-10, and it has recently been that all of those have been written to up until the point the server is shut down.

And yes, this server is pure vanilla, so the minecraft_server.jar provided by Mojang. This isn't such a huge issue for me that it would need any workarounds, mostly just annoying and tickles my "OCD" of minimizing server resource usage, and obviously not how the server is supposed to work. I didn't quite understand what you were saying about loaders❓

In case it helps, here is my current server startup commandline:
OPTIONS='nogui'
JVM_OPTS="-XX:+UseConcMarkSweepGC"
JVM_MEM_OPTS="-Xms128M -Xmx1024M"
INVOCATION="java $JVM_MEM_OPTS $JVM_OPTS -Dlog4j.configurationFile=log4j2.xml -jar $SERVICE $OPTIONS"

I just hope that this gets looked at and fixed soon, it might also help other servers that are struggling with server load or memory usage issues, but I haven't had any real problems with those with this small a server.