Balance Loss on Name Change

[PetrichorCraft]

Information

Full output of /ess version:
(I asked for help with this in Discord, and since then, I have updated EssentialsX. I received no help there, so I'm hoping I will get some here. To be safe, I'm posting both sets of /ess version & latest.log. This has happened on both versions.)

(This was the 2nd time it happened, first time I reported it)

[02:28:11 INFO]: Server version: 1.13.2-R0.1-SNAPSHOT git-Paper-492 (MC: 1.13.2)
[02:28:11 INFO]: EssentialsX version: 2.16-pre2.3
[02:28:11 INFO]: PermissionsEx version: 1.23.4
[02:28:11 INFO]: Vault version: 1.7.1-b91
[02:28:11 INFO]: EssentialsXSpawn version: 2.16-pre2.3
[02:28:11 INFO]: You are running unsupported plugins!

(This was the 3rd time it happened, 2nd time I reported it)

[18:20:45 INFO]: Server version: 1.13.2-R0.1-SNAPSHOT git-Paper-492 (MC: 1.13.2)
[18:20:45 INFO]: EssentialsX version: 2.16.0.31
[18:20:45 INFO]: PermissionsEx version: 1.23.4
[18:20:45 INFO]: Vault version: 1.7.1-b91
[18:20:45 INFO]: EssentialsXSpawn version: 2.16.0.31
[18:20:45 INFO]: You are running unsupported plugins!

Server log:
(Again, two logs)

(First time reporting it)
https://gist.github.com/PetrichorCraft/6cf4acfef77c00e427957dadda26fd75

(Second time reporting it - now)
https://gist.github.com/PetrichorCraft/4efc77ae5e262155287cab5ae6bcaaa6
EssentialsX config
https://gist.github.com/PetrichorCraft/f417be76f0d63ec35e199636ac59817e

Help request

Problem

When a player changes their Minecraft username, they lose their balance. Their sethomes are fine, they're kept at the same locations that they were set. This is not a fluke, it has happened 3 times so far, with all 3 people that have changed their usernames.
When you do /bal OldUsername, it shows the balance of the new username. Likewise with /seen. EssentialsX is our economy manager.
What I have tried

I've looked in the EssentialsX config.yml for anything related to name changes, or balance deletion. Nothing found.
I've looked in the EssentialsX userdata files to see if the old balance was recorded in the files. It was not. (And the data files are sorted by UUID - and the sethomes are fine - so it's not like a new file was created for the players with the changed names)
And I've gone onto the Discord server for help. (January 4)

[triagonal]

Hi, would you mind going into your /plugins/Essentials/userdata folder and seeing if a file named 8d714f9f-6475-3358-aa03-627188bbe5e7.yml is present? If so, could you upload that somewhere and reply with it here?

From these lines in your log, it seems like this issue may be related to #574

