Fabulously-Optimized / fabulously-optimized

A simple Minecraft modpack focusing on performance and graphics enhancements.
https://download.fo
BSD 3-Clause "New" or "Revised" License
925 stars 82 forks source link

ScreenshotSettings #535

Closed fmbellomy closed 1 year ago

fmbellomy commented 1 year ago

CurseForge link

No response

CurseForge Mod Distribution

None

Modrinth link

https://modrinth.com/mod/screenshotsettings

Source/other link

https://github.com/fmbellomy/ScreenshotSettings

Mod file size

486.7 KiB

License

(any other license)

What it does

Adds support for setting a custom screenshots folder, and attaching in-game information like coordinates to the screenshot's PNG metadata.

Also supports custom file-naming schemes, which allows players to automatically create subdirectories to organize screenshots by what shaderpack was in use, or what world the screenshot was taken on.

If you want to know more, I'd honestly just recommend looking at the description on Modrinth or GitHub.

Why should it be in the modpack

It adds somewhat niche, but useful ways to further customize the game.

All features are disabled by default, so the mod actually does nothing without the user/player explicitly choosing for it to do so. I'd say in that regard it's about as "vanilla-like" as you can get.

Why shouldn't it be in the modpack

The mod breaks if you ship a config containing paths that don't exist on a player's machine as a modpack developer. For example, you set your screenshots directory to a Windows path like C:/Users/fmbellomy/Pictures/MCScreenshots and then somehow someone ends up using your config on linux or MacOS, where that path would fail.

Besides that, the mod is dependent on YACL and Cloth Config and an optional integration with Iris, but all of those are already in Fabulously Optimized, so they shouldn't be major concerns.

Additional details

No response

Madis0 commented 1 year ago

Thank you for the suggestion! I'm afraid I'm gonna have to say no though.

While it was rejected from this pack, I hope you consider these points as constructive criticism to improve your mod. Good luck!

fmbellomy commented 1 year ago

I have a few disagreements with your reasoning for rejecting the mod and hope that you'll hear me out. I don't expect this to change your mind about inclusion, because your points about the mod not being on Curseforge and not having a system for handling external configs are valid, but those can be taken care of and maybe you'd consider the mod for inclusion more seriously by that point. I do genuinely believe this mod could fit in with the other mods in Fabulously Optimized as a QOL addition.

The rest of the points I'll address here.

It is a mod that focuses on providing settings... but modpacks cannot include theirs. That's a big red flag indicating this mod is not ready yet. Especially because by default it does nothing either.

I do think that the mod needs a better way to handle possibly broken configs from external sources, but in my opinion it would optimally entirely reject any external config and restore the default of using no features anyway. I imagine you wouldn't be a huge fan of that, since your responsibility as the maintainer of this pack is to provide "sane defaults" to people, but my responsibility as the mod dev is to make sure that my mod can't possibly harm users (like by leaking their coordinates to griefers) through the inclusion of a config in a modpack.

I firmly believe this mod should do nothing without the user's explicit knowledge and consent, but I do not think that makes this mod useless. It exists to provide settings to the individual player, and not to the modpack dev.

Embedding coordinates and other sensitive metadata to the image only calls for griefers and people with malicious intent to act upon it.

If you don't wish to include a mod that gives players the option of leaking their coordinates, then fair enough. However, I personally don't see a problem with it because:

  1. They have to enable the feature themselves.
  2. There are already a million other ways to accidentally leak your coordinates without the help of a mod, and I highly doubt griefers are going to start looking through PNG metadata to find information that will only exist if the creator enabled a niche setting in a niche mod.

If it would help you feel better about this feature existing, I could add a tooltip for that setting that warns users of the possibility of leaking their coordinates to others who see the screenshot, but I still think that ultimately it's the user's responsibility to know whether or not they're comfortable with sharing that information.

If it's a local world, the player probably knows its name. If it's a server, there is a chance something about the environment or bossbar/scoreboard gives away, which server that is.

The ability to include the world or server name in the filename or as the subdirectory name is meant to make finding those screenshots easier, not to make recognizing them easier.

If they need coordinates, they'd screenshot F3 or enable the overlay first to capture it in the screenshot.

This is true, but embedding coordinates into a screenshot isn't just so you can remember where some location is.

The feature is meant to make the screenshot reproducible, by providing the position, yaw, and pitch, which would allow someone to /tp themselves to the exact position and angle as a screenshot. This is also why its possible to embed the world seed, game version, shader packs, and resource packs into the metadata. These features are intended as ways to make screenshots reproducible without the F3 overlay cluttering the screen.

I can't really think of who would actually care about the features this currently provides.

First of all, ouch. Secondly, maybe only I do, but I find it hard to believe I'm the only person who frequently gets annoyed with how screenshots behave in vanilla Minecraft.

If I uploaded the mod to Curseforge, implemented a system that rejects modified configs from external sources, and provided a warning on how embedded metadata could potentially open you up to griefing in multiplayer, would you still feel the same way about it?

