mt-mods / signs_lib

Other
5 stars 12 forks source link

Off/On button in sign formspec is not translatable #20

Open Wuzzy2 opened 1 year ago

Wuzzy2 commented 1 year ago

There are 2 on/off buttons in the sign formspec, one for wide font, the other for Unicode font.

The problem is, these buttons have the text "OFF" and "ON" baked into the images, making them untranslatable.

I would like it if there buttons would be replaced with something that does not have text baked into the image.

wsor4035 commented 1 year ago

if im thinking of the proper toggle, its also used in pipeworks, etc and is a massive hack in terms of implementation

S-S-X commented 1 year ago

Maybe tooltip could be used for accessibility and on/off text changed to some symbol.

Wuzzy2 commented 1 year ago

You could also just use good old checkboxes.

Anyway, it's not really important for me.

OgelGames commented 1 year ago

if im thinking of the proper toggle, its also used in pipeworks, etc and is a massive hack in terms of implementation

Yeah, that's the one, and yeah, it's horrible :P

Maybe tooltip could be used for accessibility and on/off text changed to some symbol.

This is probably the cleanest solution, not requiring much code or texture changes 👍

sirrobzeroone commented 1 year ago

For Images like the below? went 32x32 for a more pixeled look - Standard - Red/Green

Or if you want to keep Colorblind people in the loop blue/red works well for 3 types and not so strange for non-colorblind CB3 - Red/Blue.

licence if anyone wants to use cc0

on_cc0 off_cc0 oncb3_cc0

sirrobzeroone commented 1 year ago

Blue/Red & Green/Red - tool tip might need a color adjust using the default colors in the below for TT? maybe lighter blue for red/blue if it was to go cb friendly?

sign_lib_rb sign_lib_rg

SwissalpS commented 1 year ago

what's wrong with good old checkboxes?

Edit: I mean, they are standardized, players know what they look like and how they work, why introduce yet another element the user needs to learn about?

sirrobzeroone commented 1 year ago

Nothing at all, couple of info points on native FS checkbox's though:

Might be better to fake the CB's as images and leave the code as is, which mitigates the two points above - image 2.

Real formspec CB's sign_lib_fsnative_cb

Fake Image CB's sign_lib_fsfake_cb

EDIT This was bugging me on asthetic level so I inset the CB's and labels about the same amount as the "write" button in relation to the image above it: Fake Image CB's more aligned sign_lib_fsfake_cb_align

SwissalpS commented 1 year ago

OK, tiny checkboxes on huge screens aren't easy to use. So why not issue a change in core so all checkboxes get 'fixed'?

sirrobzeroone commented 1 year ago

It can be added as an issue, but it is beyond my abilities to fix in source C which is where Im guessing the fix would be needed? Given most users are Im guessing still using HD vs UHD I dont think it would get a quick fix label but might get addressed as the coredevs work through those issues.

Guess it comes down to how fast and bad is this issue for sign_lib if its not a big deal probably just need to cross link the above issue and note it here as wont be fixed until Engine bug is addressed. (I'll go have a skim and see if its been reported yet)

Im more of address what you can in your own mod kind of guy but understand the other approach as well as it lowers number of changes for mods which for low grade bugs/issues is a better way to go, particularly were resources are limited.

Anyways the image CB fix is trivial (full working on my machine) the formspec fix is less trivial assuming no duplication of old formspec fields needed I have that about 90% done. if it's easier I can post both sets of code here + in the case of the first fake cb's images as well and then a decscion can be made about which way to go?

Just let me know in a reply here :)

Edit: Couldnt find anythign specific against MT so raised an issue: https://github.com/minetest/minetest/issues/13665

sirrobzeroone commented 1 year ago

Fix One Fake Image checkboxs, replace api.lua lines 1341 to 1357 inclusively and update image files, no interim image needed anymore: signs_lib_switch_off signs_lib_switch_on

