search

cc-tweaked / CC-Tweaked

Just another ComputerCraft fork
https://tweaked.cc
934 stars 211 forks source link

Better handling for playing invalid music discs (instead of crashing) #515

Closed PieKing1215 closed 4 years ago

PieKing1215 commented 4 years ago

I'm currently setting up a modpack, and I found that trying to play a modded music disc using "dj" causes the server to crash. It's triggering the requireNonNull on line 75 in PlayRecordClientMessage: https://github.com/SquidDev-CC/CC-Tweaked/blob/cd879b067fcdd7d8562f5930335d33e475ce55bc/src/main/java/dan200/computercraft/shared/network/client/PlayRecordClientMessage.java#L63-L77 I assume the root of the problem lies with the modded music discs being registered wrongly, but IMO this check could be done in more elegant way, just by adding the null check to the outer if. This way, if it can't play the disc it at least won't crash the server.

Here's the stacktrace:

Description: Ticking block entity

java.lang.NullPointerException: Sound is not registered
    at java.util.Objects.requireNonNull(Unknown Source) ~[?:1.8.0_211] {}
    at dan200.computercraft.shared.network.client.PlayRecordClientMessage.toBytes(PlayRecordClientMessage.java:75) ~[?:1.90.2] {re:classloading,pl:runtimedistcleaner:A}
    at net.minecraftforge.fml.network.simple.IndexedMessageCodec.lambda$tryEncode$4(IndexedMessageCodec.java:134) ~[?:?] {re:classloading}
    at java.util.Optional.ifPresent(Unknown Source) ~[?:1.8.0_211] {}
    at net.minecraftforge.fml.network.simple.IndexedMessageCodec.tryEncode(IndexedMessageCodec.java:132) ~[?:?] {re:classloading}
    at net.minecraftforge.fml.network.simple.IndexedMessageCodec.build(IndexedMessageCodec.java:147) ~[?:?] {re:classloading}
    at net.minecraftforge.fml.network.simple.SimpleChannel.encodeMessage(SimpleChannel.java:85) ~[?:?] {re:classloading}
    at net.minecraftforge.fml.network.simple.SimpleChannel.toBuffer(SimpleChannel.java:98) ~[?:?] {re:classloading}
    at net.minecraftforge.fml.network.simple.SimpleChannel.toVanillaPacket(SimpleChannel.java:129) ~[?:?] {re:classloading}
    at net.minecraftforge.fml.network.simple.SimpleChannel.send(SimpleChannel.java:124) ~[?:?] {re:classloading}
    at dan200.computercraft.shared.network.NetworkHandler.sendToAllAround(NetworkHandler.java:82) ~[?:1.90.2] {re:classloading}
    at dan200.computercraft.shared.util.RecordUtil.playRecord(RecordUtil.java:23) ~[?:1.90.2] {re:classloading}
    at dan200.computercraft.shared.peripheral.diskdrive.TileDiskDrive.playRecord(TileDiskDrive.java:503) ~[?:1.90.2] {re:classloading}
    at dan200.computercraft.shared.peripheral.diskdrive.TileDiskDrive.func_73660_a(TileDiskDrive.java:175) ~[?:1.90.2] {re:classloading}
    at net.minecraft.world.World.func_217391_K(World.java:473) ~[?:?] {re:classloading,pl:accesstransformer:B,pl:runtimedistcleaner:A,re:mixin,pl:accesstransformer:B,pl:runtimedistcleaner:A}
    at net.minecraft.world.server.ServerWorld.func_72835_b(ServerWorld.java:368) ~[?:?] {re:classloading,xf:fml:quark:change-sleeping-player-count,xf:fml:quark:add-rave-hook,xf:fml:immersiveengineering:IE block update callback,pl:runtimedistcleaner:A,re:mixin,pl:runtimedistcleaner:A}
    at net.minecraft.server.MinecraftServer.func_71190_q(MinecraftServer.java:849) ~[?:?] {re:classloading,pl:accesstransformer:B,pl:runtimedistcleaner:A}
    at net.minecraft.server.dedicated.DedicatedServer.func_71190_q(DedicatedServer.java:330) ~[?:?] {re:classloading,pl:accesstransformer:B}
    at net.minecraft.server.MinecraftServer.func_71217_p(MinecraftServer.java:784) ~[?:?] {re:classloading,pl:accesstransformer:B,pl:runtimedistcleaner:A}
    at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:637) [?:?] {re:classloading,pl:accesstransformer:B,pl:runtimedistcleaner:A}
    at java.lang.Thread.run(Unknown Source) [?:1.8.0_211] {}

I get this with Pigstep Backport or any of the records from Charm, but not any of the vanilla ones. I haven't tested any other modded ones. The records that crash CC work normally in a normal jukebox.

Let me know if you need more info. (I didn't think the full crash report or logs would really be of much use)

SquidDev commented 4 years ago

Having had a look at this, I'm not going to fix this. I've switched to using Forge's methods, which enforce the same checks, but should provide more helpful error messages in the general case.

If another mod isn't registering their SoundEvent (or any other ForgeRegistryEntry), it's going to blow up sooner or later. The best thing to do is file a report on their end.

Edit: Wrong button, woops.