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

Implement sending remote resource packs to Bedrock clients #4205

Closed
wants to merge 51 commits into from

Conversation

onebeastchris
Copy link
Member

@onebeastchris onebeastchris commented Oct 10, 2023

Since 1.20.30, it is possible to send clients a link to download a resource pack from - instead of having to add the pack to Geyser's pack folder. This should help with proxy networks & overall traffic usage, as now resource intensive pack sending can be offloaded to a designated location. However, unlike on Java, it is possible to send multiple entries for the client to load packs from! Further, this allows updating the remote resource packs while Geyser is still running: Geyser will detect the change, and re-download the pack. Unfortunately, this does not change when Bedrock clients can receive resource packs: only on the initial join.

Important

Note: You'll need to update Bedrock to the latest client version (1.21.1). 1.21.0 is unfortunately bugged, and ignores remote resource packs. 1.20.80 should also support this.


New config option:

# A list of links to send to the client to download resource packs from.
# These must be direct links to the resource pack, not a link to a page containing the resource pack.
# If you enter a link here, Geyser will download the resource pack once to check if it's in a valid format.
# See https://wiki.geysermc.org/geyser/packs for more info.
resource-pack-urls:
  # GeyserOptionalPack
  - "https://download.geysermc.org/v2/projects/geyseroptionalpack/versions/latest/builds/latest/downloads/geyseroptionalpack"

This will ensure that all connecting clients will, by default, get the GeyserOptionalPack.


You can download builds here:


Technical details:

  • A Bedrock client will receive the cdn entries (urls) alongside all other packs. If it fails to download the pack from the url, it'll fall back to requesting the pack from Geyser.
  • Geyser downloads all packs once when starting (or uses a cached download) to ensure that the resource pack is valid, and to parse the pack version/uuid/manifest. Further, it caches a fallback pack to use in the case that the url is offline, not reachable, or just denied by the Bedrock client for some reason.
  • Bedrock clients are very picky. The requirements for which links do and don't work are not fully known at the moment.
    What is known:
    • A "content-length" header field must be set to the correct file size. Otherwise, the pack will not be downloaded.
    • The application/zip content type seems to work best for downloading packs. The often used text/html type does not work (rip onedrive links).
    • Redirects seem to be followed correctly, but the link should still be a direct download
    • The pack can be either a .zip or .mcpack

Changes:

  • add new GeyserUrlPackCodec (and UrlPackCodec for api) classes that can create a pack based on url + content key
  • add a resource-pack-urls entry in the config for remote packs
  • try and use more fastutil for resource packs

TODO:

  • Proper error handling: URL invalid/offline, corrupt pack, manifest not where it's supposed to be, etc.
  • documentation on wiki
  • remove debug
  • dont let resource pack downloading block main thread - this is actually wanted; since Geyser should not start before we know all the resource packs
  • Don't re-download the pack if there's no need to (hash from url + size)
  • Testing

Would close #4060 - atleast the resource pack part.

@onebeastchris onebeastchris added PR: Feature When a PR implements a new feature API The issue/feature request relates to the Geyser API labels Oct 10, 2023
@Camotoy
Copy link
Member

Camotoy commented Oct 10, 2023

In the config I would use, probably, resource-pack-urls or remote-resource-packs. I don't think most people will understand what a CDN is, and some resource packs won't be hosted on content delivery networks per se.

Copy link
Member

@Konicai Konicai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a quick review without much depth - and i agree with Camo.

it would be nice if ResourcePackCDNEntry could be RemoteResourcePack, but it doesn't implement ResourcePack so... unsure about. the naming so far feels clunky tbh.

also consider using fastutil more, in the core module.

@onebeastchris
Copy link
Member Author

onebeastchris commented Oct 10, 2023

Yeah naming really isn't my strong suit, I agree. I kind of like RemotePack actually.
Or we could take a page from kyori's adventure, their new resource pack method takes in a ResourcePackRequest (Like).. not sure I really like that though. Open for suggestions :)

@Kas-tle
Copy link
Member

Kas-tle commented Oct 12, 2023

How exactly are pack versions handled with this? Is it just pulled from the downloaded pack? What happens if the version of the pack at the link is updated while the server is running?

@onebeastchris

This comment was marked as outdated.

@Kas-tle
Copy link
Member

Kas-tle commented Oct 12, 2023

Hmm ok but then how does this work? Does this mean the client has to download the entire pack every time to check if the version changed? The normal ResourcePackStack and ResourcePacksInfo packets have a field for version. So does a CDN entry also require an entry in the normal Entry list and this just tells the client where it can download said pack? Or will the client just download a CDN entry and not check the version at all?

