tomdodd4598 / NuclearCraft

NuclearCraft is a Forge mod for Minecraft 1.12.2.
https://www.curseforge.com/minecraft/mc-mods/nuclearcraft-overhauled
MIT License
196 stars 127 forks source link

Server Crash w/ Minecolonies Building Upgrade #517

Closed LightEco closed 5 years ago

LightEco commented 5 years ago

I found a conflict when attempting to upgrade certain buildings in Minecolonies. The server crashes with a NuclearCraft radiation issue and they kicked it over here. Any ideas?

Crash Log - https://pastebin.com/Us5jD3R4

Running NC version "NuclearCraft-2.15-1.12.2"

Also this is part of the "All the Mods Remix" pack

Asherslab commented 5 years ago

Please note, this was redirected from here: issue as this is a coremod that is causing the issue, not us directly

tomdodd4598 commented 5 years ago

@Asherslab This isn't a coremod, but nevertheless, I don't see why this is necessarily the fault of NC - why is the null pointer being thrown when trying to get the inventory of the colony building?

EDIT: Looks like it might be the null EnumFacing argument that's causing the problem - a simple exception catch should fix this :)

CaptainSisko commented 5 years ago

@turbodiesel4598 Just to clarify, are you saying that it might be the null EnumFacing argument in NuclearCraft or Minecolonies that appears to be the problem?

If it needs to be fixed in Minecolonies then the issue on the Minecolonies issue tracker will need to be updated with this information. Im happy to do so if it is indeed Minecolonies that needs the fix.

tomdodd4598 commented 5 years ago

What Minecolonies could do is just add a little something to handle null EnumFacing arguments, but the more general solution (to avoid problems like this with other mods that don't expect a null there) is to handle any exceptions on my side ;)

kbblakfox commented 5 years ago

@Asherslab This isn't a coremod, but nevertheless, I don't see why this is necessarily the fault of NC - why is the null pointer being thrown when trying to get the inventory of the colony building?

EDIT: Looks like it might be the null EnumFacing argument that's causing the problem - a simple exception catch should fix this :)

you made it sound so easy :) but is there a way to do that for us mere mortals? it is happening on a lot of our building now :( even thou all rad reading are a 0/t

Asherslab commented 5 years ago

ah, think he may have forgotten to fix this =P Unless there's a new version, did you check?

kbblakfox commented 5 years ago

kinda I'am running 2.15 so the newest that ain't E-mode ( might have checked that wrong )

tomdodd4598 commented 5 years ago

Fixed for the next build, which will be out shortly ;)

Asherslab commented 5 years ago

Thanks mate!

tomdodd4598 commented 5 years ago

No prob!

tomdodd4598 commented 5 years ago

@Asherslab Looks like this is still happening, even with a non-null side argument when trying to get the capability. I really don't think this should crash the game, as getting capabilities this way in general is a standard part of getting information from tile entities. There seems to be some unique structure to Minecolonies capability getting which is causing the problem, so I don't actually understand why the null-pointer is occurring.

Asherslab commented 5 years ago

Thanks for looking into it. We've never seen this NPE before without NuclearCraft. However, if it's still happening we'll take a look on our side. thanks

marchermans commented 5 years ago

This might be something on our side @Raycoms illegally returned null on some requests of the IItemHandler capability in older versions, this was fixed in newer versions however. That said we have a fast paste release cycle for ALPHA builds which contain this fix, we are currently testing if this is the case. I would ask the OP @LightEco to provide us in this issue with the version number of Minecolonies that he is/was running, to determine if this is actually the cause and help us narrow down the failure cause.

CaptainSisko commented 5 years ago

@OrionDevelopment The version of Minecolonies that is in the ATM3:Remix v1.3.0 is minecolonies-1.12.2-0.10.256-BETA-universal.

However a recent update came out for ATM3 Remix that has updated Minecolonies to minecolonies-1.12.2-0.10.291-ALPHA-universal. Unfortunately i haven't had a chance just yet to test this on ATM3:Remix v1.3.1. I will try to get this tested today and get back to you.

CaptainSisko commented 5 years ago

I just tested with the following setup.

Fresh ATM3 Remix 1.3.1 pack which has: NuclearCraft-2.17a-1.12.2 minecolonies-1.12.2-0.10.291-ALPHA-universal

Still crashing with the following crash-log https://hastebin.com/wonisiyese.rb

Steps i followed to reproduce the crash

1) Create a new world with cheats enabled 2) Spawn in, change to creative and teleport away from the spawn area (2500, 100, 2500 used in this case) 3) Spawn in a Town Hall and Building Tool from MineColonies 4) Right click with the Building Tool to bring up the building selection 5) Change the building level to 5 6) Change the building style to Medieval Oak 7) Click the Paste button to spawn the building in

The client will then freeze and the crash log i linked above is generated.

If you follow these steps but instead of a Medieval Oak building you use the Acacia level 5 town hall the crash does not occur. I haven't gone through all of the building types because the ATM3 client takes way too long to start up. I have tested a few and most of the Medieval styles will crash.

This issue originally occurred on my server when an NPC builder was upgrading a Medieval Oak Town Hall from level 3 to 4 ( or possibly 4 to 5, not exactly sure).

marchermans commented 5 years ago

Okey looks like something is broken then. Will need to check what is up.

marchermans commented 5 years ago

@Raycoms ping.

Raycoms commented 5 years ago

I checked through the crash log, and I believe strongly that this crash is not on our side (still).

What is crashing is when we add the inventory in the set of inventories. This then crashes in SidedInvWrapper in the hashSet calculation.

image

Because the side is null. We don't use SidedInvWrapper, is it possible that your mod creates a sidedInvWrapper somewhere with a null side?

tomdodd4598 commented 5 years ago

I'll check again, I may have missed something when I rushed the last build out - sorry I haven't been able to for a while, just had some exams to do, but nearly there :)

tomdodd4598 commented 5 years ago

@Raycoms Found the mistake on my side, soz for not spotting it earlier :P Will get the build out soon ;)

marchermans commented 5 years ago

@turbodiesel4598 could you post the version number here. That way we CArelay that information to our users thanks.

tomdodd4598 commented 5 years ago

The next version will be 2.17b ;)

Raycoms commented 5 years ago

Awesome, great job

Am Fr., 12. Juli 2019 um 20:59 Uhr schrieb Tom Dodd < notifications@github.com>:

The next version will be 2.17b ;)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/turbodiesel4598/NuclearCraft/issues/517?email_source=notifications&email_token=ABRD3S3POMQBBAR2JAIC5B3P7ELFXA5CNFSM4HM6HRS2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZ3EIGY#issuecomment-511067163, or mute the thread https://github.com/notifications/unsubscribe-auth/ABRD3S3UCD2EASJ66BPCPM3P7ELFXANCNFSM4HM6HRSQ .