[macOS] Fixed Export to Clipboard and FFmpeg detection.

[qwerasd205]

Addresses #46 and #54.

[qwerasd205]

Related to this PR, you might consider adding a config/options screen, and providing options for "always copy to clipboard after export" and a place to enter an explicit ffmpeg path if it still couldn't be found automatically.

[qwerasd205]

I've begun cleaning up and refactoring/reworking my code to make it more robust and maintainable.

[qwerasd205]

Alright, that should be more robust, and also gets things going in the right direction for the config stuff.

Needs Windows/Linux testing

I haven't tested this code on anything but macOS- and since I touched the way non-macOS stuff is handled in some places, it needs testing on Windows and Linux before accepting these changes. LWJGL can be very tempermental with FS related stuff; while working on this I encountered a very non-obvious segfault on macOS, so even though it doesn't seem like anything I changed should cause any problems or different behaviors, it's better safe than sorry- you should probably check to be sure.

[gliscowo]

Hey there,

I'd like to begin this by saying that I really appreciate you taking the time and effort to make a clean implementation for these features. It's nice to see you care about the mod

I'm afraid, however, that I need to express my discomfort with the general goal being chased here. There are two primary reasons for this:

• I find the prospect of having 60 lines of macOS-specific code for copying things to the clipboard quite unsettling, given that there's no saying it will keep working and it's trying to support a quite superficial convenience feature that is in no way essential to the mod's functionality. Nobody on our team has access to an Apple system to test and keep maintaining this on, and generally the userbase with this platform is very small - making it simply not worth the maintenance effort in my opinion

  • There should be no need for having 200 lines of code purely dedicated to finding the FFmpeg binary. Maybe I'm suffering from skewed expectations here, but I strongly believe that any user capable of installing a mod is also capable of either adding the binary's location to their PATH or, even easier and more similar to installing the mod, copying the binary to their game's installation directory. I'd much rather add some paragraphs to the documentation on how to do this than maintain a whole class for the 1% of users who require additional guidance before they get it working

I'm interested to hear your opinion on these issues Cheers

[qwerasd205]

I'd like to begin this by saying that I really appreciate you taking the time and effort to make a clean implementation for these features. It's nice to see you care about the mod

Well, as someone who uses the mod regularly, I have an interest in making it better, for myself if no one else. I'd like to thank you for making such a great mod in the first place.

To address your concerns:

• I find the prospect of having 60 lines of macOS-specific code for copying things to the clipboard quite unsettling

  It's more like about 15 lines of macOS specific code, with 40 lines dedicated to the inline copy of the license, and a little boilerplate.

• given that there's no saying it will keep working and it's trying to support a quite superficial convenience feature that is in no way essential to the mod's functionality.

  I feel that it's not really that superficial of a feature, it's excellent ergonomically for when you want to take an isorender to share quickly without cluttering your filesystem or having to painstakingly go to the folder to copy it. It also provides an interface which would make implementing additional ergonomic features trivial as well (e.g. button in export notification to copy exported file to clipboard).

• Nobody on our team has access to an Apple system to test and keep maintaining this on, and generally the userbase with this platform is very small - making it simply not worth the maintenance effort in my opinion

  Since the macOS-specific code comes from another mod, if a breaking change occurs, chances are it will be updated there, and can be re-sourced from there for whatever updates are needed. Additionally, as long as I remain a user of the mod, since I use macOS I will be maintaining this feature for myself, so would be perfectly happy to accept the maintanence burden for macOS compatibility for this mod - which simply amounts to making a PR whenever I need to fix a breaking change, which I'll be doing regardless. Note: I honestly don't expect this code to break, at least not due to anything on macOS's side, any time soon- it's based on making calls to Objective C APIs, which are extremely stable and are the types of things that Apple keeps backwards compatibility for for quite a long time. And finally, if it ever does break and I'm MIA, and macOS-compatible Screenshot to Clipboard mods are gone from the face of the earth, all you have to do is remove the offending code and disable the feature on macOS once more.

If you do end up deciding that it's still not worth it to do this, I understand- and I'll either just make a fork of the mod (which I'd like to avoid if possible since it feels a little disrespectful), or a stand-alone patcher/compatibility mod (which I'd also like to avoid since the code would be... less than ideal for such a thing).

As for your second item:

• There should be no need for having 200 lines of code purely dedicated to finding the FFmpeg binary.

  This one you are probably right about 😅 - I suppose I did go a teensy bit overboard in my implementation of the FFmpeg resolver. If you think such a robust user-proofed solution is unnecessary, I'd be more than happy to scale it back to something more lightweight. Though, a good ~20% of that code does come from the lines removed from FFmpegDispatcher to decouple the command resolution from the actual job dispatching.

• Maybe I'm suffering from skewed expectations here, but I strongly believe that any user capable of installing a mod is also capable of either adding the binary's location to their PATH or, even easier and more similar to installing the mod, copying the binary to their game's installation directory.

  Launchers these days let you install mods without even knowing what a directory is- I absolutely think it's never possible to underestimate how incapable users can get. That aside- the two things you mentioned both do not work on macOS; on macOS, MultiMC-based launchers (i.e. MultiMC, PolyMC, Prism Launcher) launch the game with an incomplete PATH, since they make no attempt to resolve paths added in /etc/profile, ~/.profile or anywhere else, or through use of /usr/libexec/path_helper. Instead, they launch the game with the bare-bones /usr/bin:/bin:/usr/sbin:/sbin- which is notably missing /usr/local/bin, by far the most common (and most correct) install location for FFmpeg on macOS (especially when installed with a package manager like homebrew or macports). Additionally, macOS does not resolve ffmpeg to ./ffmpeg, meaning placing the binary in the working directory does not work. Fixing this bug is exactly where I started with wanting to submit a patch - which is probably why I went a little overboard with the implementation. I decided to also do the clipboard while I was at it since it's fairly straightforward.

• I'd much rather add some paragraphs to the documentation on how to do this than maintain a whole class for the 1% of users who require additional guidance before they get it working

  This is fair- though if you do want to go with a bare-bones "move the files around on your own" approach, I'd at least adjust the message for missing FFmpeg in-game to link to the documentation (unless it already does and I'm blind). Do note, I tried to make the code as maintainable as possible; it's written to be generic, avoiding tight coupling and tries to provide a transparent interface so that it's the only part of the code worried about the implementation details (which are unlikely to change, since it's simple FS and process interaction).

If having more classes gives you an uncomfortable feeling, one option that could be explored (I'd be more than happy to take a shot at writing it) is to go back to a unified class for both resolving the FFmpeg command and running the FFmpeg jobs (though I personally find this sort of coupling to be more uncomfortable than just having the additional file), but just rework it to be more generic. A unified, generified FFmpeg handler class would also make implementing configurable FFmpeg settings (as I suggested in #56) much more straightforward.

Ultimately (obviously) it's your choice- but I think using decoupled generic interfaces like this is over-all a positive, and would help make implementing new features and changes to existing behaviour over-all simpler. I hope to contribute several QOL changes and improvements to this mod (the details of which I won't get in to here, we can talk about my ideas on the Discord if you're curious) and I'm willing to put in a considerable amount of developer time to do so, but if your vision for this mod is more of an ultra-lightweight "just enough to work" arrangement I may be better off making a fork or stand-alone tweak mod.

I hope I assuaged your concerns, though, with regards to most of it, and if so then you can get back to me about whether to trim down the FFmpegResolver and/or try my hand at a single unified FFmpeg handler class.

Thanks once again for making this mod, and thanks for the response ^-^ I hope my answers weren't too long and rambly 😅

[qwerasd205]

Never mind I guess