[Bug]: Maptool hangs at splash-screen after macro import with unkown asset-link

[Xun71]

Describe the Bug

Importing a macro featuring an ICON (asset-link) into maptool's Global macro window leads to the effect that maptool does not start anymore after closing.
Starting MT again keeps it frozen at the splash-screen, last entry in log is: 'INFO net.rptools.maptool.client.AppUpdate - Cleaned version: 1.11.5'. Memory-usage displayed in TM: ~232MB, no CPU activity.

Only de-install, clearing of all reg-entries referring 'maptool', and re-start of the computer allows a functionig new-install.
The issue can be narrowed down to the case, where the asset-link is not known ('red X as defaouzlt ICON)

To Reproduce

1. Start fresh installation of MT.
2. import macro with an (unknown) asset referred in the details tab (e.g.
   
3. apply the change to the macro
4. close MT
5. start MT
6. *hangs at splash-screen'

Expected Behaviour

I would expect MT to ignore the unknown asset-link and keep the default 'red X' Icon

Screenshots

No response

MapTool Info

version 1.11.5

Desktop

Windows 11, 64bit

Additional Context

No response

[kwvanderlinde]

A quick test shows that this also impacts Linux, and a check of the code suggests all platforms will be impacted similarly. As far as I can tell from drilling into the code, this is what causes the issue:

At a high level, the problem stems from a weakness in MapTool's initialization code: there is no guarantee that all the "plumbing" exists (e.g., the server command, or the connection beneath that) prior to all the UI-centric code that might rely on it. In this case, the UI is trying to load assets and expects to be able to transfer them from the server, but since the server plumbing isn't set up yet that just fails.

At a very low level, this particular issue is caused by a weakness in realized in AssetLoader.ImageRetrievalRequest::run(), which runs deep in the call stack and through a tangle of asynchrony. The fallback logic is particularly at fault:

MapTool.serverCommand().getAsset(id);

Right now the problems with it are:

1. ImageRetrievalRequest relies on this fallback logic to actually load the asset and therefore notify listeners.
2. There is no ServerCommand created yet, so this line in fact throws a NPE and does not notify listeners.
3. Even if we have a ServerCommand (e.g., by creating it only one line earlier), it won't have a connection and so doesn't do anything.
4. Way up the stack in the main thead, ImageManager::getImageAndWait() is awaiting a latch and since ImageRetrievalRequest fails to ensure that the request is completed, ImageManager's listeners aren't called and therefore the latch is never released and MapTool hangs.

We can work around this for now by just checking whether the MapTool.serverCommand() is not null and return a broken asset otherwise (I'll get a PR up for that soon unless someone else chimes in). But more importantly, I feel we should work toward a couple of general goals in the codebase:

1. Make dependencies clear and enforceable. E.g., UI shouldn't be able to summon plumbing that may or may not exist, ServerCommand instances should require a connection to exist before it the ServerCommand can exist, etc.
2. Aim for more simplified asynchrony. Avoid locking and bespoke stateful asynchronous solutions, and instead prefer task queues / futures / etc that have clear indicators of success and failure and which can be understood locally.

[Phergus]

Tested. Macros with invalid asset IDs can now be imported and no longer cause MT to hang on launch.