DeflatedPickle / Quiver

A resource pack creator and manager for Minecraft
https://www.planetminecraft.com/mod/programme-quiver---resource-pack-creatormanager/
MIT License
65 stars 9 forks source link

PackSquash integration #23

Closed AlexTMjugador closed 3 years ago

AlexTMjugador commented 4 years ago

I've just found this application and, as the author and current maintainer of PackSquash, the only open-source Minecraft resource pack optimizer I and its users are aware of, I think Quiver could benefit substantially from having PackSquash integration. Server side resource packs currently have a limit of 100 MiB, so staying below that threshold is important for server owners. Moreover, in any case map and resource pack makers can leverage the extra storage efficiency to add more content per MiB to their creations.

I'm interested in knowing whether this would be in scope for this project and, if so, whether you'd accept pull requests for this feature or otherwise just implement it yourself. Thanks! 🙂

DeflatedPickle commented 4 years ago

Thanks for bringing this to my attention! I haven't heard of it before (and wasn't aware there was a limit with server-side packs), but it sounds very useful!

I intend to add an 'export' option to zip up packs for distribution; it shouldn't be too taxing to add a checkbox to 'Use PackSquash'

My only worry is the integration with PackSquash. I haven't had to include native dependencies before, so I'll have to look into that

AlexTMjugador commented 4 years ago

I'm glad you found it useful and in scope for Quiver!

About the coding side of the integration, PackSquash is a command line application that accepts a settings file in its input stream, and some command line parameters. It also outputs messages to the standard output and error streams to let the user know progress and error information, and returns a non-zero exit code if something is very wrong. I imagine that, with the ProcessBuilder class that Java provides (and by extension I guess Kotlin too), it should indeed not be so difficult. Please feel free to let me know if I can do any changes to PackSquash that make it more amenable for the integration.

DeflatedPickle commented 4 years ago

Sorry, the problem isn't with how to use the CLI, it's how to properly package it for use in-dev and when built

AlexTMjugador commented 4 years ago

Oh, sure thing. Although the docs state you need to install the GStreamer libraries for the sake of user friendliness, all the GStreamer installer does that is relevant to PackSquash is copying some DLLs. PackSquash needs these DLLs on its directory or on a %PATH% directory to launch, and then some extra GStreamer plugins (which are also DLLs) to handle OGG, OGA, FLAC, WAV and MP3: DLL dependency list So just bundling the executable with the needed libraries along it is going to work and be proper packaging.

DeflatedPickle commented 4 years ago

