feat: adding beet.contrib.snapshot#502
Conversation
|
Hello? |
|
@Stoupy51 Please don't spam comments on PR's |
Sorry, that wasn't my intention |
rx-dev
left a comment
rx-dev
left a comment
There was a problem hiding this comment.
couple of thoughts but i think it would be chill
| resolved = search_version(version) | ||
| try: | ||
| version = split_version(resolved) | ||
| except ValueError: |
There was a problem hiding this comment.
is the best way to determine whether we can use a specific version to chk for a ValueError, this feels wrong to me. esp since LATEST_MINECRAFT_VERSION is also the default here?
There was a problem hiding this comment.
i see, this was sorta how it was previously done...
There was a problem hiding this comment.
I don't really know how to do better, because if someone wants to develop in old snapshot schema, the split_version function raise a ValueError
Should i delete this try catch block or keep it that way ?
| """URL template to download the version manifest from. The {version} placeholder will be replaced with the Minecraft version.""" | ||
|
|
||
| url_command_tree: str = "https://raw.githubusercontent.com/misode/mcmeta/refs/tags/{version}-summary/commands/data.json" | ||
| """URL template to download the command tree from. The {version} placeholder will be replaced with the Minecraft version.""" |
There was a problem hiding this comment.
@misode do u think it's a good idea to encode the misode/mcmeta links as constants here? i don't see anything wrong with it, curious if u think it's a bad idea (maybe fine for contrib).
There was a problem hiding this comment.
Seems fine in this case. The alternative would be to run the data generator in beet and cache it in beet_cahce but that's obviously a lot more involved. Maybe could consider adding adding a few backup urls (like the spyglass api, or Ersatz' mcdata repo) if you don't want to rely on mcmeta specifically.
| ResourcePack.pack_format_registry.add_format(pack_format) | ||
|
|
||
| path = cache.download(opts.url_command_tree.format(version=ctx.minecraft_version)) | ||
| ctx.meta.setdefault("mecha", {}).setdefault("commands", []).insert(0, path) |
There was a problem hiding this comment.
hmm, i wonder if the best way for the contrib to enact the new tree is by setting it via the config. shouldn't it like just require a plugin or something? this feels odd to me.
There was a problem hiding this comment.
I was doing it that way, because if someone adds a custom command tree, i don't want to interfer with it and all instances of the Mecha class can use it by default.
I based this implementation on this example by Fizzy on discord : https://discord.com/channels/900530660677156924/1407893565601742909/1408164150336815184
Co-authored-by: rx <20506548+rx-dev@users.noreply.github.com>
Co-authored-by: rx <20506548+rx-dev@users.noreply.github.com>
5 participants
Theses PR implement the plugin that adds at require time a new minecraft version, it uses misode/mcmeta repo as a default source but it can be customized.
This PR also comes with a fix for
1.21.xsyntax handling in mecha, and adds the ability to resolve a minecraft version by calling a function to resolve a specific minecraft_version if necessary.