TeamMetallurgy / Atum2

Atum 2: Return to the Sands - Adds an entirely new Egyptian-themed dimension to Minecraft
106 stars 38 forks source link

Re #355: Issue with crates not closing happens with only Atum and curios api installed #357

Closed Kaleidio closed 2 years ago

Kaleidio commented 2 years ago

mc 1.16.5 forge 36.2.8 atum 2.2.8

This "not closing" issue can still result in server lockup. No other mods need to be installed.

Vanilla chests do not exhibit the same behaviour no matter for how long I attempt to get them to whilst in the Atum dimension

Screenshots of proof this bug actually exists: 2021-10-12_07 38 54 2021-10-12_07 41 06

logs: (has nothing useful, debug log did not show up) latest.log

Kaleidio commented 2 years ago

F.Y.I you can get this to trigger yourself by spamming the crate.

GirafiStudios commented 2 years ago

I´ve tried spam-opening a Crate on a server for about 10 minutes now, without any luck at all. I can´t really do much, if I can´t reproduce it myself, when there is no crash log either.

Just to be clear, you´re playing on an actual server, not just "Open to LAN"?

Kaleidio commented 2 years ago

Yes I am VERY sure it's just singleplayer. No LAN, no server.

I have a ryzen 5800x and rtx 3080. this is a genuine bug. I'm curious if it's something specific to AMD CPU processors, for example if you're using a library for that model in some sort of fashion that has render code in it. lately some mods using C++ libraries have been breaking due to ryzen's difference in usage of cache.

another possibility is a race condition where the client's framerate is somehow just so fast that sometimes the packet to close the chest never gets sent to the server, this requires very precise timing (and quite frankly speed)

the last (and frankly most plausible) thing I could think of is incorrect or deprecated methods in the forge API are being used for the storage system.

Kaleidio commented 2 years ago

The only thing I can suggest is that the crate code be extremely similar to, if not inherited from, the vanilla chest class. from what I can tell here your current code has the capability to fail to see if the chest was properly closed. might you be using a deprecated or missing method for the crate's closure process?

Kaleidio commented 2 years ago

If that's not the case, then some other process in your code might be causing this client server desync. whatever it is, it doesn't happen with vanilla chests. vanilla chests seem to have a desync failsafe that sort of polls which chests were opened and lets the client send requests again to sync the chest in the event of lag (maybe tied to nbt). worst case scenario your server packets aren't grabbing certain nbt's of blocks correctly at all

though I highly doubt you bothered with this considering the forge API handles this if you used the right methods.

Kaleidio commented 2 years ago

I'm always open to being sent debug builds on Discord (Kaleidio#1714), some sort of build that spits out information in the log about every method being fired during the crate opening, saving and closing process or something

Kaleidio commented 2 years ago

https://user-images.githubusercontent.com/82226641/138008640-792eb536-a3ab-4d19-a065-d3448ce76b07.mp4

Proof of bug via video, along with screenshot of installation below image

Kaleidio commented 2 years ago

in fact it might purely just be that the animation of the block is getting stuck in the open position and the client just locks up because of that or something.

GirafiStudios commented 2 years ago

I have an AMD Ryzen CPU and Nvidia graphics card as well, so it´s unlikely it´s related to that. The crates are already based on the chest code, as much as possible. There is a chance it could be capability related, will have to take a look at that, when I get time.

Could you try making sure you´re using the latest version of Java 8? You´re currently using the built-in version, which is known to cause issues.

Kaleidio commented 2 years ago

Happens with Java 8 u 304

okay so I was watching my mouse clicks whilst testing, and it seems to happen if you click twice without moving the mouse, sometimes even if the GUI has just opened. apparently this counts as a double-open somehow, probably because the mouse isn't reporting that it's in the menu instead of interacting with the world before the mouse moves whilst the menu is open, but the game lets the block stay in the open state because it thinks it only has to close one of two opens

make sure the code has a failsafe to not fire the open method if it's already open. it's possible the second click is somehow returning a null or otherwise invalid player but the code isn't crashing so it doesn't close because this null player never fired a close event, but I doubt this.

GirafiStudios commented 2 years ago

Looked at it a bit now. For sure can´t be the capabilities, as it´s defaulting to the exact same code as chests uses. Could be related to how the GUI Container is opened, that´s certainly a possibility.

For now, I added some null checks for world in the Tile Entity for the Crate, as there is a chance it could be that. If you could attempt with the build here, that would be great! :) https://jenkins.girafi.dk/job/Team%20Metallurgy%20Mods/job/Atum%202/job/Atum%202%201.16.5%20Jenkins/297/

Kaleidio commented 2 years ago

artifact crashes on startup.

Caused by: java.lang.NullPointerException at com.teammetallurgy.atum.world.gen.AtumDefaultFeatures.addMaterialPockets(AtumDefaultFeatures.java:40) ~[?:1.16.5-2.2.8.jenkins297] at com.teammetallurgy.atum.world.biome.AtumBiomeMaker.makeDeadOasis(AtumBiomeMaker.java:32) ~[?:1.16.5-2.2.8.jenkins297] at com.teammetallurgy.atum.init.AtumBiomes.(AtumBiomes.java:15) ~[?:1.16.5-2.2.8.jenkins297] ... 16 more

looks like you used an unstable dev branch when applying your changes

GirafiStudios commented 2 years ago

Could you try deleting your config?

Kaleidio commented 2 years ago

Did nothing, crashlog is the same

GirafiStudios commented 2 years ago

Could you try the build previous to the one I linked you?

Kaleidio commented 2 years ago

296: fails the same 295: latest working

GirafiStudios commented 2 years ago

Thanks! I´m able to reproduce that crash too actually. The config is not generating properly, will look into that now.

GirafiStudios commented 2 years ago

Sorry for the very late response, never got around to try to fix the config issue. Ended up reverting the whole thing, as I found out it was not possible due to Forge load order.

Could you try this build, so see if it fixes your issue? :) https://jenkins.girafi.dk/job/Team%20Metallurgy%20Mods/job/Atum%202/job/Atum%202%201.16.5%20Jenkins/303/

Kaleidio commented 2 years ago

hiya, gonna start running tests now, sorry for late response

Kaleidio commented 2 years ago

no crash. testing crate itself

GirafiStudios commented 2 years ago

Thanks for checking, glad it fixed it! Part of the release on CurseForge today too.

Kaleidio commented 2 years ago

yeah it looks like the crate does sync as well, so all good.