Theta-Dev / ConstructionWand

Minecraft Mod - Construction Wands make building easier!
https://www.curseforge.com/minecraft/mc-mods/construction-wand
MIT License
13 stars 14 forks source link

Crash on Client Startup 1.16.5-2.1 #34

Closed kreezxil closed 3 years ago

kreezxil commented 3 years ago

https://gist.github.com/ce43cbd433a3a52bbaed60c30efb8f0b

Seems to be related to 1.16.5-36.1.4

Doesn't crash a dedicated server tho on this same forge build.

Just tried it on Forge 1.16.5-36.1.2, and it did not crash the client.

Just tried again on 36.1.4 and this time it did not crash the client.

I'll leave this here, you can kill it after a month if no one adds to or I don't find more info on it.

Theta-Dev commented 3 years ago

Hi @kreezxil, I just tried it myself on both Java8 and 11 and could not reproduce the issue.

Then I did some further investigation. Here are the functions in question based on your error report:

constructionwand.items.ModItems.registerModelProperties (my code)

@OnlyIn(Dist.CLIENT)
public static void registerModelProperties() {
    for(Item item : WANDS) {
        ItemModelsProperties.func_239418_a_(
                item, ConstructionWand.loc("using_core"),
                (stack, world, entity) -> entity == null || !(stack.getItem() instanceof ItemWand) ? 0 :
                        new WandOptions(stack).cores.get().getColor() > -1 ? 1 : 0
        );
    }
}

minecraft.item.ItemModelsProperties.func_239418a (Minecraft code)

public static void func_239418_a_(Item item, ResourceLocation resourceLocation, IItemPropertyGetter getter) {
   field_239415_f_.computeIfAbsent(item, (x) -> {
      return Maps.newHashMap();
   }).put(resourceLocation, getter);
}

java.util.HashMap.computeIfAbsent (Java code)

