natinusala / borealis

Hardware accelerated, controller and TV oriented UI library for PC and Nintendo Switch (libnx)
Apache License 2.0
260 stars 83 forks source link

Add enum kbdDisableBitmask to manage keyboard imput (#72) #73

Closed H0neyBadger closed 3 years ago

H0neyBadger commented 3 years ago

I'm keeping the PR in draft state to test the code correctly. let me know if I have to change anything. Thank you a lot

H0neyBadger commented 3 years ago

Hello, regarding the default value :

SwkbdKeyDisableBitmask_At | SwkbdKeyDisableBitmask_Percent | SwkbdKeyDisableBitmask_ForwardSlash | SwkbdKeyDisableBitmask_Backslash

Currently (in this PR), all filters are disabled. should I port this filter as default value for swkbd ?

natinusala commented 3 years ago

@WerWolv do you know why you disabled all "special" chars in swkbd by default? should we keep it that way or enable everything back?

natinusala commented 3 years ago

This is fine otherwise !

Let's wait for the answer from Wer and I'll try it once it's finished (and formatted 👀 ).

H0neyBadger commented 3 years ago

Hi @natinusala, thank you a lot for this review, by fixing the format, you mean the multiline on function declaration ? let me know if you have any suggestion. I will fix it asap

class Swkbd
{
  public:
    static bool openForText(std::function<void(std::string)> f, std::string headerText = "", std::string subText = "", int maxStringLength = 32, std::string initialText = "");
    static bool openForNumber(std::function<void(int)> f, std::string headerText = "", std::string subText = "", int maxStringLength = 32, std::string initialText = "", std::string leftButton = "", std::string rightButton = "");
    static bool openForText(std::function<void(std::string)> f, std::string headerText = "",
        std::string subText = "", int maxStringLength = 32, std::string initialText = "",
        int kbdDisableBitmask = KeyboardKeyDisableBitmask::KEYBOARD_DISABLE_NONE);
    static bool openForNumber(std::function<void(int)> f, std::string headerText = "",
        std::string subText = "", int maxStringLength = 32, std::string initialText = "",
        std::string leftButton = "", std::string rightButton = "",
        int kbdDisableBitmask = KeyboardKeyDisableBitmask::KEYBOARD_DISABLE_NONE);
};

I ran ./scripts/format.sh --fix with clang-format 11 (default version on my distrib)

WerWolv commented 3 years ago

@natinusala I think I just copied that function from my EdiZon codebase where I don't want these characters anywhere. You can absolutely change that behaviour

natinusala commented 3 years ago

Okay then @H0neyBadger I'd like everything to be available by default (so nothing disabled)

As for the formatting, we use clang-format, there is a script available : ./scripts/format.sh --fix

H0neyBadger commented 3 years ago

yes, I already ran the fix ./scripts/format.sh --fix but with clang-format 11 instead of clang-format-8. I do not know if there is big difference. (clang-tools-extra-11.0.0-1.fc33.x86_64) I can use a container with clang-format-8 to compare the result if needed

H0neyBadger commented 3 years ago

I just tested with ubundu's clang-format-8 and there is nothing to fix anymore (the current proposed patches are not related to this PR). As far as I can see, the scripts/format.sh --fix is ok. But let me know if I have to change some formats/syntax/names.

natinusala commented 3 years ago

So you're telling me that it's clang-format that splits the ListItem constructors in two lines? Same for openForNumber and friends

H0neyBadger commented 3 years ago

Hi, I did the split first but clang-format does not complain about it. Should I put the ListItem construtors back in one line ?

H0neyBadger commented 3 years ago

hi, all function declaration are back in one line (manually reverted). let me know if I have to reformat something. thank you a lot for your help.

H0neyBadger commented 3 years ago

Hello @natinusala, sorry for asking. I know that's very difficult to find a spare moment for this. Do you know if this change can be reviewed in a near future?

I would like to improve our "chiaki" project & user inputs. But currently, I prefer to avoid workarounds in our code. let me know If I have to implement this feature in our side. thank you a lot

natinusala commented 3 years ago

Hi, I am so sorry, I missed the latest changes you made, I always get an email and check as soon as I can but I never got the emails for that...

The code looks fine! Can I request one last thing (sorry), to add some example ListItems to the example app to showcase the different swkbd modes?

Thanks!

H0neyBadger commented 3 years ago

Hello, thank you a lot for your reply, Ok no problem, I will add few examples ( like numbers and string ...)

btw some sxos users tend to report errors on swkbdShow. When they are using the regular homebrew channel the keyboard never show up. It affects all swkbdShow calls (not only borealis). the workaround is to run the homebrew channel in app mode, with full memory access.

H0neyBadger commented 3 years ago

Hi, I just added an example. Let me know if I have to change something.

H0neyBadger commented 3 years ago

Nice catch. I've made the change requested

natinusala commented 3 years ago

Hey @H0neyBadger how are you?

I am currently in the process of changing the license of borealis from GPLv3 to Apache 2.0, and I need explicit permission from all contributors before continuing.

Do you allow me to change the license?

Thanks!

H0neyBadger commented 3 years ago

Hi, I m fine and you ? ;-) Yes, you have my explicit permission to change the license from GPLv3 to Apache 2.0.

Is there any particular reason to change the license ? Does it has any consequences for gpl3 projects that use borealis as gui ?

On Thu, 18 Feb 2021, 21:48 Nathan S., notifications@github.com wrote:

Hey @H0neyBadger https://github.com/H0neyBadger how are you?

I am currently in the process of changing the license of borealis from GPLv3 to Apache 2.0, and I need explicit permission from all contributors before continuing.

Do you allow me to change the license?

Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/natinusala/borealis/pull/73#issuecomment-781624801, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3ZUM7DDTMINFRPBRWALCTS7V4JBANCNFSM4TH7ALKA .

natinusala commented 3 years ago

I'm great thank you😄

The reason is that GPLv3 is objectively a bad choice for a library, it's too limiting. And people told me that when I released it.

With yoga I made the library portable by abstracting away everything platform specific, but I realized that it's useless if the code stays GPL because nobody would be able to port it to their own platform.

I would like the library to grow and for that I need to allow usage for everyone, even commercial projects.

As for your question, it should not change anything for projects using borealis as long as the license is compatible with Apache 2.0. If it's not then you are stuck with the latest GPL version.

The change is only for yoga anyway, so I think that you're safe for now with chiaki.

H0neyBadger commented 3 years ago

Ok thank you a lot for this explanation. I have to save some time to try the latest yoga version :) . Hope your project will find the success it deserves.

I remember when I tried borealis for the first time, the multiplatform was a huge point in favour of borealis.

On Fri, 19 Feb 2021, 08:40 Nathan S., notifications@github.com wrote:

I'm great thank you😄

The reason is that GPLv3 is objectively a bad choice for a library, it's too limiting. And people told me that when I released it.

With yoga I made the library portable by abstracting away everything platform specific, but I realized that it's useless if the code stays GPL because nobody would be able to port it to their own platform.

I would like the library to grow and for that I need to allow usage for everyone, even commercial projects.

As for your question, it should not change anything for projects using borealis as long as the license is compatible with Apache 2.0. If it's not then you are stuck with the latest GPL version.

The change is only for yoga anyway, so I think that you're safe for now with chiaki.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/natinusala/borealis/pull/73#issuecomment-781895287, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3ZUM3X7AQBM2IBOL5VVW3S7YIWXANCNFSM4TH7ALKA .