yurisuika / Raised

Raises the hotbar so the selector is not cut off!
GNU Lesser General Public License v3.0
17 stars 5 forks source link

Texture Support Option Very Poorly Described #84

Closed Obscure2020 closed 2 months ago

Obscure2020 commented 5 months ago

I take issue with several aspects of this button and its description text. My reasons are below.

2024-03-13_01 05 49

1. The text is wrong.

The tooltip should not instruct users to set this option to false when that is not an option available on the button. Clicking this button can make it say On or Off. Don't instruct users to make the button say false when they can't, because false is not an option. The button does not say true or false. It says On or Off.

2. The text is confusing.

What exactly does Enables texture support mean?

Yes, the tooltip does say that it "fixes the missing row of pixels with a new asset," but that explanation is a little brief and developer-y. Not everyone is going to understand what it's trying to say. It feels like this was written in a double-negative manner. It should have said "fixes the missing row of pixels with row mirroring" or instead maybe it could have said "enables custom Raised resource pack support." But instead, you went with a confusing mashup of the two: "fixes the missing row of pixels... with a new asset."

3. The options are insufficient.

There should honestly be three options that this button would toggle between. Let me lay out the three options below, in the manner that I think actually makes some friggin' sense.

  1. The option On should activate the row mirroring strategy. When the button says On, Raised should be taking the currently loaded (possibly overwritten by a resource pack) default hotbar selector texture and using the row mirroring on that to produce the full-square version that Raised will display.
  2. The option Off should cause Raised to check if any resource packs are providing a texture at assets/raised/textures/gui/sprites/hud/hotbar_selection.png. If a custom texture has been provided at this path by a resource pack, then that texture should be used by Raised. If no such texture has been provided by any of the user's resource packs, then Raised should fall-back to the 24x24 version shipped inside of Raised.
  3. The option Auto should act like a hybrid of the previous two. When the button says Auto, the game should check if any resource packs are providing a texture at assets/raised/textures/gui/sprites/hud/hotbar_selection.png. If a custom texture has been provided at this path by a resource pack, then that texture should be used by Raised. If no such texture has been provided by any of the user's resource packs, then Raised should be taking the currently loaded default hotbar selector texture and using the row mirroring on that to produce the full-square version that Raised will display.

Once implemented, Auto should be the default option. The tooltip for this button should also be re-written to reflect this behavior. It's up to you to write a new tooltip if you implement my proposed third option, but I do think you should at least follow these two rules:

  1. The description for the option On should contain the words "Fixes the missing row of pixels."
  2. The description for the option Off should not contain the word "Fixes." It should not contain the word "asset." It should contain the words "custom texture" and also possibly "resource pack."
yurisuika commented 3 months ago

I don't think it makes sense to use the terms ON and OFF because those are just what the Minecraft buttons translate the boolean values to, which are what the user will find in the config. The same is why the button names are the exact same as the config keys. Just look at the game rules tooltips, same thing.

The automatic search for an asset is an interesting idea, but it is not an end all to deciding what a user may want, as some users now desire that hotbar selectors are unmodified because newer resource packs are being designed around only having 23 pixels in height, and thus do not need either method of modifying the hotbar selection asset. But it doesn't matter too much because implementing it doesn't seem to work... For an auto option it would be essential to ignore the internal resource pack, otherwise it would always succeed, but trying to filter out the packs to just ones with id's not containing "raised" or some similar filter makes it not find anything and spams endless errors when ran in situ.

MinecraftClient.getInstance().getResourcePackManager().createResourcePacks().forEach(pack -> {
    if (!pack.getId().contains("raised")) {
        pack.findResources(ResourceType.CLIENT_RESOURCES, "raised", "textures/gui/sprites/hud/", (id, supplier) -> {
            if (id.equals(new Identifier("raised:textures/gui/sprites/hud/hotbar_selection.png"))) {
                args.set(0, new Identifier("raised:hud/hotbar_selection"));
                args.set(4, 24);
            }
        });
    }
});

Beats me cause that should work. And if we do the opposite and run only searching for packs w/ "raised" i.e. the internal one, it runs to the defeat of the purpose.

yurisuika commented 2 months ago

Good news! I took another crack at this and resolved the issue. This will be part of the upcoming release.