[14:44:45] [User Authenticator #47/INFO]: UUID of player Officer_Cam is d3630f94-bbc1-4619-bca9-f7fcb7164643
[14:44:46] [pool-25-thread-1/INFO]: [Essentials] Creating empty config: /home/container/plugins/Essentials/userdata/8d714f9f-6475-3358-aa03-627188bbe5e7.yml
[14:44:46] [Server thread/INFO]: Officer_Cam[/<redacted>:53448] logged in with entity id 1746567 at ([world]-8200.845030327007, 56.0, -4034.478643712719)
[14:44:46] [Server thread/INFO]: [Essentials] Found new UUID for Officer_Cam. Replacing 8d714f9f-6475-3358-aa03-627188bbe5e7 with d3630f94-bbc1-4619-bca9-f7fcb7164643
[14:44:46] [Server thread/INFO]: Officer_Cam (formerly known as CamStanley) joined the game

[PetrichorCraft]

Yeah, that does seem to be related. (note: He does still have access to his homes, even though they're not in this file)

https://pastebin.com/nXU4SGSX

[triagonal]

This definitely looks like the same issue. Until it's fixed, you can work around it by finding the npc file and giving the user back their balance, as you've probably already gathered:

[Essentials] Found new UUID for <playername>. Replacing <npc-uuid> with <player-uuid>

Where <npc-uuid>.yml will contain the missing balance.

[PetrichorCraft]

Thank you very much for the help!

[mdcfe]

From a quick glance, it looks like the economy is somehow migrating the player's balance to an offline mode UUID, then the user map is detecting that offline mode UUID and migrating it back to online mode. I'm looking into this further now.

Update 1

Essentials/Essentials/src/com/earth2me/essentials/api/Economy.java

Lines 379 to 393 in 023cf6a

	/**
	* Creates dummy files for a npc, if there is no player yet with that name.
	*
	* @param name Name of the player
	*
	* @return true, if a new npc was created
	*/
	public static boolean createNPC(String name) {
	User user = getUserByName(name);
	if (user == null) {
	createNPCFile(name);
	return true;
	}
	return false;
	}

Essentials/Essentials/src/com/earth2me/essentials/api/Economy.java

Lines 39 to 53 in 023cf6a

	private static void createNPCFile(String name) {
	File folder = new File(ess.getDataFolder(), "userdata");
	name = StringUtil.safeString(name);
	if (!folder.exists()) {
	folder.mkdirs();
	}
	UUID npcUUID = UUID.nameUUIDFromBytes(("NPC:" + name).getBytes(Charsets.UTF_8));
	EssentialsUserConf npcConfig = new EssentialsUserConf(name, npcUUID, new File(folder, npcUUID.toString() + ".yml"));
	npcConfig.load();
	npcConfig.setProperty("npc", true);
	npcConfig.setProperty("lastAccountName", name);
	npcConfig.setProperty("money", ess.getSettings().getStartingBalance());
	npcConfig.forceSave();
	ess.getUserMap().trackUUID(npcUUID, name, false);
	}

It looks like the plugin is somewhere trying to access the player's economy account while the user is connecting, forcing an NPC userdata file to be created through the createNPC method. I'm trying to work out where exactly this gets called during login now.

Update 2

IEssentials#getUser(String) relies on the usermap, but the plugin doesn't update the usermap with their new time until delayedJoin. This means no user is returned and so Economy.createNPC creates a new NPC file for that player. I'm still not sure where this gets called yet.

Update 3

To start with, I was confused - EssentialsX only internally calls Economy.createNPC when running automated tests, so I had no idea where this was coming from... until I checked Vault.

https://github.com/MilkBowl/Vault/blob/81c38e983b4dfb3decdd83385a92f477ba41bb1f/src/net/milkbowl/vault/economy/plugins/Economy_Essentials.java#L87-L93

Vault automatically calls Economy.createNPC if it determines that the player doesn't have an account, which is done by checking Economy.playerExists:

Essentials/Essentials/src/com/earth2me/essentials/api/Economy.java

Lines 351 to 360 in 023cf6a

	/**
	* Test if a player exists to avoid the UserDoesNotExistException
	*
	* @param name Name of the user
	*
	* @return true, if the user exists
	*/
	public static boolean playerExists(String name) {
	return getUserByName(name) != null;
	}

This method relies on the aforementioned Essentials#getUser(String), which relies on the usermap. We should prevent NPC users being created if their UUID already corresponds to a valid Essentials user, even if they can't be found via username yet.

Update 4

After looking around a bit more, I believe getUserByName should be updated to try and search for users by UUID if they can't be found by name. This will avoid player accounts being reported as not existing to Vault.

In an ideal world, the whole Economy API would be migrated to using UUIDs, but this would require a greater effort and for now we should fix this particular issue.

[mdcfe]

@PetrichorCraft Could you update to the latest build and see if the issue persists?

[PetrichorCraft]

Thank you so much! Yes, I just tested, and it's fixed!

[mdcfe]

Glad to hear this fix has worked. Thanks to everyone who helped test and replicate this.