Piotrekol / StreamCompanion

osu! information extractor, ranging from selected map info to live play data
MIT License
374 stars 59 forks source link

Add: Support instructing browsers to cache backgroundImage. #383

Closed LamGC closed 1 year ago

LamGC commented 1 year ago

This Pull Request adds a Cache-Control response header to the /backgroundImage endpoint in WebSocketDataSender plugin.

By instructing the browser to cache background images, it can significantly improve the loading speed of previously loaded Beatmap background images reloading in the browser page.

image

It should be noted that by default, the Cache-Control response header will not be added, and the requester must attach the query string cache=true in order for the plugin to add cache instructions to the response. (This is to ensure compatibility)

The recommended approach is to use it in conjunction with a "meaningless" query string that identifies the background image, like this:

http://localhost:20727/backgroundImage?cache=true&mapid=668662

The mapid will not be processed by the plugin, but it can allow the browser to uniquely identify the background image and cache it for use during the next load.

Piotrekol commented 1 year ago

I don't see a reason why this couldn't be new default.. more than that would there be a real reason for a client to ever want to opt out of this? all overlays provided with SC already use map hash as a "meaningless" query parameter, that changes with map updates - which indicates that map image could have changed.

so I'd vote for always including cache header

LamGC commented 1 year ago

My main concern is that some of the overlays created by players themselves directly use URLs without map hashes, which may cause problems.

I cannot guarantee that the player creating the Overlay has sufficient knowledge to prevent cache expiration issues. I hope to allow cache as an optional feature to transition for a period of time, so that players can understand and use browser caching.

If caching needs to be set as the default behavior, it is recommended to point it out as a Breaking Change in the next version.

Piotrekol commented 1 year ago

I forgot we're working with /backgroundImage endpoint, with no parameters. In that case having caching under a flag does absolutely make sense.

Update API docs with cache flag and it should be good to go - https://github.com/Piotrekol/StreamCompanion/blob/master/docs/docs/development/SC/api.md#backgroundimage

Piotrekol commented 1 year ago

reworded second paragraph and changed it to a tip instead image