Maptool: 1.5.0-rc.4 defineFunction with open edit window

[aliasmask]

If I have an open edit window and run onCampaignLoad defining the current macro I am editing, this will cause and error.

java.lang.NullPointerException
    at net.rptools.maptool.client.ui.syntax.MapToolScriptSyntax.addToken(MapToolScriptSyntax.java:125)
    at net.rptools.maptool.client.ui.syntax.MapToolScriptTokenMaker.addToken(MapToolScriptTokenMaker.java:341)
    at net.rptools.maptool.client.ui.syntax.MapToolScriptTokenMaker.addToken(MapToolScriptTokenMaker.java:330)
    at net.rptools.maptool.client.ui.syntax.MapToolScriptTokenMaker.yylex(MapToolScriptTokenMaker.java:684)
    at net.rptools.maptool.client.ui.syntax.MapToolScriptTokenMaker.getTokenList(MapToolScriptTokenMaker.java:396)
    at org.fife.ui.rsyntaxtextarea.RSyntaxDocument.getTokenListForLine(RSyntaxDocument.java:431)
    at org.fife.ui.rsyntaxtextarea.WrappedSyntaxView$WrappedLine.calculateLineCount(WrappedSyntaxView.java:1159)
    at org.fife.ui.rsyntaxtextarea.WrappedSyntaxView$WrappedLine.getPreferredSpan(WrappedSyntaxView.java:1217)
    at java.desktop/javax.swing.text.View.getMinimumSpan(View.java:249)
    at java.desktop/javax.swing.text.BoxView.calculateMajorAxisRequirements(BoxView.java:872)
    at java.desktop/javax.swing.text.BoxView.checkRequests(BoxView.java:931)
    at java.desktop/javax.swing.text.BoxView.setSpanOnAxis(BoxView.java:330)
    at java.desktop/javax.swing.text.BoxView.layout(BoxView.java:710)
    at java.desktop/javax.swing.text.BoxView.setSize(BoxView.java:398)
    at org.fife.ui.rsyntaxtextarea.WrappedSyntaxView.setSize(WrappedSyntaxView.java:1009)
    at java.desktop/javax.swing.plaf.basic.BasicTextUI$RootView.setSize(BasicTextUI.java:1815)
    at java.desktop/javax.swing.plaf.basic.BasicTextUI.getPreferredSize(BasicTextUI.java:942)
    at java.desktop/javax.swing.plaf.basic.BasicTextAreaUI.getPreferredSize(BasicTextAreaUI.java:121)
    at java.desktop/javax.swing.JComponent.getPreferredSize(JComponent.java:1680)
    at java.desktop/javax.swing.JTextArea.getPreferredSize(JTextArea.java:609)
    at java.desktop/javax.swing.ScrollPaneLayout.layoutContainer(ScrollPaneLayout.java:795)
    at java.desktop/java.awt.Container.layout(Container.java:1535)
    at java.desktop/java.awt.Container.doLayout(Container.java:1524)
    at java.desktop/java.awt.Container.validateTree(Container.java:1720)
    at java.desktop/java.awt.Container.validate(Container.java:1655)
    at java.desktop/javax.swing.RepaintManager$3.run(RepaintManager.java:737)
    at java.desktop/javax.swing.RepaintManager$3.run(RepaintManager.java:735)
    at java.base/java.security.AccessController.doPrivileged(Native Method)
    at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:87)
    at java.desktop/javax.swing.RepaintManager.validateInvalidComponents(RepaintManager.java:734)
    at java.desktop/javax.swing.RepaintManager$ProcessingRunnable.run(RepaintManager.java:1879)
    at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:313)
    at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:770)
    at java.desktop/java.awt.EventQueue.access$600(EventQueue.java:97)
    at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:721)
    at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:715)
    at java.base/java.security.AccessController.doPrivileged(Native Method)
    at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:87)
    at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:740)
    at net.rptools.maptool.client.swing.MapToolEventQueue.dispatchEvent(MapToolEventQueue.java:43)
    at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
    at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
    at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:117)
    at java.desktop/java.awt.WaitDispatchSupport$2.run(WaitDispatchSupport.java:190)
    at java.desktop/java.awt.WaitDispatchSupport$4.run(WaitDispatchSupport.java:235)
    at java.desktop/java.awt.WaitDispatchSupport$4.run(WaitDispatchSupport.java:233)
    at java.base/java.security.AccessController.doPrivileged(Native Method)
    at java.desktop/java.awt.WaitDispatchSupport.enter(WaitDispatchSupport.java:233)
    at java.desktop/java.awt.Dialog.show(Dialog.java:1070)
    at java.desktop/java.awt.Component.show(Component.java:1674)
    at java.desktop/java.awt.Component.setVisible(Component.java:1621)
    at java.desktop/java.awt.Window.setVisible(Window.java:1031)
    at java.desktop/java.awt.Dialog.setVisible(Dialog.java:1005)
    at net.rptools.maptool.client.swing.MapToolEventQueue.displayPopup(MapToolEventQueue.java:79)
    at net.rptools.maptool.client.swing.MapToolEventQueue.dispatchEvent(MapToolEventQueue.java:55)
    at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
    at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
    at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:117)
    at java.desktop/java.awt.WaitDispatchSupport$2.run(WaitDispatchSupport.java:190)
    at java.desktop/java.awt.WaitDispatchSupport$4.run(WaitDispatchSupport.java:235)
    at java.desktop/java.awt.WaitDispatchSupport$4.run(WaitDispatchSupport.java:233)
    at java.base/java.security.AccessController.doPrivileged(Native Method)
    at java.desktop/java.awt.WaitDispatchSupport.enter(WaitDispatchSupport.java:233)
    at java.desktop/java.awt.Dialog.show(Dialog.java:1070)
    at java.desktop/java.awt.Component.show(Component.java:1674)
    at java.desktop/java.awt.Component.setVisible(Component.java:1621)
    at java.desktop/java.awt.Window.setVisible(Window.java:1031)
    at java.desktop/java.awt.Dialog.setVisible(Dialog.java:1005)
    at net.rptools.maptool.client.swing.MapToolEventQueue.displayPopup(MapToolEventQueue.java:79)
    at net.rptools.maptool.client.swing.MapToolEventQueue.dispatchEvent(MapToolEventQueue.java:55)
    at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
    at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
    at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
    at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
    at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
    at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