Julienraptor01 commented 1 year ago

I have a few disagreements with your reasoning for rejecting the mod and hope that you'll hear me out. I don't expect this to change your mind about inclusion, because your points about the mod not being on Curseforge and not having a system for handling external configs are valid, but those can be taken care of and maybe you'd consider the mod for inclusion more seriously by that point. I do genuinely believe this mod could fit in with the other mods in Fabulously Optimized as a QOL addition.

The rest of the points I'll address here.

It is a mod that focuses on providing settings... but modpacks cannot include theirs. That's a big red flag indicating this mod is not ready yet. Especially because by default it does nothing either.

I do think that the mod needs a better way to handle possibly broken configs from external sources, but in my opinion it would optimally entirely reject any external config and restore the default of using no features anyway. I imagine you wouldn't be a huge fan of that, since your responsibility as the maintainer of this pack is to provide "sane defaults" to people, but my responsibility as the mod dev is to make sure that my mod can't possibly harm users (like by leaking their coordinates to griefers) through the inclusion of a config in a modpack.

* There is no sane default for a custom screenshots directory, because you can't pick a directory outside of the instance and guarantee that it will exist on the player's machine.

* Embedding potentially sensitive metadata into users' screenshots without their knowledge or consent is a substantial no-no, so that obviously should be disabled unless the user decides it's something they want to use.

* Automatically creating a hierarchical structure for screenshots will confuse players who did not enable it, and create an inconvenience for people who barely take any screenshots in the first place.

I firmly believe this mod should do nothing without the user's explicit knowledge and consent, but I do not think that makes this mod useless. It exists to provide settings to the individual player, and not to the modpack dev.

If it should not do anything whitout user intervention then there is no reason for it to be in the modpack. For people who don't want to use it, it will be bloat and for people who want to use it, well they can install it by themselves. FO is made to get changes from the player if they find it needed.

(EDIT: yes, AntiGhost, Borderless Mining and Midnight Controls do require user intervention but Madis pointed the reason)

Embedding coordinates and other sensitive metadata to the image only calls for griefers and people with malicious intent to act upon it.

If you don't wish to include a mod that gives players the option of leaking their coordinates, then fair enough. However, I personally don't see a problem with it because:

1. They have to enable the feature themselves.

2. There are already a million other ways to accidentally leak your coordinates without the help of a mod, and I highly doubt griefers are going to start looking through PNG metadata to find information that will only exist if the creator enabled a niche setting in a niche mod.

If it would help you feel better about this feature existing, I could add a tooltip for that setting that warns users of the possibility of leaking their coordinates to others who see the screenshot, but I still think that ultimately it's the user's responsibility to know whether or not they're comfortable with sharing that information.

If it's a local world, the player probably knows its name. If it's a server, there is a chance something about the environment or bossbar/scoreboard gives away, which server that is.

The ability to include the world or server name in the filename or as the subdirectory name is meant to make finding those screenshots easier, not to make recognizing them easier.

If they need coordinates, they'd screenshot F3 or enable the overlay first to capture it in the screenshot.

This is true, but embedding coordinates into a screenshot isn't just so you can remember where some location is.

The feature is meant to make the screenshot reproducible, by providing the position, yaw, and pitch, which would allow someone to /tp themselves to the exact position and angle as a screenshot. This is also why its possible to embed the world seed, game version, shader packs, and resource packs into the metadata. These features are intended as ways to make screenshots reproducible without the F3 overlay cluttering the screen.

That's why we recommend to users to use sodium extra show coordinates features instead of the F3, since the F3 is made to debug and is cluttered for a user + it is very performance heavy.

I can't really think of who would actually care about the features this currently provides.

First of all, ouch. Secondly, maybe only I do, but I find it hard to believe I'm the only person who frequently gets annoyed with how screenshots behave in vanilla Minecraft.

If I uploaded the mod to Curseforge, implemented a system that rejects modified configs from external sources, and provided a warning on how embedded metadata could potentially open you up to griefing in multiplayer, would you still feel the same way about it?

Again, what's the point of it being in a modpack which is basically ready to use for everything else if the player need to do some manual configuration ?

Madis0 commented 1 year ago

Alright, let me give you some more context. Fabulously Optimized currently has three opt-in mods:

All of them have the following in common:

Compare that to your mod, which

First of all, ouch. Secondly, maybe only I do, but I find it hard to believe I'm the only person who frequently gets annoyed with how screenshots behave in vanilla Minecraft.

Sorry, if that was harsh, but who is the target audience of your mod besides yourself? Have you seen posts or comments of people looking for this? Do those features exist in any popular client or mod (which would explain the demand/familiarity at least)? Can you explain the mod's goals and setup to both 7-year old and 70-year old?

Maybe I could consider your mod if it was able to replace Fabrishot for example or contained something more simply explainable such as file size compression.¹ But for now, 🤷

¹ P.S. Don't add that or similar feature just for the sake of it. I'm talking about a general idea of focusing the/a mod to something that more people can understand and are looking for.