Using F5 (show BT editor) in-game during multiplayer causes crash via NPE

Cervator commented 6 years ago

What you were trying to do

Open the behavior tree editor in-game via F5

What actually happened

Client crashed with an NPE

How to reproduce

Client connected to a multiplayer server with Behaviors active (assumed, might take less)
Press F5

Log details and game version

Omega 900 during today's play test

14:03:27.201 [main] ERROR o.terasology.engine.TerasologyEngine - Uncaught exception, attempting clean game shutdown
java.lang.NullPointerException: null
    at org.terasology.rendering.nui.TextLineBuilder.addText(TextLineBuilder.java:56)
    at org.terasology.rendering.nui.TextLineBuilder.getLines(TextLineBuilder.java:51)
    at org.terasology.rendering.nui.TextLineBuilder.getLines(TextLineBuilder.java:46)
    at org.terasology.rendering.nui.itemRendering.StringTextRenderer.getPreferredSize(StringTextRenderer.java:69)
    at org.terasology.rendering.nui.widgets.UIList.getPreferredContentSize(UIList.java:117)
    at org.terasology.rendering.nui.internal.CanvasImpl.calculateRestrictedSize(CanvasImpl.java:347)
    at org.terasology.rendering.nui.internal.CanvasImpl.calculatePreferredSize(CanvasImpl.java:333)
    at org.terasology.rendering.nui.layouts.ScrollableArea.getPreferredContentSize(ScrollableArea.java:193)
    at org.terasology.rendering.nui.internal.CanvasImpl.calculateRestrictedSize(CanvasImpl.java:347)
    at org.terasology.rendering.nui.internal.CanvasImpl.calculatePreferredSize(CanvasImpl.java:333)
    at org.terasology.rendering.nui.layouts.miglayout.MigComponent.calculatePreferredSize(MigComponent.java:41)
    at org.terasology.rendering.nui.layouts.miglayout.MigLayout.layoutContainer(MigLayout.java:223)
    at org.terasology.rendering.nui.layouts.miglayout.MigLayout.getPreferredContentSize(MigLayout.java:182)
    at org.terasology.rendering.nui.layouts.miglayout.MigLayout.getMaxContentSize(MigLayout.java:188)
    at org.terasology.rendering.nui.internal.CanvasImpl.calculateMaximumSize(CanvasImpl.java:362)
    at org.terasology.rendering.nui.internal.CanvasImpl.drawWidget(CanvasImpl.java:385)
    at org.terasology.rendering.nui.CoreScreenLayer.onDraw(CoreScreenLayer.java:115)
    at org.terasology.rendering.nui.internal.CanvasImpl.drawStyledWidget(CanvasImpl.java:406)
    at org.terasology.rendering.nui.internal.CanvasImpl.drawWidget(CanvasImpl.java:392)
    at org.terasology.rendering.nui.internal.NUIManagerInternal.render(NUIManagerInternal.java:475)
    at org.terasology.engine.modes.StateIngame.renderUserInterface(StateIngame.java:235)
    at org.terasology.engine.modes.StateIngame.render(StateIngame.java:219)
    at org.terasology.engine.subsystem.lwjgl.LwjglGraphics.postUpdate(LwjglGraphics.java:166)
    at org.terasology.engine.TerasologyEngine.tick(TerasologyEngine.java:446)
    at org.terasology.engine.TerasologyEngine.mainLoop(TerasologyEngine.java:400)
    at org.terasology.engine.TerasologyEngine.run(TerasologyEngine.java:376)
    at org.terasology.engine.Terasology.main(Terasology.java:154)
14:03:27.201 [main] INFO  o.terasology.engine.TerasologyEngine - Shutting down Terasology...

Maybe of interest to @l0ftyWhizZ and/or @Naman-sopho since they're digging around in the BT area. Probably an easy fix?

shreyasiyer14 commented 6 years ago

Although this isn't the cause of the problem, having a simple NULL check for text in TextLineBuilder would be better. Upon further inspection, the error arises when Pathfinding is enabled. (Which is auto-enabled as a dependency to Behaviors).
It occurs due to the presence of the deprecated convention based behavior node checkTargetSet, as a prefab in the module, and by removing it the BT-editor works fine.

Cervator commented 6 years ago

Thanks for the extra detective work :-)

I've merged in the two module PRs so this specific case shouldn't happen anymore. But it would still be good to add that null check with a quick warn log entry to go with it. Knowing a bit more about where should make resolving this issue trivial and indeed a Good First Issue!

On a kind of related note, we have the BT editor button in the main menu - but it doesn't do anything?

shreyasiyer14 commented 6 years ago

Yes, the button events for the BT editor button aren't subscribed in MainMenuScreen. It's currently just a dummy button. Following line is to be added in MainMenuScreen :

WidgetUtil.trySubscribe(this, "behavioreditor", button -> getManager().toggleScreen("engine:behaviorEditorScreen"));

Although this does result in a crash currently.

Cervator commented 6 years ago

Yeah I suppose the button made sense back when BTs were purely in the engine. Now they require at least the Behaviors module active. So lets add to this issue outright removing that button from the main menu. At least until we have the ability to activate modules inside the main menu.

Cervator commented 6 years ago

Closing this as complete. The button was just removed technically after v2.0.0 but still the crash etc was fixed then so leaving this issue in that milestone still.

MovingBlocks / Terasology