Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add: Support instructing browsers to cache backgroundImage. #383

Merged
merged 3 commits into from
Jun 22, 2023

Conversation

LamGC
Copy link
Contributor

@LamGC LamGC commented Jun 13, 2023

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.

By instructing the browser to allow caching of resources, it can significantly speed up the browser's loading of previous images (due to the use of hard disk caching).
@Piotrekol
Copy link
Owner

Piotrekol commented Jun 17, 2023

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
Copy link
Contributor Author

LamGC commented Jun 19, 2023

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
Copy link
Owner

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

LamGC and others added 2 commits June 22, 2023 12:50
…oundImage" endpoint.

P.S. If I were to speak, it would probably be a lengthy discussion, so I tried to briefly explain its usage.
@Piotrekol
Copy link
Owner

reworded second paragraph and changed it to a tip instead
image

@Piotrekol Piotrekol merged commit b6b7396 into Piotrekol:master Jun 22, 2023
github-actions bot pushed a commit that referenced this pull request Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants