Confirmation (Yes/No/OK/Quit/Logout) Overhaul #884
Conversation
|
The implementation and overall design looks good to me. Do we want this as a new fixed key singleton, or could we use pfGameGUIMgr as the receiver of plConfirmationMsg, and have it own a plConfirmationMgr that it passes them to? Putting this in pfCharacter seems a bit weird, but pfCharacter is already a bit weird... 🤷🏼♂️ |
|
I considered putting this into the pfGameGUIMgr target, but I felt like pfCharacter was more appropriate, since that used to house player interface elements many moons ago. I didn't really consider adding it directly to pfGameGUIMgr the class because everything is terrible over there. 😞 |
|
I've adding the Python bindings for |
| if (pyKey::Check(cbObj)) { | ||
| cb = pyKey::ConvertFrom(cbObj)->getKey(); | ||
| } else if (PyCallable_Check(cbObj)) { | ||
| cb = plPythonCallable::BuildCallback<plConfirmationMsg::Result>("PtYesNoDialog", cbObj); |
There was a problem hiding this comment.
@zrax I'm a little unhappy that this function requires manual specification of the callback arguments. I'm sure there's a way to erase those, but I'm drawing a blank. Any ideas?
There was a problem hiding this comment.
I don't think you can in this case, at least not without moving the construction of the tuple outside the callback...
There was a problem hiding this comment.
I got it down to specifying the variant index, which is good enough... I guess...
Hoikas
changed the title
|
We're ready for more detailed review 🚀 |
|
One of my grievances with the old Logout dialog is that it is not keyboard-accessible. A good UI should let a person tab between the buttons, with some indication of which is selected, and ENTER or SPACE or something like that should trigger the button. Should I get my hopes up that this might be addressed with this overhaul? :) |
That's probably a task for a separate round of changes, since it probably involves adding a concept of "focus" to the GUIDialog control classes and potentially changes to the art asset files. Accessibility is important though, and thanks for raising this. Reporting a new issue is probably the best way to keep it from getting lost. |
pfConfirmationMgr is designed to replace the ad-hoc Yes/No dialog implementation in xKI. Currently, the KI handles throwing up its own dialogs and can respond to pfKIMsgs sent with a kYesNo type. The result is the handling of confirmation dialogs is: - Dependent on having a functioning KI python. - Spread over many hundreds of lines of python. - Requires having a plKey to receive a response. These limitations make doing things that require confirmation annoying. This system will allow you to dispatch a confirmation dialog from anywhere in the engine that links against plMessage and receive the results either in a (potentially stateful) lambda or via a legacy notification message.
This will allow them to be reused more easily.
This also includes the first stab at some general code that allows firing an arbitrary callback from the engine into Python code without leaking resources.
This conflicts with what we're doing in pfConfirmationMgr and therefore *MUST* be replaced in order for everything to work correctly.
Low-hanging fruit and improves our localization coverage.
|
Rebased to merge without conflicts. |
While working on the clickable links in KI Chat concept, it became apparent that I needed to issue a confirmation or Yes/No dialog from a Python script that did not have its own
plKey. Currently, Yes/No dialogs are handled by the xKI python scripts over many hundreds of lines in an especially ad-hoc way and can only be responded to using aplNotifyMsg. This implements a singleton whose job is to receive confirmation requests from anyone who links against plMessage and respond to them via either a (potentially stateful) lambda or using a legacyplNotifyMsg.This new functionality will allow us to be very flexible with where we issue and how we respond to confirmation requests. Some examples floating around in my mind:
PtYesNoDialog()can accept a Python callable to call back in addition to a key to notify.