TeamMetallurgy / Aquaculture

Aquaculture is an expansion of Minecraft's base fishing system. It allows you to catch a variety of new fish with a brand new series of rods and tools
69 stars 72 forks source link

Incompatibility with Roughly Enough Resources - "Invalid player data" #388

Closed unilock closed 1 year ago

unilock commented 1 year ago

With Aquaculture 2 installed alongside Roughly Enough Resources, the player is unable to join any world, with the error message "Invalid player data".

To reproduce:

  1. Install Aquaculture 2.
  2. Install Roughly Enough Items + dependencies.
  3. Install Roughly Enough Resources.
  4. Start game; create and join a new world.
  5. Error.

Screenshot:

image

(it says "Back to Server List" regardless of whether you're joining a singleplayer or multiplayer world)

Versions:

Relevant bit of latest.log, snipped from when creating a new world: https://gist.github.com/unilock/fd9b4942846ddb16fb9678b50e9a9f46#file-latest-log

Relevant issue: #364

GirafiStudios commented 1 year ago

Thanks for the very thorough report! I was able to reproduce the issue. But it looks like it's unfortunately caused by Roughly Enough Resources. Don't think there is much I can do about it on my end.

unilock commented 1 year ago

If it helps, the dev of Enigmatic Legacy fixed the same issue on their end in their 1.19.2 branch: https://github.com/Aizistral-Studios/Enigmatic-Legacy/commit/857aa211318873180761aaa01a132a43cb0ac36e

I (hackily) cherry-picked that commit into its 1.18.X branch, and it worked fine. Maybe something similar can be done in Aquaculture?

GirafiStudios commented 1 year ago

There is not much in that commits, that really relates to stuff AQ2 does tbf. Only thing that it maybe could be, is the added event.enqueueWork. I've tried doing that in AQ2 as well, so see if it helps

A build will be available here shortly to test :) https://jenkins.girafi.dk/job/Team%20Metallurgy%20Mods/job/Aquaculture/job/Aquaculture%202%201.18/28/

unilock commented 1 year ago

Unfortunately, that build did not solve the issue :(

But, I think I did! (probably not in the "best" way, but it does work!)


Here: https://github.com/TeamMetallurgy/Aquaculture/blob/0a45b04ecc7f4ecedb66c29c4cec59f9bb4c6d37/src/main/java/com/teammetallurgy/aquaculture/loot/BiomeTagPredicate.java#L133

I think Gson is tripping up, because the "add" element doesn't exist in Aquaculture's fishing loot tables - so it's trying to convert nothing into a boolean.
Gson's getAsBoolean() requires that the element being passed be a JsonPrimitive or a JsonArray - neither of which a non-existent element is, obviously.


I replaced the above line of code with this:

object.addProperty("add", net.minecraft.util.GsonHelper.getAsBoolean(object, "add", false));

GsonHelper#getAsBoolean is a built-in method that checks if an object contains a given element, and if it does not, returns a default value. In this case, if the "add" element does exist under the biome_tag_check condition, the method returns the value of said element as it is; if the element does not exist, the method returns false by default.


As far as I've tested, it works! Aquaculture fish are still only caught in their defined biome types (so the "biome_tag_check" condition is being considered correctly), and vanilla Minecraft fish can still be caught as well.

I'm not sure of the implications of this when an "add" element does exist in a loot table object, as I have nothing to test with. But, since this is what Minecraft uses for its own loot tables, I assume it would be fine.


(note that the above "fix" is functionally equivalent to

if (object.has("add")) {
    object.addProperty("add", object.getAsBoolean());
  } else {
    object.addProperty("add", false);
  }
}

...probably)

GirafiStudios commented 1 year ago

Excellent detective work! I've tried implementing the fix. Here is a Jenkins build with the fix: https://jenkins.girafi.dk/job/Team%20Metallurgy%20Mods/job/Aquaculture/job/Aquaculture%202%201.18/

unilock commented 1 year ago

Everything's working on my end! Thanks! :)

ErzengelLichtes commented 1 year ago

I'm getting the same issue on 1.19.2