int mc = this.modCount;
v = mappingFunction.apply(key);
   if (mc != this.modCount) {
       throw new ConcurrentModificationException();

Appearently you must not modify a HashMap in the function passed into ComputeIfAbsent. Otherwise the exception noted in your crash report gets thrown. How that could have happened - no idea. All that function (x) -> { return Maps.newHashMap(); } does is return a new HashMap to be inserted as new item, not modify the main one.

Guess I'll do what you suggested. I'll leave the issue open for a month or so. If you encounter another crash, send me the report.

thx for reporting anyway

kreezxil commented 3 years ago

yeah, most often, reports like these just point at minecraft or the mod loader, this one just pointed out you this time.

Hmm, this seems similar to what happens when inject into registries. Which is probably why I never got deep into modding. Having to learn all these extra nuances of Java.

Keep up the good work and the hard fight. As long as you keep updating and being so kind you can rest assured I'll add your mod continuallyl to all my new packs and current ones where it is already.

Heads, I'm waiting for 1.17 before I make a proper successor to World of Dragons : https://www.curseforge.com/minecraft/modpacks/world-of-dragons

Even tho is has about 20 sisters already.

On Sat, Apr 17, 2021 at 3:24 PM ThetaDev @.***> wrote:

Hi @kreezxil https://github.com/kreezxil, I just tried it myself on both Java8 and 11 and could not reproduce the issue.

Then I did some further investigation. Here are the functions in question based on your error report:

constructionwand.items.ModItems.registerModelProperties (my code)

@OnlyIn(Dist.CLIENT)public static void registerModelProperties() { for(Item item : WANDS) { ItemModelsProperties.func_239418a( item, ConstructionWand.loc("using_core"), (stack, world, entity) -> entity == null || !(stack.getItem() instanceof ItemWand) ? 0 : new WandOptions(stack).cores.get().getColor() > -1 ? 1 : 0 ); } }

minecraft.item.ItemModelsProperties.func_239418a (Minecraft code)

public static void func_239418a(Item item, ResourceLocation resourceLocation, IItemPropertyGetter getter) { field_239415f.computeIfAbsent(item, (x) -> { return Maps.newHashMap(); }).put(resourceLocation, getter); }

java.util.HashMap.computeIfAbsent (Java code)

int mc = this.modCount; v = mappingFunction.apply(key); if (mc != this.modCount) { throw new ConcurrentModificationException();

Appearently you must not modify a HashMap in the function passed into ComputeIfAbsent. Otherwise the exception noted in your crash report gets thrown. How that could have happened - no idea. All that function (x) -> { return Maps.newHashMap(); } does is return a new HashMap to be inserted as new item, not modify the main one.

Guess I'll do what you suggested. I'll leave the issue open for a month or so. If you encounter another crash, send me the report.

thx for reporting anyway

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Theta-Dev/ConstructionWand/issues/34#issuecomment-821882335, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5TJCGO4SFFOFZRMXWRDCDTJHVBTANCNFSM43BN4OKQ .

kreezxil commented 3 years ago

ok it did it again. on startup, I wish it would do it during gameplay.

https://gist.github.com/c6d4ab602fa025a0621689efe6108373

Theta-Dev commented 3 years ago

I still can't reproduce the issue myself. Might have an idea though. It could be that the HashMap holding the model properties gets modified in another thread. So maybe there is another mod registering custom model properties at the same time? That would explain why it crashes only sometimes (these kind of race conditions are the worst to reproduce and debug). Could you tell me which modpack you are using? I might be able to reproduce it that way.

I looked at the Botania mod (they are using custom item models and colors for example for the Wand of the Forest). Appearently they are running the model properties and color registry stuff using a queue.

event.enqueueWork(ClientProxy::registerPropertyGetters);

https://github.com/Vazkii/Botania/blob/3bc7cf88f243a65b0b61f206bcd74525cf9417a4/src/main/java/vazkii/botania/client/core/proxy/ClientProxy.java#L160

So maybe I should do this, too. Can I send you a version of my mod with that change for testing? If that fixes the issue, I'll release it.

kreezxil commented 3 years ago

It's my new pack Sky Stream Madness, version 2.2.3 is the last version with your construction wand in it. And it was crashing on client startup rather consistently, the server doesn't however.

Do note there is an odd issue, where you can re-enter a singleplayer world without restarting the client. I haven't figured out if it is a mod issue or a forge issue.

Theta-Dev commented 3 years ago

I have tested Sky Stream Madness 2.2.3 on my machine, but I could not get it to crash on startup.

Here you have a test version of Construction Wand which might fix the issue. Would you test it on your machine and tell me if it works or crashes? If it is OK, I'll publish it as a new update.

constructionwand-1.16.5-2.2_TEST.zip

Info: Unzip the file before adding it to your mod folder, GitHub does not like jar file attachments. And use this file for testing and private purposes only and dont publish it. Would be a nightmare to support.

Thank you in advance.

kreezxil commented 3 years ago

Which java were you using? That might be part of the problem. I've been using Java11 for the better memory features and faster startup times.

On Tue, Apr 20, 2021 at 1:16 PM ThetaDev @.***> wrote:

I have tested Sky Stream Madness 2.2.3 on my machine, but I could not get it to crash on startup.

Here you have a test version of Construction Wand which might fix the issue. Would you test it on your machine and tell me if it works or crashes? If it is OK, I'll publish it as a new update.

constructionwand-1.16.5-2.2_TEST.zip https://github.com/Theta-Dev/ConstructionWand/files/6345762/constructionwand-1.16.5-2.2_TEST.zip

Info: Unzip the file before adding it to your mod folder, GitHub does not like jar file attachments. And use this file for testing and private purposes only and dont publish it. Would be a nightmare to support.

Thank you in advance.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Theta-Dev/ConstructionWand/issues/34#issuecomment-823498172, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5TJCGXGOCIFDS26CTJPZTTJXAGVANCNFSM43BN4OKQ .

Theta-Dev commented 3 years ago

I have tested it with both Java8 and Java11 on 2 machines and did not have any crashes.

Have you tried the test version that I linked in the post above? It might fix the issue, but I would need your confirmation since I cant test it myself.

kreezxil commented 3 years ago

the test mod worked, i didn't get an issue starting up the pack.

kreezxil commented 3 years ago

yes, i just sent you an update. I thought i did, it works now.

On Wed, Apr 21, 2021 at 7:23 AM ThetaDev @.***> wrote:

I have tested it with both Java8 and Java11 on 2 machines and did not have any crashes.

Have you tried the test version that I linked in the post above? It might fix the issue, but I would need your confirmation since I cant test it myself.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Theta-Dev/ConstructionWand/issues/34#issuecomment-824017928, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5TJCGFRKNTHMG7S7Z3KYLTJ27TZANCNFSM43BN4OKQ .

Theta-Dev commented 3 years ago

Okay. Guess I'll publish it then.

Thank you for testing.

PS: you dont have to send direct replies to me, I'm subscribed to this repository and get all updates into my inbox.

Theta-Dev commented 3 years ago

Released ConstructionWand 2.2 which (probably) fixes the issue.