Error doesn't occur until I click on the edit window. After the original error is closed and bunch of NPE windows pop up after it spamming the spacebar closing the error windows followed by clicking on the X to close the edit window will eventually get you out of loop.

[Azhrei]

Lol, and this is why a modal dialog might be better!

Yes, I can understand that trying to modify the object you’re editing could result in bad state in either of two places, and we can likely fix the editor, but when the state problem is on the other end, it may not manifest cleanly in one spot.

Thanks for the report. 🙂

[Azhrei]

Perhaps a lock around the methods that retrieve/modify the function list would be sufficient. I can’t imagine they’re called too often, except during campaign load, so there would likely be very little contention...

[sentry-io[bot]]

Sentry issue: MAPTOOL-S

[aliasmask]

You could probably just close any open edit windows if defining the macro being edited.

[source-knights]

got a fix. Code should not use the instance variable but call the getter to get the initialized map

[source-knights]

just checking if we need locking as well, not really sure how the thread handling works. I hope we only have kind of 1 thread for handling UI calls (mostly I see that's the case by adding runabled to the queue)

[source-knights]

Fix in https://github.com/RPTools/maptool/pull/330

I think changed it to use the get... method fixes it already (tested) but just in case there is some multi threading somewhere I added double check locking as well

[Azhrei]

@rkathey You marked this "post Nerps merge", but isn't this a fix we want in 1.5.0? I won't touch it, but if you think it should go into 150, you should change the project designation. I don't consider this high priority, but it seems an easy fix.

[cwisniew]

This seems like it's pretty easy to avoid the problem so lets document it as a known issue for the release notes and deal with it after release.

[rkathey]

I wouldn’t hold up the release for it. We should include it it known issues and fix it in 151.

[rkathey]

I could not recreate the stack trace on this one so it seems like it's fixed. @aliasmask will need to test his scenario to make sure.

However, there is an interesting side effect of loading a campaign with an editor window up: if you hit ok after the load, it will save the macro in the new campaign. That could have some interesting consequences for things like onCampaignLoad.

For a future patch, the devs should consider either force closing all editors between campaign loads or giving the user a warning.