CodeCrafter47 / AdvancedTabOverlay

GNU General Public License v3.0
5 stars 3 forks source link

AdvancedTabOverlay bugging NPCs and Scoreboards #24

Open KhrysAK47 opened 2 years ago

KhrysAK47 commented 2 years ago

Hey!

I want to report his issue which breaks the NPCs and Scoreboards, along with this error: https://pastebin.com/BJ0pE8Lx

I also reported this to Citizens but after further testing, it seems that its all related to AdvancedTabOverlay, since when I disable it the NPCs and Scoreboards work as intended, meaning its something to do with ATO.

I am unsure if this is relevant but I'm using Java 11 (planning to switch to 16 shortly).

Link to the issue with detailed screenshots and more; https://github.com/CitizensDev/Citizens2/issues/2632

My latest console.log: https://pastebin.com/Ka6C8Cm8

Thank you.

Andre601 commented 2 years ago

Make sure you're using the latest recommended ProtocolLib version for your server version.

KhrysAK47 commented 2 years ago

Make sure you're using the latest recommended ProtocolLib version for your server version.

I actually was using latest, same issue, then I tried downgrading it, same issue.

mcmonkey4eva commented 2 years ago

Hi, I'm not familiar with this plugin (but am with Citizens), however from the limited info I've picked up looking at these two issues, I can venture a guess as to what went wrong:

It looks like this plugin modifies the tab list using packet interception. Citizens uses tab-list packets to spawn NPCs (player entities that aren't played by a real human). Specifically in a pattern of: Add To List packet (ClientboundPlayerInfoPacket with action = ADD_PLAYER), Spawn Named Entity packet, Remove From List packet (action = REMOVE_PLAYER), a few ticks apart. This is the necessary order of logic to spawn a player-entity but prevent it showing up on the tab list (other than flickering there for a second). My guess is that this plugin is intercepting those packets from Citizens and altering them in some incompatible way or outright blocking them. If any of those packets don't get through with their exact original data in their exact original order and timing, the spawning can fail.

The likely solution would probably just be to outright ignore packets from Citizens. Among other options, you might just check the uuid in the GameProfile to see if it corresponds to a real currently online player. If it doesn't, it's obviously an NPC (or some other non-mojang-originated packet) and thus should be ignored (let it through unaltered). Alternately, you could check the version bit of the UUID - v4 = valid real player, v3 = offline mode player, v2 = NPC. Same concept - let packets with v2 ids through unaltered.

If you need any more information about Citizens works here or anything else, feel free to ask me.

Andre601 commented 2 years ago

I myself use the tab list and Citizens and outside of the flickering, which is normal because MC is stupid in terms of showing Players, did I myself not encounter such an issue. I myself use 1.16.5 so I can't tell what is not working here. Maybe another plugin in combination with ATO causes this issue here? Anyway, that's my 2 cents about this.

KhrysAK47 commented 2 years ago

Just bumping this up with https://pastebin.com/DTScQjPK

CodeCrafter47 commented 2 years ago

The error message you are seeing has been fixed in ProtocolLib several weeks ago. Do not rely on their version numbers. Download the latest version of ProtocolLib again. Use the new file. Even if it has the same version number. It will fix the issue.

For reference, this is the commit to ProtocolLib, where they fix the issue: https://github.com/dmulloy2/ProtocolLib/commit/4c0c18d7c61b0471cfbfd7a0017a170dd2264e50.

As for Citizens compatibility, the way we implement it is to not alter any packets with v2 uuids.

KhrysAK47 commented 2 years ago

The error message you are seeing has been fixed in ProtocolLib several weeks ago. Do not rely on their version numbers. Download the latest version of ProtocolLib again. Use the new file. Even if it has the same version number. It will fix the issue.

For reference, this is the commit to ProtocolLib, where they fix the issue: dmulloy2/ProtocolLib@4c0c18d.

As for Citizens compatibility, the way we implement it is to not alter any packets with v2 uuids.

That error persists with latest ProtocolLib.