local formspec = {
        "size[6,4]",
        "background[-0.5,-0.5;7,5;signs_lib_sign_bg.png]",
        "image[0.1,2.4;7,1;signs_lib_sign_color_palette.png]",
        "textarea[0.15,-0.2;6.3,2.8;text;;" .. minetest.formspec_escape(txt) .. "]",
        "button_exit[3.7,3.4;2,1;ok;" .. S("Write") .. "]",
        "label[0.65,3.3;"..FS("Unicode font").."]",
        "image_button[0.3,3.4;0.45,0.4;signs_lib_switch_" .. state .. ".png;uni_"
            .. state .. ";;;false;]",
    }

    if minetest.registered_nodes[nodename].allow_widefont then
        state = meta:get_int("widefont") == 1 and "on" or "off"
        formspec[#formspec+1] = "label[0.65,3.8;"..FS("Wide font").."]"
        formspec[#formspec+1] = "image_button[0.3,3.9;0.45,0.4;signs_lib_switch_" .. state .. ".png;wide_"
                .. state .. ";;;false;]"
    end

Fix Two No images needed at all. Replace api.lua lines 1335 to 1371 inclusively

This is a bit untidy but without fully updating code to line 1401 does work - I can do that as well just was trying for a minimal change version using formspec checkboxes.

function get_sign_formspec(pos, nodename)

    local meta = minetest.get_meta(pos)
    local txt = meta:get_string("text")
    local state = meta:get_int("unifont") == 1 and "true" or "false"
    local formspec = {
        "size[6,4]",
        "background[-0.5,-0.5;7,5;signs_lib_sign_bg.png]",
        "image[0.1,2.4;7,1;signs_lib_sign_color_palette.png]",
        "textarea[0.15,-0.2;6.3,2.8;text;;" .. minetest.formspec_escape(txt) .. "]",
        "checkbox[0.3,3.2;unicode_font;".. FS("Unicode font") ..";".. state .."]",
        "button_exit[3.7,3.4;2,1;ok;" .. S("Write") .. "]",

    }

    if minetest.registered_nodes[nodename].allow_widefont then
        state = meta:get_int("widefont") == 1 and "true" or "false"
        formspec[#formspec+1] = "checkbox[0.3,3.6;wide_font;".. FS("Wide font") ..";".. state .."]"
    end

    return table.concat(formspec, "")
end

minetest.register_on_player_receive_fields(function(player, formname, fields)

    if formname ~= "signs_lib:sign" then return end

    -- This is reversed to how I think but retains current behaviour
    -- and minimises code changes.
    if fields.unicode_font == "true" then fields.uni_off = ""
    elseif fields.unicode_font == "false" then fields.uni_on = ""
    end

    if fields.wide_font == "true" then fields.wide_off = ""
    elseif fields.wide_font == "false" then fields.wide_on = ""
    end

I did not test to see what fix two would do to an existing sign, In theory it should do nothing as meta is still stored the same way just used slightly differently. Either fix needs more testing as this is very much untested in regards to existing signs I just didnt wish to do a ton of testing until a decscion on sol'n is made :)

SwissalpS commented 1 year ago

(I don't even have HD)

SwissalpS commented 1 year ago

Didn't take long. Please test with a build after this commit: https://github.com/minetest/minetest/pull/13666#event-9841412801

sirrobzeroone commented 1 year ago

Yes Smalljoker picked it up which was great :)

Edit: had a hunt around and cant find a build that includes the fix yet, so ill wait a couple of days then give it another try.

tenplus1 commented 6 months ago

@sirrobzeroone - I've added your checkboxes to my own fork of signs_lib if you wanna test it.

https://codeberg.org/tenplus1/signs_lib

tenplus1 commented 6 months ago

@Niklp09 & @wsor4035 - Don't worry, it's my personal fork that doesn't use reverse masking and tinkers with features that may not make it into the official signs_lib.

Niklp09 commented 6 months ago

But it would make sense to get as much features/fixes upstream as possible.

tenplus1 commented 6 months ago

@Niklp09 - Of course! but it doesn't hurt to have a test edition to try out features on first :)