I'm mainly wondering how this works if the user just adds an entry to their config for one of these. In this scenario you're saying we never have to know the pack version?

@onebeastchris
Copy link
Member Author

onebeastchris commented Oct 14, 2023

By what I can tell, we never have to get/tell the client the pack version.This does raise the question of whether the client will always download the pack...
I'll definitely have to do more testing here; I am not fully sure about the exact workings of this feature. In my (brief) testing, the optional pack seems to have been loaded fine.

After some testing:

  • We need to send an entry in the ResourcePackInfo packet, even for these cdn packs.
  • the "packid" in the cdn entry consists of uuid_version, not just the UUID
  • the client falls back to requesting the packs normally/the old way if the download didn't succeed
  • We can actually detect the last part, and try and re-load packs. Not sure if we can do that fully automatically, but in theory, should be doable.

@onebeastchris onebeastchris added the Work in Progress The issue is currently being worked on. label Oct 16, 2023
@onebeastchris onebeastchris marked this pull request as draft October 16, 2023 17:43
Instead:
- add UrlPackCodec & GeyserUrlPackCodec
- try and provide the resource pack via a stream from the url - either because the client does not support packs via the url, or because it failed to get the packs
…haos ensues since the client will either show bogus resource pack size values (invalid sizes), or skip resource packs altogether (if not properly zipped).
@onebeastchris onebeastchris marked this pull request as ready for review November 9, 2023 11:44
@onebeastchris
Copy link
Member Author

From more testing locally, here's what i found:
This PR should be functional, but Bedrock is very picky about what sort of packs from a url are accepted/downloaded.

  • The link must be a direct link.
  • The size must be set properly in the Content-Length header. If size is wrong, or not present (-1), the client will show a bogus, stupidly large size value and wont allow downloading the packs.
  • Similarly, the Content-Type header must be set to application/zip - otherwise, the client wont download the resource packs.

Further, the resource pack can be either a .zip or .mcpacks file that does not contain the pack files at its root, but rather a folder with the actual resource pack. (In other words: zip/manifest.json wont work, zip/mypack/manifest.json will....)

If anyone wishes to test this PR, you can set up a working file server using caddy and the following config - please do not use it in production, it doesnt even use https.

http://localhost {
    file_server browse
    route {
        header Content-Type application/zip
        header Content-Length {file.size}
    }
}

It however would ensure correct content type/content lengths being set.

On a positive note, downloading even a 100MB pack is insanely quick!

@onebeastchris onebeastchris added PR: Needs Testing When a PR needs testing but is currently not under review and removed PR: On hold When a PR is on hold like if it requires a dependency to be updated first PR: Needs Testing When a PR needs testing but is currently not under review labels Jun 20, 2024
@onebeastchris onebeastchris added the PR: Needs Testing When a PR needs testing but is currently not under review label Jun 25, 2024
@onebeastchris onebeastchris changed the title Implement sending CDN entries to Bedrock clients to download resource packs from Implement sending remote resource packs to Bedrock clients Jun 26, 2024
// This doesn't seem to be a requirement (anymore?). Logging to debug as it might be interesting though.
if (type == null || !type.equals("application/zip")) {
logger.warning(String.format("Application type received from remote pack at URL %s uses the content type: %s! This may result in packs not loading " +
"for Bedrock players.", url, type));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion this should say something along the lines of Try using a link that is application/zip type instead! So that people know what the content type should be instead of being confused on why the content type wont work and not being able to know what the right one is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! Yeah; this is temporary to figure out what application types are (or aren't) supported by Bedrock edition while testing. This will be changed before merging.

So far, it unfortunately seems that only application/zip works - the popular octet-stream type seems to not work. Which is unfortunate, as that's what many services use

@onebeastchris onebeastchris added the PR: On hold When a PR is on hold like if it requires a dependency to be updated first label Aug 2, 2024
@onebeastchris
Copy link
Member Author

Put on hold until we transition to using configurate - otherwise these changes will result in confusion about why packs are loaded that don't seem to be in the config.

@onebeastchris
Copy link
Member Author

Superseded by #4978

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API The issue/feature request relates to the Geyser API PR: Feature When a PR implements a new feature PR: Needs Testing When a PR needs testing but is currently not under review PR: On hold When a PR is on hold like if it requires a dependency to be updated first
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loading resource pack from URL
6 participants