Loading Village.dat containing player reputation causes NullPointerException

I did not, that's right. I only tested on version 1.9 and 1.9.2 (so you can add "1.9" to the list). I might try out a couple of snapshots in an iterative manner to circle in the time when it first occurred. At least I'm assuming that's of any help for the devs, right?

It's helps the devs to find the change that causes this, yes.

Also, we can only mark current versions as affected, past versions are not mark-able.

I believe I was a bit over-enthusiastic about the game removing the village entry from the file if there is a reputation value present. Could also be just because I tested on a server with no player on and therefore the village even though located in the spawn chunks got unloaded. What I find strange though is that it removed the reputation value first and only after some time removes the village entry entirely from the villages.dat file. However this is true for a version not showing the NullPointerException as well, so I guess that's a wrong reporting from my side, I am sorry.

What I definitely can contribute though is the exact version this appeared the first time: 15w36. My guess is that MC-87871, which has been reported fixed in this version and in fact has to do with villager reputation, introduced this bug as regression.

For more information about this bug: https://www.youtube.com/watch?v=pOV-08RHLlM (made by Tango Tek)

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.

I run a server on 1.9.2 Bukkit - Lastest release as of 27th april 2016, without any plugins or further modifications isntalled, and I do not encounter this bug.

I know this is only for Vanilla stock jars. Just wanted to add this to the conversation, hoping it might be helpful in some way 🙂

Still happens in 1.9.3-pre2.

Will this be fixed for 1.9.3-pre3 ?

@unknown, 1.9.3-pre3 is already there. Try to reproduce the bug on this version, and you'll see =)

@SunCat I currently cannot confirm due to the fact That I do not have this built. I just know a youtuber (TangoTek) made this and I was going to build it and I want it fixed before issues happen. It would be nice if someone confirmed it/checked for me 🙂

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

Thank you Matti Ruohonen

@ Remi Barna
If you mean with that you don't have the Iran Titan built, then this is not necessary. TangoTek just found out about it while building the Iron Titan. The bug needs just a loaded village with existing player reputation and a server restart to show a NullPointerException in the console on startup.

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.

This "problem" is designed to break the Titan. As is the required change in timing needed to keep spawn chunks active. There is nothing to fix.
Not related to the Titan issue directly, this new practise of changing behaviours of redstone devices to get rid of large scale uses of these devices. I am supposing that this practise is what is being referred to as "better server memory management" and the like.

They don't break farms on purpose. I watched a more detailed explanation of how it works and it doesn't use bugs. Why do you think quasiconnectivity is still in the game? Because people will complain when it gets fixed. They don't introduce new bugs on purpose.

I understand were you are coming from James, but this is simply not the case. This is a bug that deletes villages if you punch a villager. This should not be in the game.

i wrote a scrip for windows commandpromt (cmd) that automaticly replace the wrong data file here is the code:

del villages.dat
copy villages_fixed.dat villages.dat
PAUSE

you need to have the fixed file as the tutorials :https://goo.gl/pJB6wZ says in the data folder you put the script in the folder if you run the script the wrong file
will be replaced with the fixed file you can put a line of code thtat runs this script in your server starter so i wil do this every reboor