Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Readd support for comma decimal separator in editor #24532

Closed

Conversation

akien-mga
Copy link
Member

Fixes #24531, re-implements #6028 in new property editor.

@bojidar-bg
Copy link
Contributor

So, how would you do pow(2, 3) now? 😃

@akien-mga
Copy link
Member Author

So, how would you do pow(2, 3) now? smiley

Hm, didn't think about that. Should we make it optional, or just drop support for using comma as decimal separator in EditorSpinSlider?

@vinterskog
Copy link

Would be really disappointing not to be able to use comma in decimal values in GUI widgets anymore to be honest. There's parts of the world where that's just how it's done, you know? ;)
In fact, I did notice the regression immediately and found it instantly annoying.

@thomasruiz
Copy link
Contributor

thomasruiz commented Dec 22, 2018

@akien-mga You need to change 2 things around here instead, I think, though I don't know if that doesn't have a side effect somewhere.

@bojidar-bg
Copy link
Contributor

Well, it won't work with pow(2,3) then (as it will parse as one number), but I'm mostly fine with this.

@eon-s
Copy link
Contributor

eon-s commented Dec 27, 2018

I prefer this optional, even if my local system uses comma, using dot is less confusing, is consistent with programming languages and allows to use things like the expressions mentioned before.

@thomasruiz
Copy link
Contributor

It's probably a bad idea anyway, the pow problem that @bojidar-bg mentioned would make things tricky. It's a safer bet to allow only one decimal character, for consistency.

@akien-mga
Copy link
Member Author

Given that it would break using commas for builtin methods like pow, I think it's better not to merge this.

IINM, the only reason why some users may really want this is because some keyboard layouts have a "comma" key in place of the "dot" key in the numpad (KEY_KP_PERIOD in Godot). I know at least German and Danish keyboard layouts do that. English US and French have a dot key in the numpad, like most other keyboard layouts I suppose (even though French uses comma as decimal separator).

So maybe instead of replacing commas by dots in the input text, we could catch KEY_KP_PERIOD and force it to input a dot in the evaluator widget? (I just checked with setxkbmap de and pressing the "comma" key on the numpad does trigger a KEY_KP_PERIOD event.)

@vinterskog
Copy link

Sounds like a good compromise. And yes, the numpad really is the issue here (for me at least).

@akien-mga akien-mga changed the title Readd support for comma decimal separator in editor WIP: Readd support for comma decimal separator in editor Jan 4, 2019
@akien-mga
Copy link
Member Author

I just checked with setxkbmap de and pressing the "comma" key on the numpad does trigger a KEY_KP_PERIOD event.

Actually I was wrong, it currently reports "0" as scancode on X11. It's actually a XK_KP_Separator event which we currently ignore: https://github.com/godotengine/godot/blob/master/platform/x11/key_mapping_x11.cpp#L95
I checked Windows and macOS and we also ignore the similar event there.

So supporting detection of a keypress of this numpad comma will require some more work. I had a quick go at it, but it's too late for 3.1 so I'll look at it again later.

@kondelik
Copy link

kondelik commented Jan 11, 2019

IINM, the only reason why some users may really want this is because some keyboard layouts have a "comma" key in place of the "dot" key in the numpad (KEY_KP_PERIOD in Godot). I know at least German and Danish keyboard layouts do that. English US and French have a dot key in the numpad, like most other keyboard layouts I suppose (even though French uses comma as decimal separator).

France have comma too: https://brilliantmaps.com/decimals/

What about something simple like if not textbox contains non-digit character with exception of dot/comma, then replace else nothing?

or even better try to replace and check if its float? (i dont do c++, but pseudo code isif(float.tryparse(input.replace)) then replace else nothing)

@akien-mga
Copy link
Member Author

France have comma too: https://brilliantmaps.com/decimals/

Not on numpad. http://www.ibt.ca/v2/items/btc5121w/images/azerty_1.jpg
I have a French Azerty keyboard and can confirm that. Actually the actual non-numpad dot is so hard to reach (Shift+;) on French Azerty keyboard that many users use the numpad one for their punctuation needs.

What about something simple like if not textbox contains non-digit character with exception of dot/comma, then replace else nothing?

That would start becoming quite involved if we need to parse the input strings all the time to check that, and that's also pretty random from an end user perspective.

or even better try to replace and check if its float? (i dont do c++, but pseudo code is if(float.tryparse(input.replace)) then replace else nothing)

That would break on pow(2,3).

@kondelik
Copy link

kondelik commented Jan 11, 2019

Umm... Never heard about azerty, but It should not really matter what is printed on keyboard (i have dot on mine, too), what matter is your setting in system (i write comma every time i hit that numpad button, look: ",,," 🤣 )

I can go to Windows Setting (https://resrequest.helpspot.com/index.php?pg=kb.page&id=279) and change what is printed after i hit "dot on numpad". I really dont want to do that only for godot, becouse it break every other app i am using.

or even better try to replace and check if its float? (i dont do c++, but pseudo code is if(float.tryparse(input.replace)) then replace else nothing)

That would break on pow(2,3).

Actually thats the point 😄 I dont know c++, but in c# i would write it as

// this is NOT c++ code!!!! Dont copy paste it in godot!!!
TryReplace("pow(2,3)"); // return "pow(2,3)"
TryReplace("2,3"); // returns "2.3"
TryReplace("2.3");  // returns "2.3" too

// this is NOT c++ code!!!! Dont copy paste it in godot!!!
public string TryReplace(string input)
{
  float tmp;
  if(float.TryParse(input.Replace(",", "."), out tmp)) // TRY Replace - maybe prg language specific
  {
    return input.Replace(",", ".");
  }
  return input;
} 

@kondelik
Copy link

kondelik commented Jan 11, 2019

Let me say it this way:

Doesnt matter what keyboard layout you have (qwertz, qwerty, azerty, chinese) - what matter is you CurrentCulture windows setting.

We can swap your fancy keyboard for my cheap and dirty one and your keyboard will write comma on my desktop and my keyboard will write dot on your desktop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants