Farmer's Delight

Farmer's Delight

97M Downloads

[1.16.4] Tag error breaks JEI

WenXin20 opened this issue ยท 16 comments

commented

Description

  • When connecting to the server, the tag farmersdelight:tray_heat_sources errors according to the log, breaking JEI, which makes the JEI screens not load.
  • Latest.log

Steps to reproduce:

  1. Connect to a server with the mod
  2. JEI doesn't load

Expected behavior:

  • For the tag to function properly with JEI

Mod list:

  • JEI v7.6.0.62
  • Farmer's Delight v0.2.4b
commented

Hmm... you seem to have a few more mods in there, according to your logs.

Did you test this on a minimal instance (FD + JEI only)? If not, please inform the other mods too so I can test it.

commented

It seems Apotheosis tries to call getStateForPlacement() whenever ItemTooltipEvent fires, to determine something related to enchantments. The Cooking Pot calls a tag there to see if it should render a tray, so Apotheosis is accessing an unbound tag.

Not sure why they're asking for getStateForPlacement in these methods, since they only consult the Block, which the event already has... I'll look more into it later.

commented

Alright thanks! Let me know if you still need a mod list.

commented

That specific tooltip handler is responsible for adding information about what enchanting stats a certain block provides to it's item form. These methods are state-based, which requires accessing a state from the block. The default state is not sufficient, as the placed state and the default state may have different stats.

Your usage of tags here is generally considered unsafe/improper (forge considers tag usage for game logic as an improper use at this point).
You could move that check into updatePostPlacement, which should never be invoked without the world existing, and would resolve the issue. I will be wrapping my call to getStateForPlacement in a try/catch, but that method should never throw.

commented

Hmm, I see... I assumed getStateForPlacement was a method used only after world load. I'll test moving it to updatePostPlacement.

commented

@Shadows-of-Fire Actually "forge considers tag usage for game logic as an improper use at this point" is incorrect. While Forge itself should not check for tags due to desync issues with vanilla clients, mods absolutely can use tags for game logic. Vanilla does this all the time. See base_stone_overworld tag used for what blocks vanilla and modded ores can replace. climbable tag to make blocks be able to be climbed. Even beehive_inhabitors is used to determine what mobs can be stuffed inside a beehive (somehow lol). Basically, tags are a great way to have customizable stuff without resorting to ugly lists in configs that you have to handle and parse yourself.

Heck, I would refer you to a wiki of Minecraft's tags. Notice how there are many other tags that affect game logic. https://minecraft.gamepedia.com/Tag#Block_tags

Back to the original issue, vanilla's AbstractCoralPlantBlock actually checks for the water tag in getStateForPlacement which means Apotheosis's code is probably broken for vanilla too. Heck, BambooBlock checks for bamboo_plantable_on tag in its own getStateForPlacement. ConcretePowderBlock too checks for water in getStateForPlacement. This isn't an issue with everyone's mod. This is an issue with how Apotheosis is setup as it is going to continue to blow up for many more mods that follow vanilla's footstep for doing game logic with tags.

Please rework how Apotheosis works instead of placing the blame on other mods and/or vanilla. Game logic tags is a very valid use case for mods and will not be going away and so, please stop discouraging people from using tags properly like here.

commented

Tell lex to stop doing that then, I'm not making that up, I had a PR closed because it used tags for logic.

commented

Like I said, for Forge, which needs to be able to connect to vanilla clients, shouldn't use tags to prevent desync issues. For mods that are required to be on server/client, game logic tags are perfectly fine and safe. Though worldgen tags for serverside only mods are also safe as well as many of my mods uses those.

Stuff like climbable being only on a serverside mods is gonna cause issues I bet as the client handles some of the movement itself which could cause the desync issue. But tags in getStateForPlacement is totally fine.

Actually, I should test the climbable thing myself. Make torches climbable datapack, put it on a vanilla server, and see what happens when a vanilla client connects without the datapack. If it works, then even movement based tags (if done right), may also be safe as well. But it makes sense that Forge themselves would be cautious and try to prevent any sort of potential desync with vanilla clients.

Edit: I was just told that tags are synced to client so that makes this whole thing even more of a moot point lol. The only issue then I could see is if a serverside tag refers to modded stuff that the client doesn't have. But at that point, the mod should be required to be on the client anyway. And how the client even connected to the server with unsynced registries is even more baffling at that point lol

commented

Would you mind me asking, is this problem something generated by Client or Server sided only mods existing in the environment?

commented

I have a tooltip that depends on blockstates to populate information. This crash is caused by A) a mod crashing when getStateForPlacement is invoke before tags are loaded, and B) something invoking tooltips before tags are loaded.

There is something in vanilla that causes B, the client recipe book, so theoretically adding FastWorkbench to disable that should fix this crash, it has fixed this crash for another case that I know of.

commented

I see. I am fortunately not receiving a crash, just that JEI screens fail to display when connected to a server (works fine on singleplayer) - and also have fast workbench installed.

commented

I have a tooltip that depends on blockstates to populate information. This crash is caused by A) a mod crashing when getStateForPlacement is invoke before tags are loaded, and B) something invoking tooltips before tags are loaded.

There is something in vanilla that causes B, the client recipe book, so theoretically adding FastWorkbench to disable that should fix this crash, it has fixed this crash for another case that I know of.

adding FastWorkBench fixed it for me! thanks for the hint :)

commented

Not sure if this is related, but I'm having an issue where all of the cooking pot recipes are not being shown by JEI - but only when connected to a server. They display just fine in a single player world. The configurations and datapacks should be identical across the client and server.

commented

@Boingboingsplat If you have access to them (ask your server admin), could you send me the logs? If recipes are failing to load in JEI, there should be error messages popping up. If your server has Apotheosis, the messages might be similar to the ones in the issue's original post.

commented

I checked the server logs for anything related and came up empty (no mentions of farmer's delight recipes or JEI at all), and after a /reload the issue seems to have resolved. If it happens again I'll see if the log has anything useful.

commented

According to the issue posted in Apotheosis, this seems to be resolved... and my usage of tags doesn't seem to infringe on good practices, at least. I'll be closing it for now; if anyone has further input on this, let me know.

As for @Boingboingsplat 's issue, we found out that it was actually a JEI bug (see #148). It was "dual-fixed" on both FD and JEI, and should be displaying properly now in the 0.3.2 update.