That's not what I meant, sorry, but thank you for the explanation. It's integrating it into the Gradle build that I'm not sure about. It might be as simple as putting it in lib/natives then adding from files('lib/natives/*') to distributions.main.contents

AlexTMjugador commented 4 years ago

I've never used Gradle, but yeah, something along the lines of that should work: treating PackSquash files like library files that are copied over. The trickiest part is getting the exact list of DLL that PackSquash requires for working completely, because it may require some trial and error, but once that's done, updating it should be as easy as replacing the executable. I consider the configuration file format and command line switches to be pretty forward-compatibility friendly, too 😉

nm17 commented 4 years ago

Maybe it's more reasonable to add it as an external tool just like in idea? It would run a certain console command and make it available to call from the upper menu. Something like this:

idea64_K9lHHolWM3 idea64_L0RUhhzpbN

nm17 commented 4 years ago

Or making something like Tools -> PackSquash this texture pack and then make the user specify the path to it in Quiver settings.

DeflatedPickle commented 4 years ago

I didn't really think through my prior reply and messed up. The binary would be added to /tools/. But I realize the more major problem here is that it's a native binary, so I'd have to include each system's binary

Having the user specify the path would work! That's probably the route I'll go, then if someone knows how to bundle it, they can work on a PR. And since it wouldn't use resources, it could even be (and will be) made as a plugin

nm17 commented 4 years ago

I didn't really think through my prior reply and messed up. The binary would be added to /tools/. But I realize the more major problem here is that it's a native binary, so I'd have to include each system's binary

Having the user specify the path would work! That's probably the route I'll go, then if someone knows how to bundle it, they can work on a PR. And since it wouldn't use resources, it could even be (and will be) made as a plugin

I would like to help, but I really don't understand Kotlin that well and the plugin in Quiver. Possibly, If needed, I can make a python script that downloads binaries for release and puts it in some folder.

DeflatedPickle commented 4 years ago
  1. I don't do Kotlin justice, I write it like a less verbose Java
  2. The plugin system is fully documented... in the wiki for the project I wrote it for
  3. We unfortunately can't expect every user to have Python installed, but a Kotlin-based downloader can be written into the plugin, when I make it
AlexTMjugador commented 4 years ago

After reading your comments, I came across these thoughts on how to implement this in a way that would be convenient for users and feasible for developers:

The first point is, in my opinion, perhaps a bit easier to implement than the second, but the second has the advantage that it has the potential to basically interface with any other application, which is powerful. The third is a kind of compromise on the other two.

In any case, it seems that for a proper integration some kind of script that downloads PackSquash and what it needs would be at least part of the way to go. Is it useful if I draft a quick POSIX shell script or something similar to generate a package, so then it can be translated to Kotlin or whatever other code more easily? 🙂

DeflatedPickle commented 4 years ago
  1. I hadn't thought about downloading them during build-time, though I'd prefer to download the binaries for each system available, then check what the current operating system is before targeting one
  2. It possibly will be a plugin, as I mostly make this as a way to further my plugin framework development. If downloading PackSquash was done at run-time, I could add a config field to the plugin, for if it should be downloaded, and have a second field for the path to a local PackSquash

I don't think a shell script is needed. If it's downloaded at build-time, I will need to include that in the build.gradle, but if it's at run-time, it can be a function in the plugin

DeflatedPickle commented 3 years ago

Just a quick update on this - I thought of a system I could integrate PackSquash into and have written it and a PackSquash plugin, and it all works Hopefully, all that's needed now is;

The way it works, is when you have an open pack, you can now click on a handy "export pack" button to open a GUI, where you'll see each registered exporting step that was added to the registry - there are two types of steps; PerFileExportStep, which is run for each file extension in it's array and BulkExportStep which runs after the single file ones and is run on the whole pack. The PackSquash plugin was written with the BulkExportStep

So, the code for the PackSquash plugin ended up being 77 lines including import statements and package declarations and the config class. It could have been a little less but I decided for once to not fill my code with also/apply hell

AlexTMjugador commented 3 years ago

Great work, keep it up! 🙌

I've skimmed through https://github.com/DeflatedPickle/Quiver/commit/b4df891b7c069301133baa6a0bb076595b9e6107 and it looks very nice, although I have seen that you use the old command line switches to customize PackSquash behavior: https://github.com/DeflatedPickle/Quiver/blob/b4df891b7c069301133baa6a0bb076595b9e6107/packsquashstep/src/main/kotlin/com/deflatedpickle/quiver/packsquashstep/PackSquashStepSettings.kt#L16-L21 However, these switches were removed in v0.2.0 in favor of a settings file, which is intended to be much more flexible and manageable, and I do not intend to change this in future versions, so I will just add more configuration switches or things like that. You can generate a settings file on the fly via a OutputStreamWriter (make sure you use a constructor that receives a charset to specify a UTF-8 encoding, as it is required by the TOML spec) on top of the PackSquash process output stream. The full documentation for the settings file format is available here, but the examples at the end provide a TL;DR version of it.

Please get back in touch with any more news, questions or whatever. I'd love to mention in the README.md of PackSquash that you can use Quiver as a nice GUI for authoring resource packs and compressing them!

DeflatedPickle commented 3 years ago

Thank you! Great work with PackSquash! I tested it on a pack extracted from a version + the downloaded assets (such as sounds) whilst making sure the plugin was fine and it got down to a third of the size!

Yes, I've since decided against using these, but I originally wanted everything to be customisable with the Quiver config and therefor I was going to bundle PackSquash v0.1.2, but that would be silly for me to maintain any kind of wrapper config around an old version of PackSquash. I think I'll put the config into /data/packsquash/packsquash.toml, then read it with Quiver and feed it into PackSquash. Thanks for making that possible! I do think it would have been useful to still have been able to use a command-line flag to pass the pack location, and this is why I originally chose to use an older version (but I can just read it in at runtime then add the pack directory key myself)

I'll comment here again when if I have any questions and when I'm finished. I'll tick off the list when each item is finished

Thank you for wanting to mention Quiver in the PackSquash README! I'll make sure to add a credits window to specify it (and all of my dependencies) and mention it in the README and docs!

DeflatedPickle commented 3 years ago

@AlexTMjugador, I'm having a little trouble passing a config to the PackSquash executable. I would prefer to not have to write the resource_pack_directory to the file and instead read the config in myself, add the pack path and then write it into the stdin for PackSquash to use, however it always errors with this error field passed in

I don't know how versed you are with the JVM world, but my code is as follow;

ProcessBuilder(
    "packsquash",
    "-"
)
    .redirectInput(ProcessBuilder.Redirect.PIPE)
    .redirectOutput(ProcessBuilder.Redirect.INHERIT)
    .redirectError(ProcessBuilder.Redirect.INHERIT)
    .start().apply {
        try {
            if (this.isAlive) {
                outputStream.bufferedWriter().write(
                    "resource_pack_directory = \"/home/path/to/pack\""
                )
                outputStream.flush()
                outputStream.close()

                waitFor()
            }
        } catch (e: IOException) {
            e.printStackTrace()
        }
    }
AlexTMjugador commented 3 years ago

I don't know how versed you are with the JVM world

Don't worry about that! In addition to attending university classes about Java and reading lots of docs over time, I coded several Bukkit plugins and software projects in Java that used "advanced topics" such as concurrency, reflection, networking and remote procedure calls, so I don't find it difficult to read Kotlin even though I didn't write a line of Kotlin code ever 😉

My understanding of the situation is as follows:

With this in mind, I don't see anything that is obviously wrong with your code. It could be helpful to know the exact error message that PackSquash showed (i.e. the value of deserialize_error in PackSquash's source code), so we can know for sure what the settings file deserializer didn't like. I guess that maybe it expected a line ending after the key/value pair, or some other subtle thing like that.

Anyway, I have just coded a small toy application (somewhat of a proof of concept) in Java that implements the points I understood you need: it generates some config, passes it to a PackSquash process, lets it work and then finishes. You can check it out its code in this Gist to understand how it works and/or copy and paste some bits. I tried to write it in a way that handles errors properly, with lots of comments I think that may be useful, and it works on my end, so I hope it's useful for you!

DeflatedPickle commented 3 years ago

@AlexTMjugador, it seems the part that was missing was setting the working directory of the process to the pack directory. As soon as it's set to the pack's path, it works fine. Thank you for that!

AlexTMjugador commented 3 years ago

No problem, I'm glad it helped! 😀

DeflatedPickle commented 3 years ago

I'm considering this feature done, so there's a new release for it

I chose to have PackSquash be downloaded at build-time and bundled with each release, though if it grows in size or there are requests, I can look into a system to download tools at run-time I've written and bundled a config for PackSquash that people can use instead of having to write their own immediately to use it

If there are any questions or complaints about how I've handled this, please leave a comment

AlexTMjugador commented 3 years ago

Great work! I've ran Quiver and the PackSquash export step on my Debian PC and it worked flawlessly (yeah, I use Debian too). Even though it is a fairly simple thing, for some reason I loved the export step dialogs, with the progress bar and its status updates. I've just added a paragraph in the PackSquash README.md pointing to Quiver, as I've said in a comment.