[Bug]: Token/Map assets showing "received invalid image" error
Describe the Bug
After installation of 1.16.2, players (but not the GM) when attempting to load a map will receive and error noting "received an invalid image" and list of what seems to be an asset id. In my particular case this was two tokens created at approximately the same time (may have been done either pre- or post-installation of the new version of Maptool; cannot be certain). Was necessary to recreate the tokens (go back and pull the art asset into TokenTool) to make the problem go away. Accompanying will be screenshots of the error reports, and copies of the two tokens producing the errors.
To Reproduce
There seems no reliable way to duplicate the bug, though two other users on the Discord have spoke about similar situations and error reports post 1.16.2
Expected Behaviour
Normal map loading behavior.
Screenshots
See above.
MapTool Info
Version 1.16.2
Desktop
Windows 11 Version 10.0.26100 Build 26100
Additional Context
No response
I tried reproducing this with the supplied images but was not able to. Though the IDs I got are different than in the shared screenshot, so I wonder if these images are a bit different of if there is something else going on here. For me, the first image has ID 02e1e4e3c35a481e0971d8f88de7cfca and the second has ID 4c7ae49b6f37ad073ef231aa167c2d95.
It might have to do with my having to download them again after I'd uploaded them to the Discord (I didn't think to keep the originals after that). As I recall, the way Discord does image "download" is it loads it into the browser and you have to save it from there. It would not surprise me that through all that process whatever bit is creating the problem might be reset.
However, duplicable or not, two other people reported similar problems on the Discord right after I did, so I still think there's a problem that needs monitoring here. Personally, I rolled back to 1.15.2
I was one of the users he mentioned that also had an issue with players getting invalid images when trying to connect. Unfortunately he did not get a screenshot of the error message. My tokens are not from TokenTool. I had not made any changes to my maps and after upgrading to 1.16.2 my players were getting the error. I fixed by creating a respository. That fixed the issue for all but one player.
I also encountered this, the most immediate cause appears to be that the hash of the downloaded image doesn't match the hash from the server's asset cache, I'm going to dig further.
The commit that introduces the problem was mine, https://github.com/RPTools/maptool/pull/5153 Sorry about that. Working on a fix ASAP.
Problems were not discovered earlier because to trigger the bug you need to have multiple map tokens sharing the same large asset.
Current hypothesis is that when a map with multiple copies of a token is loaded it starts transferring that asset once for each token, so it'll get duplicate chunks of the asset, and since the AssetHeader DTO doesn't have a range or chunk sequence number it will write those duplicate chunks to a file and complete early with incorrect data.
The commit that introduces the problem was mine, #5153 Sorry about that. Working on a fix ASAP.
That's ok, these things happen... but we are going to have to dock your pay 😆
Inevitably with software--or games, far as that goes--things will crop up once they're out in the wild that didn't show with the narrower testbed populace.
I think I tracked it down.
AssetManager calls assetLoader.isIdRequested(key) and then assetLoader.requestAsset(key), and both of those methods are synchronized, but if a request is made in parallel for every token on the map it'll block waiting to determine if there's already a pending request, they'll probabably all get the response that it hasn't been requested and then all proceed to request it in parallel.
I could wrap the calls in another synchronized block, but needing to remember to do that before calling isIdRequested is an API that's too easy to misuse, so I'm going to try making requestAsset idempotent.
I've created https://github.com/RPTools/maptool/pull/5451 targeting the 1.16 release branch since that's the first place I broke it.
🤔 this bug got past me because my test campaign wasn't sufficiently complex, and it's possible to miss it entirely if your GM only introduces new tokens one at a time while you're connected.
I think this means I should sort out a NERPS-like soft-fork for my group so I can do some real-world testing of big changes before breaking them up and trickling them into MapTool upstream through PRs.
Fixed in #5451