Describe the bug
Describe exactly what you observed
When right clicking an Easy NPC zombie with the EFM compat it causes the game to crash.
Expected behavior
A clear and concise description of what you expected to happen.
Expected to open the EasyNPC interface.
To Reproduce
Steps to reproduce the behavior:

1. Go to '...'
   Installed both EasyNPC Epic Fight Compatability Mod and the latest PMMO mod
2. Click on '....'
   spawn an EasyNPC Epic Fight Zombie
3. Scroll down to '....'
   Right click on it.
4. See error
   Error Happens

Screenshots
If applicable, add screenshots to help explain your problem.
crash-2024-03-09_17.57.49-client.txt

Versions:

• Minecraft: (eg 1.20.1)
• Loader: (eg. Forge-47.2.20)
• PMMO: 1.20.1 1.2.20
• Epic Fight Mod 1.20.1 - 20.6.5
• Easy NPC Mod 1.20.1 - 3.8.0
• EasyNPC - Epic Fight 1.20.1 - 1.0.0

Additional context
Add any other context about the problem here.

This crash is on EpicFight. They aren't null checking a call in their own ActionEventSet behavior. I would submit the report to them and have them fix it.

Sorry, but your answer is not correct. You are using saveWithoutId on the client side (at least for the 1.20.1 version) over the onEntityInteract event.

NBT data are normally only handled on the server side because some data are only available there.
Trying to read all NBT data with saveWithoutId on the client side is expected to cause an exception and needs to be fixed from your side and not on the other side.

Seems that this issue causing other crashes as well like: #497

Seems that this issue causing other crashes as well like: #497

@MarkusBordihn Thank you for looking into this. However, your assumption about the invocation of Entity#saveWithoutId is incorrect. Vanilla calls this method as part of entity debugging. To demonstrate that this is not a pmmo bug, I created a custom instance with just MC 1.20.1, Forge 47.2.0, and Easy NPC 3.11.0 and executed the following steps

1. open a new world in creative
2. call /summon easy_npc:humanoid
3. while looking at the entity press Shift+F3+I
4. observe crash.

Crash log for your reference
crash-2024-03-19_17.09.53-client.txt

Your point about MNA having the same issue would be for the same reason. That author is also assuming that this call is not intended to be client-safe when vanilla itself expects it to be.

@Caltinor
Thanks a lot for the feedback and fair enough. I still not understand why the mod needs all of the NPC data on a right click.
I expected this for a debugging tool or for debugging reasons like you mentioned but not from a MMO system for NPCs.

As you could assume the NPCs in this cases stores a lot of data and requesting to serialize and store of all these on right click could cause lags and performance issues. The version 1.18.2 of PMMO seems to have a different behavior.

However I will add additional checks to make sure that pure server-side data are only stored server-side over saveWithoutId.

@MarkusBordihn I am happy to explain. the core features of PMMO are the earning of XP for actions and the prevention of certain actions for players with insufficient XP. Most configurations use the block/item/entity ID and that's sufficient, but users request more granular configurations.

To use a vanilla example which is relevant to EasyNPCs, players often want to prevent players from interacting with villagers of certain profession levels. For example preventing players from interacting with Master Librarians unless they have 100 levels in Charisma. In order to do this, I serialize the NBT then mimic the /data command syntax so they can use EntityData{}.VillagerData{\"profession\":\"minecraft:librarian\"}.level to then set requirements based on the levels they care about. You can then imagine that users want to use details about NPCs to decide if the entity can be interacted with or to change how much XP they earn for interactions with the NPC.

Now, you would be correct to assume that the action prevention and XP awarding are done serverside. The reason we serialize on the client is to reduce server burden for when the client wants to inspect a villager/npc to see what XP it awards or why they are unable to interact. PMMO provides two glossaries: one that works like a dictionary on generic instances of items/blocks/entities/etc and one that works on specific ones (so that we can parse the NBT and not assume the generic data). By serializing and parsing on the client, we unburden the server for what is simply a tooltip/gui behavior.

With regards to 1.18.2, that may depend on your version. I took over pmmo and rewrote it from the ground up during this version, so there is a legacy version from the original author, and the newer versions are functionally identical to the 1.19 and 1.20 versions that are out now.