search

jpenilla / squaremap

squaremap is a minimalistic and lightweight world map viewer for Minecraft servers, using the vanilla map rendering style
https://modrinth.com/plugin/squaremap
Other
338 stars 40 forks source link

Registered icon removal causes 404 errors on external web servers #114

Closed cbs228 closed 1 year ago

cbs228 commented 1 year ago

Icons added via IconRegistry.register() mechanism are removed from the web directory when squaremap shuts down. While this is fine for the internal web server, it causes 404 errors when:

  1. An external web server is in use with settings.internal-webserver.enabled = false; and
  2. The squaremap plugin has been unloaded and/or the game server has shut down.

404

This is known to impact the squaremap-signs plugin as of jpenilla/squaremap-addons@dd136b94df86f2d, but other plugins are likely impacted as well. Map tiles retrieve just fine when the game server is shut down, but the sign icons do not.

Should IconRegistry.unregister() and friends be updated to avoid removing files? Does the registry need a "persistent" flag for icons that should remain?

jpenilla commented 1 year ago

I think for 99% of cases we would be okay with skipping the delete on unregister (and only having the registry delete files on initialization/replace them when an unregistered key gets used again).

The only downside I see would be for an addon that registers a lot of transient icons that don't need to be left around using disk space. But like I said that's quite the edge case and a trade-off I'm fine with to keep the map usable after shutdown. As far as expanding the icon registry API, sadly it would need to wait for 2.x unless we do something ugly to avoid breaking the ABI of Squaremap#iconRegistry.

cbs228 commented 1 year ago

PR #115 appears to do the trick. If cleanup is desired at some point in the future, we could implement one on startup or shutdown using an mtime-based time filter. I don't believe this is necessary for now.