Added option to enable or disable global actions

[NicholeMattera]

Another thing we talked about on Discord. When running as qlaunch you don't want the user from being able to ever exit out of it. The other use case was for updaters (Or even when just an app is updating itself) where if the user were to exit it would leave their SD card or the app itself in a possible broken state. I also included an option to enable and disable the FPS toggle as I could see that being useful for debugging, but probably don't want to keep that in for release builds.

[natinusala]

Thanks for the PR!

IMO the global quit option should be false by default. I expect borealis to be used to make "proper" apps with a navigation flow between multiple activities, etc... I feel like this global quit option would be a nuisance to be enabled by default.

I have a few requests:

• can you add documentation above the two methods you added? to explain what they toggle, and most importantly, say what key is used for each global action (using the brls controller buttons enum entries)
• can you add a method to "add an exit action" on an activity? that way people can easily add a "+ to exit" feature to their "main menus" if they want to, by calling that method on the activity
• can you make it so that unregistering an action only takes the controller button as parameter? unless we want to support multiple actions on the same view with the same button, but I don't think that would be a good thing to have

A cleaner but harder to implement alternative would be to return an identifier to an action when registering it. That way you can store the identifier and use it later in unregisterAction. It's how Event works, when you subscribe to an event it gives you a "subscription" handle that you can reuse later to remove that one particular callback from the event.

[natinusala]

And so the point of all those changes is to remove all the redundancy of the registerAction / unregisterAction calls that give the same hint, button, callback, etc...

[NicholeMattera]

can you make it so that unregistering an action only takes the controller button as parameter? unless we want to support multiple actions on the same view with the same button, but I don't think that would be a good thing to have

I originally had this, but I was afraid of someone overriding the action and then their custom one is wiped out when using setGlobalQuit(false). I like the idea of returning an identifier and using that to unregister an action.

[natinusala]

The other solution is to remove the setGlobalQuit() entirely and just require people to call addQuitAction() on every activity if they need to 🤷 I don't really know if people expect + to quit everywhere or just on the "main menu"

[Slluxx]

I don't really know if people expect + to quit everywhere or just on the "main menu"

Imo it would me mildly annoying to have to go to the main menu to quit the application. We are not just used to it in Borealis but the entire switch with the home button. Though it does not quit the application, it does take you home wherever you are on the system. I am certain that changing this "flow" would feel pretty disrupting.

[natinusala]

Yeah but take setup wizards for instance, you don't want the user to abort the install process in the middle of it

You also may want to save what has changed before quitting. Or put a dialog "you have unsaved changes"... etc

I'd say it depends on the app, and by default I'd prefer to disable it to avoid "damage" in case it's forgotten.

Le 11 mars 2021 10:54:52 Calvin @.***> a écrit :

I don't really know if people expect + to quit everywhere or just on the "main menu" Imo it would me mildly annoying to have to go to the main menu to quit the application. We are not just used to it in Borealis but the entire switch with the home button. Though it does not quit the application, it does take you home wherever you are on the system. I am certain that changing this "flow" would feel pretty disrupting. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub, or unsubscribe.

[NicholeMattera]

I will try to work on this tonight. Sorry been preoccupied with other things. 👍

[NicholeMattera]

can you add a method to "add an exit action" on an activity? that way people can easily add a "+ to exit" feature to their "main menus" if they want to, by calling that method on the activity

Just wanted to verify if you wanted to do this. Sounds like you would prefer if people didn't follow this pattern, and if they really want this then they can either use brls::Application::setGlobalQuit(true); or just register an action and call brls::Application::quit(); manually.

[natinusala]

Actually now that I think about it, why can't we have both to please everyone?

• make an addExitAction method for people who want exit on some activities, like main menu
• keep setGlobalQuit for people who want the ability to quit from everywhere, but setGlobalQuit must call addExitAction to reduce redudancy
• global quit is off by default, so that every developer makes a deliberate choice to implement one of the two options

How does that sound?

[NicholeMattera]

Done. Also modified the demo so you can exit out of it. I can always change it to use registerExitAction or setGlobalQuit if you want. I just made it so BUTTON_B would exit. Should registerExitAction maybe take a button as an argument, but default the argument to BUTTON_START? Went ahead and changed the demo to use registerExitAction and made it optionally take an argument of what button.

[natinusala]

Actually :see_no_evil: After testing your changes I realized that I use START to quit the demo myself when I'm writing code for the library. I open the demo, go where I want to test my stuff, test it and press START to quit.

So... can you enable global quit for the demo please?

[NicholeMattera]

Actually see_no_evil After testing your changes I realized that I use START to quit the demo myself when I'm writing code for the library. I open the demo, go where I want to test my stuff, test it and press START to quit.

So... can you enable global quit for the demo please?

Changed it to use the global quit and added documentation.

[NicholeMattera]

Something I thought about... Would it make more sense to change std::vector<Action> actions; to be std::unordered_map<ControllerButton, Action> actions;. Actions are unique by ControllerButton and this should come with a slight performance increase. I can wait and do a separate PR if you want. It would just touch a lot of the same files.

[natinusala]

Well if you can make it so that the original behavior is entirely kept, sure. That would be in another PR though :p