search

AvarionMC / graves

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

Fixed compass overwriting main hand when auto-looting full grave #3

Closed jarbarsi closed 2 months ago

jarbarsi commented 2 months ago

The line removed here caused the grave compass to overwrite the item in the main hand if the player died with a full inventory and an item in their offhand. I could not find any situation where this line did anything productive other than causing this bug, so I decided to just remove it entirely.

svaningelgem commented 2 months ago

Verification: Before: image

After: image

svaningelgem commented 2 months ago

However, the items were still in the grave: image (see the hoe & the obituary got dropped)

So it's not that the items are lost. They are simply not equipped because of the compass.

jarbarsi commented 2 months ago

That is not the behavior I'm seeing on my server, hold on one moment and I'll send a few screenshots. (also worth noting that this is being observed on a paper server, and I believe the bug is caused by some weird race condition so it may not occur within spigot)

svaningelgem commented 2 months ago

I'm running:

[08:07:04 INFO]: Starting minecraft server version 1.20.1
[08:07:04 INFO]: This server is running Paper version git-Paper-196 (MC: 1.20.1) (Implementing API version 1.20.1-R0.1-SNAPSHOT) (Git: 773dd72)
jarbarsi commented 2 months ago

I am running 

[00:33:27 INFO]: Starting minecraft server version 1.20.4
[00:33:27 INFO]: This server is running Paper version git-Paper-492 (MC: 1.20.4) (Implementing API version 1.20.4-R0.1-SNAPSHOT) (Git: fc53ff5)

Before death note the sword in the main hand, pickaxe in offhand image

After auto-loot (importantly, with compass in primary hand) 2024-04-23_00 35 21 image

The sword that was in my main hand is not anywhere in my inventory after auto-looting with the compass in the main hand

svaningelgem commented 2 months ago

Ok, thanks for the info! I'll look into this one today.

jarbarsi commented 2 months ago

accidentally made a new PR (I am still not very well practiced in using github) however #7 should fix the issue properly without just blindly removing functionality lol

svaningelgem commented 2 months ago

Continuing this PR in #7.

svaningelgem commented 2 months ago

accidentally made a new PR (I am still not very well practiced in using github) however #7 should fix the issue properly without just blindly removing functionality lol

FYI: you can just push new commits to the branch to update a PR. No need to open a new one 😁. But let's continue in 7 for now.