PC-Logix / OpenFM

Streaming radio mod for Minecraft, with OpenComputers control support
MIT License
25 stars 17 forks source link

[Crash] Playing an invalid song will crash the game #38

Closed 5lx closed 8 years ago

5lx commented 8 years ago

When I tried to listen an invalid song (m4a format), the log will show:

[Client thread/INFO]: javax.sound.sampled.UnsupportedAudioFileException: file is not a supported file type

Then if I try to click any button on the Radio GUI, It won't work any more and the log will say something about the codec:

io.netty.handler.codec.EncoderException: java.lang.NullPointerException
    at io.netty.handler.codec.MessageToMessageEncoder.write(MessageToMessageEncoder.java:107)
    at io.netty.handler.codec.MessageToMessageCodec.write(MessageToMessageCodec.java:116) 

Then if I close the GUI, and reopen it, it will crash the game:

java.lang.NullPointerException
    at pcl.OpenFM.GUI.GuiRadio.func_73866_w_(GuiRadio.java:84) ~[GuiRadio.class:?]
    at net.minecraft.client.gui.GuiScreen.func_146280_a(GuiScreen.java:255) ~[bdw.class:?]
    at net.minecraft.client.gui.inventory.GuiContainer.func_146280_a(GuiContainer.java) ~[bex.class:?]

OpenFM version: OpenFM-1.7.10-0.1.0-34 Forge version: 1.7.10-10.13.4.1614 OC version: OpenComputers-MC1.7.10-1.5.22.46-universal

5lx commented 8 years ago

In pcl/OpenFM/GUI/GuiRadio.java

if (!(this.radio.streamURL == null) || !this.radio.streamURL.equals("")) {
    this.streamTextBox.setText(this.radio.streamURL);
} else {
    this.streamTextBox.setText(OFMConfiguration.defaultURL);
}

The problem is the code determine !(this.radio.streamURL == null) and !this.radio.streamURL.equals("") in the same time.

CaitlynMainer commented 8 years ago

This is short-circuiting

One great example of short-circuiting relates to object references:

if (a != null && a.getFoo() != 42) {
    // Do something
}
a.getFoo() would normally throw a NullPointerException if a were null, but because the expression short-circuits, if a != null is false, the a.getFoo() part never happens, so we don't get an exception.

Note that not all expressions are short-cicuited. The || and && operators are short-circuited, but | and & are not, nor are * or /; in fact most operators are not.

The issue is not that they are "on the same line" But that I used Logical OR, not Logical AND (I have ||, not &&). The check needs to be if streamURL is not null AND streamURL is not empty then set the textBox, Currently it checks if its not null OR it's not empty.

CaitlynMainer commented 8 years ago

Build 35 will be on curse as soon as it is approved if you could test this change for me it would be great.

5lx commented 8 years ago

@CaitlynMainer Thanks for the fixing, it won't crash any more.

But I found another small issue. After playing an invalid song, the Radio can't play new songs, even if I corrected the url. There's no response when I click the play button, and the logs didn't show anything. In this situation, if I try to use .play() function, the logs will show it is streaming, but I can't hear the song.

PS: After playing an invalid song, the .isPlaying will return true. I don't know if this is the reason.

CaitlynMainer commented 8 years ago

Yeah that's been an issue since the beginning, basically the backend decoder fully crashes, and there isn't a decent way I've found the respawn it.

5lx commented 8 years ago

Maybe now the best way is to destroy and place it on again. Thanks for your detailed explanation.