search

raphaelgoulart / ya_inputdisplay

Yet another input display for Clone Hero / YARG
16 stars 3 forks source link

Setting non-number values in the config file for scroll speed and width breaks (a lot) #15

Closed SkyJumper409x closed 2 months ago

SkyJumper409x commented 3 months ago

setting the width or scroll_speed to a non-numerical value prevents the input display from loading entirely, it is just uncolored, and static. In some instances, doing this specifically to the scroll_speed results in all input bars being crammed into the top left corner. Restarting the program can fix this, but sometimes just breaks the config further by more and more sections missing, requiring deletion of the config entirely.

raphaelgoulart commented 3 months ago

Hey! In your latest PR you mentioned "I am currently doing a lot of config modifications on a separate branch to fix a variety of issues relating to it, that will use config_version 2 because it changes a lot more" -- is it related to this?

If so I'll wait for that, then release an update with all of your changes once that arrives

SkyJumper409x commented 3 months ago

yes, there are way more issues. this is just the only one i haven't taken under inspection yet as i want your opinion on it :3.

Putting commas into the numbers breaks them apparently, this could be fixed by assuming the commas are for decimal values, as in some countries, the comma rather than the decimal point is used to represent that. Should this just be error corrected like that, or should the user be informed the config is broken, or should the value be silently ignored and the default is saved upon the next config save?

raphaelgoulart commented 2 months ago

Personally I'm more inclined towards "value be silently ignored and the default is saved upon the next config save" -- we can include in the README that fractional values should be input as, say, 1.5 rather than 1,5.

However, if you really want to go through the additional effort of supporting commas, feel free to. I'd like for the program to be as unobtrusive as possible, which is why I don't really like the warning idea. (Personally I wouldn't even support fractional values in the first place, but if you feel there's a genuine use for that, I won't oppose that).

BTW appreciate the work on cleaning up/fixing the program!

SkyJumper409x commented 2 months ago

i think fractional values only really make sense for scroll speed, as the width value is in pixels. (and well, we cant have fractional pixels, can we :3) Maybe a console warning or info message could be logged at least? So if someone runs the program through the console to figure out why their config isn't working, they'll see a message that tells them what the issue is.

I am glad i can help fixing the program, after all, i do it because the issues (mainly console errors) also affect me as an end user, not just as a programmer.

raphaelgoulart commented 2 months ago

Console warnings are fine yeah

SkyJumper409x commented 2 months ago

Well, turns out that some odd things happen when a variable has an invalid symbol in it. Now godot seems to just parse until it hits a letter, and then stops. So if i set scroll_speed to 40h0.0, it parsed until the h and then stops. All the program receives is 40, which makes it impossible to tell what is actually in the config. This did not happen previously and I have no clue why it is happening now. I think it might be a good idea to make the available keybinds for width and scroll speed changing more clear, so the config doesnt have to be edited to adjust those.

SkyJumper409x commented 2 months ago

[] and = are not single keys on some keyboard layouts (like mine). I would allow + in addition to =, aswell as adding , and . as keybinds for width adjusting.

SkyJumper409x commented 2 months ago

what do you think of adding a menu button ("hamburger button") in the top left corner like so: image All the configuration stuff could be put in a dialog which opens when the menu button is clicked. No need for hacky config editing or stepwise adjusting the scroll speed and width.

raphaelgoulart commented 2 months ago

I like it! I suggest making it togglable though, here's an idea: Have it visible by default, so the user knows where the hamburger button is. In the configuration dialog, add a setting to hide the button. The button should still work, just invisible, since the user will already know where the button is. And/or maybe make it also openable by hitting Esc

SkyJumper409x commented 2 months ago

That sounds like a good plan yeye :3