
Server crashes when elytra are enabled
kang-png opened this issue · 14 comments
Please check any boxes that apply to you and your issue
-
I use a translator application to post this issue.
-
This is a crash. Please upload, Pastebin, Gist or copypaste the whole crash report along with this issue.
-
This is a mod incompatibility. If I do this in vanilla Forge with only Et Futurum Requiem installed, it works normally.
-
This is a code problem. Please link the probably problematic lines of code in the issue and/or explain in detail what is wrong.
Version number of Et-Futurum-Requiem (IMPORTANT)
Specify the actual version, please don't just say "latest", tell us the version number of the mod in-game, or the name the jar had when downloading it.
Et_Futurum_Requiem-2.4.2
Describe the issue (IMPORTANT)
A list of steps to reproduce the issue may be helpful too. Screenshots could also prove beneficial for visual issues.
Mod list (OPTIONAL)
Crash reports contain the mod list to copy. Please use Pastebin or Gist if it is too large. If you don't have a crash report, open any world and hold F3 + C for 10 seconds to crash the game manually.
Et_Futurum_Requiem-2.4.2.jar
worldedit-forge-6.1.1-dist.jar
AsyncWorldEditInjector.jar
Additional Context (OPTIONAL)
Any other info that may have been missed? If you suspect other mods to cause this issue, list them here and tell us why you suspect each of them.
log:https://pastebin.com/gSbXpYNp
crash:https://pastebin.com/r1TXmBMU
When I disable this, the mod loads normally
# Enables the elytra item. # Modified Classes: net.minecraft.entity.EntityLivingBase net.minecraft.entity.player.EntityPlayer net.minecraft.entity.EntityTrackerEntry net.minecraft.network.NetHandlerPlayServer net.minecraft.client.entity.AbstractClientPlayer net.minecraft.client.entity.EntityPlayerSP net.minecraft.client.model.ModelBiped net.minecraft.client.renderer.entity.RenderPlayer [default: true] B:enableElytra=false
Oh you got the Link that actually lines up properly, in which case ete is indeed null, no amount of "denial" can change that. So better add a null check there, or a "remove(null);" aimed at the HashSet to prevent further Issues elsewhere too.
And does Vanilla really not do any null checking? Even an instanceof is a null check. Or something in EFR is adding a null to it? I dont actually know, I only hypothesize there.
@Roadhog360 the Error should be in the following Line, since the Code does not line up according to the Exception in those Logs.
ws.getEntityTracker().trackedEntities
if getEntityTracker() can return null, this will fail with a NullPointerException.
I thought it was this line - this is line 1250 in the 2.4.2 version of the source code.
Which would mean
ete
is null, which is possible if ws.getEntityTracker().trackedEntities
(a HashSet) contains a null element. But vanilla code doesn't do null checking when iterating over the elements of the set either, so in this case a crash should happen somewhere else even if it wasn't for EFR.
As for your explanation, it doesn't add up either. getEntityTracker()
returns the value of a final field which is initialized to something in WorldServer
's constructor, meaning it should never be null. Uranium is probably doing something weird.
And does Vanilla really not do any null checking?
Yep, see EntityTracker#updateTrackedEntities
for example.
I would suggest just adding a null check and if someone comes by with a real update on this issue, we can find the problem later. Better than just randomly crashing anyone for no reason, since that causes more damage than not knowing the source of the issue. The damage being that people disable the elytra on their servers, and their end users being left without a feature they hoped they would have, which is far worse than a mystery crash.
I need to reproduce this before I can guarantee that is the issue. As previously pointed out, vanilla does not do any null checks. So there's no way that's the issue, or else the vanilla code would be crashing instead. I don't think lists can even contain a null value.
uranium has some code for "compatible plugins to register forge events", the attached file
is a comparison of two forge+bukki hybrid server cores
报告.pdf
https://github.com/UraniumMC/Uranium/blob/f73a12f22bda26b7db8bf2c80258c63767b272aa/patches/cpw/mods/fml/common/eventhandler/ASMEventHandler.java.patch#LL2C5-L2C8
I had not noticed this was on a Bukkit hybrid server. Those are unfortunately not supported because they break too easily.
Ah that explains it!
Would still be nice if there was a basic null check though, those are practically free in java and dont hurt. As a bonus, people will be less likely to report this Bug over and over and over again, wasting your time. ;)
We don't need a null check because lists I don't think can even contain null values, and if it was null then vanilla code would crash first.
Lists IN GENERAL can contain a LOT of null values if such are added to them. There is no inherent protection against them. You should at least TRY to support these cases, especially when it is easy to make them not happen, because what if this was an optimization they did to vanilla code because removing elements from a list is more expensive than checking for a null? This is why just adding a null check is a good idea overall.
It's not that I believe it's expensive, I just don't like the way unnecessary checks look