wisp-forest / numismatic-overhaul

Terraria-style currency in Minecraft
https://modrinth.com/mod/numismatic-overhaul
MIT License
27 stars 20 forks source link

Freeze when using Yung's Better Ocean Monuments #93

Open Mirsario opened 1 year ago

Mirsario commented 1 year ago

This is kind of a reopen of #80 (Cartographer level-up freezes servers), but after both of the suggestions from there failed to help me I found one actual cause that isn't "it takes a long time" or vanilla's fault.

The cause of the freeze for me turned out to be the fact that cartographers were trying to locate the vanilla minecraft:monument structures, which Yung's Better Ocean Monuments removes from generation and replaces with the betteroceanmonuments:ocean_monument structure. If I were to make a guess, it's probably due to Numismatic Overhaul changing the trade type to its own special numismatic-overhaul:sell_map, so the generation mod doesn't recognize it to swap the vanilla structure type to its new one? Though I couldn't find the code for that, so I don't know for sure which side is at fault.

Pinging @yungnickyoung just in case.

Temporary Workaround

Applying the following change in NurismaticOverhaul.jar/data/nurismatic-overhaul/villager_trades/cartographer.json helped me avoid the issue for myself:

      {
        "type": "numismatic-overhaul:sell_map",
        "price": 3000,
-        "structure": "monument",
+        "structure": "betteroceanmonuments:ocean_monument",
        "max_uses": 2,
        "villager_experience": 15
      }

Crash Report

If you want to look at my colossal collection of mixins, which I don't think matters, here it is: crash-2023-07-07_00.09.06-server.txt

Ricenami commented 1 year ago

How do I do this fix myself? Do I make a datapack?

Noaaan commented 1 year ago

Yes. Copying and editing the cartographer trades from that location will fix this issue. It is unfortunately not easy to handle, as Better Ocean Monuments creates its own structure rather than replacing the vanilla one. The watchdog crash in question is actually a vanilla crash, which happens when a cartographer map tries to look for a structure for too long. I recommend upping the max-tick-time setting in your server.properties as an attempt to mitigate the issue, or fix it via data.

Ricenami commented 1 year ago

I appreciate your quick response, as for the server max-tick-time, what amount do you recommend?

Mirsario commented 1 year ago

I recommend upping the max-tick-time setting in your server.properties as an attempt to mitigate the issue, or fix it via data.

The former can't help without the latter in this conflict's case, as the game's structure locator is for some reason completely fine with spending an actual eternity looking for a non-existing vanilla structure, and seemingly doesn't have its own failure state, with only the tick watchdog there to stop that madness. If someone sets max-tick-time to a year - it'll look for that monument for a year.