"ReturnKeyPressed" Signal on textbox widget

Started by billarhos, 01 March 2019, 08:42:03

billarhos


pTextBox->connect("ReturnKeyPressed", [&]()
{

});

pTextBox->connect("ReturnKeyPressed", pressedFunc, pTextBox);


both above declarations are giving me an exception.

Quote
Exception thrown at 0x743B18A2 in Tgui.exe: Microsoft C++ exception: tgui::Exception at memory location 0x0113EA68.
Unhandled exception at 0x743B18A2 in Tgui.exe: Microsoft C++ exception: tgui::Exception at memory location 0x0113EA68.

in
Quoteunsigned int SignalWidgetBase::connect(std::string signalName, Func&& handler, const Args&... args)

I am using the day before yesterday's patch. (before "right click" in listview fixed)

Missing something?



texus


billarhos

like this:

void CALLBACK pressedFunc(tgui::TextBox::Ptr box)
{

}


Tip: I do not use both connect functions at once in my code. I just used one at a time. But both has given me exception.

texus

#3
The TextBox class has no "ReturnKeyPressed" signal, only EditBox has it.
If you would catch the exception and print it then it would print: "No signal exists with name 'returnkeypressed'".

I always find it weird that tools don't show the exception message by default, gcc on linux just shows the message in the terminal when the program exits due to an exception.

billarhos

My silly mistake!!

I had opened "Editbox.hpp" to find the signal strings but i was using textbox instead of editbox!

In the future can you gather all signal strings for all widgets in on hpp file? is my thought correct?

texus

In the future I want to add a list of all signals for all widgets somewhere on the website and there was a suggestion a while back to define constants containing the string (so that you do e.g. connect(tgui::EditBox::ReturnKeyPressed) instead of connect("returnkeypressed")), but I haven't made any decisions yet about how I'm actually going to do it.

billarhos

Defining constants is a brilliant idea and this can eliminate stupid errors like mine. Adding a web page with all signals on the other hand, will be very helpful but not necessary if you implement constants.



texus

Maybe I should just throw away the signal system like it is now.

The biggest issue that I have with constants was that they make the connect calls longer and add duplication (widget type has to be repeated):
Code (cpp) Select
childWindow->connect("closed", []{});
vs
Code (cpp) Select
childWindow->connect(tgui::ChildWindow::Closed, []{});

If I go the route of defining things in the widget itself, I might as well just remove the string altogether:
Code (cpp) Select
childWindow->onClose.connect([]{});

Currently that code actually already works, there exists an onClose object (and one for every other signal). But because there are currently 2 ways to connect signals (via the "closed" string or with the onClose object) I had deprecated the onClose variant and only documented the string version.

There are 2 reasons why I don't like the onClose version:
- It is less flexible, you can't load the signal names from a text file.
- It just doesn't fit with the rest of the library. Widgets only have public functions, not public objects.

But maybe I shouldn't worry so much about these issues. When loading signals from a text file you would use a general callback function so I wouldn't need to support unbound parameters. So I could keep a simplified version of the current connect function for this case and suggest using the onXyz variant (which would support all features) in all other cases.
Instead of onClose, I could maybe rename it to Closed. I think childWindow->Closed.connect([]{}); looks slightly better than using onClose.

Do you perhaps have an opinion on what you think would be good or bad about the signal system?

billarhos

Let me think about it.
In meantime think about enums.
Instead of string and constants you can use enums and along with some inline functions you can easily check validness or you can use "defineImprovedEnum.h".
I use it all the time.


https://www.codeproject.com/Articles/32000/Improving-C-Enums-Adding-Serialization-Inheritance?msg=3818041#xx3818041xx

billarhos

QuotechildWindow->onClose.connect([]{});

The above declaration isn't easy to be found if you looking for the signals.

QuotechildWindow->connect.onClose([]{});
QuotechildWindow->connect.onShow([]{});
QuotechildWindow->connect.onMinimize([]{});

These are much more clarified for me but i don't know if it can be implemented in your current code.

I would prefer constants or enums for the general purpose since signals should be loaded from txt file.

texus

QuoteThe above declaration isn't easy to be found if you looking for the signals.
i agree with this, but the current signal names aren't any easier to find. The constants might also be hard to find.

QuoteThese are much more clarified for me but i don't know if it can be implemented in your current code.
It might be very hard to do because of the inheritance between the widgets.

QuoteI would prefer constants or enums for the general purpose since signals should be loaded from txt file.
I also prefer something flexible like this (which is why I started using strings in the first place), but I'm not sure if I can continue to justify this design as it has some severe downsides. One connect function that takes the type as argument (as string or as enum) can't know which function signatures are valid so it somehow has to delay checking the function type to runtime.
I got it working like this so I can continue using it for 0.8, but I doubt I will continue the current signal system in 0.9. Just have a look at the comment above the TGUI_UNSAFE_TYPE_INFO_COMPARISON define in Config.hpp to get an idea of what kind of issues the design brings. Almost the entire SignalImpl.hpp file could also be removed if I wouldn't have to delay things until runtime.

So if I stick with the current design in 0.8 (as opposed to starting a new one and deprecating the current one) then I will just add constants so that you can do the following. But I will still be looking for a better way for TGUI 0.9.
Code (cpp) Select
childWindow->connect(tgui::ChildWindow::Closed, []{});


billarhos

Had u any time to look at improved enum?
The only disadvantage is that strings in txt file should be case sensitive.



STATIC_METHOD inline const ENUMSTRING Enum2String(const EnumType& t)
STATIC_METHOD inline const ENUMSTRING Enum2FullString(const EnumType& t)
STATIC_METHOD inline const EnumType String2Enum(const ENUMSTRING& s)
STATIC_METHOD inline const EnumType FullString2Enum(const ENUMSTRING& s)
STATIC_METHOD inline const EnumType NextEnumItem(const EnumType& t)
STATIC_METHOD inline const EnumType PreviousEnumItem(const EnumType& t)
STATIC_METHOD inline const int NumberOfValidEnumItem()

//EXAMPLE
state = LangPageState::PreviousEnumItem(state);

if (state == LangPageState::NotValidEnumItem)
{

}
else
{

}

//----------------------------------------------------------------------------------
#define IMPROVED_ENUM_NAME  TextureAlign
#define IMPROVED_ENUM_LIST   ENUMITEM(ALIGN_NONE) \
ENUMITEM(ALIGN_EXPAND) \
ENUMITEM(ALIGN_LEFT) \
ENUMITEM(ALIGN_RIGHT)         \
ENUMITEM(ALIGN_TOP) \
ENUMITEM(ALIGN_BOTTOM) \
ENUMITEM(ALIGN_BOTTOM_RIGHT)
#include "defineImprovedEnum.h"


//----------------------------------------------------------------------------------
#define IMPROVED_ENUM_NAME  ccTalkFlags
#define IMPROVED_ENUM_LIST ENUMITEM_VALUE(Log, 1) \
ENUMITEM_VALUE(ErrorLog, 2) \
ENUMITEM_VALUE(BoxMessage, 4) \
ENUMITEM_VALUE(TextMessage, 8) \
ENUMITEM_VALUE(Acknowledge, 16) \
ENUMITEM_VALUE(Reset, 32)
#include "defineImprovedEnum.h"

texus

Can you show an example of how it would look like for the user to connect a callback?
Wouldn't it be very similar to "childWindow->connect(tgui::ChildWindow::Closed, []{});"? At least as far as I can tell it has the same advantages and disadvantages as this method.

QuoteThe only disadvantage is that strings in txt file should be case sensitive.
I can live with that.

billarhos

#14
like this:


namespace tgui
{
#define IMPROVED_ENUM_NAME  ChildWindowConnect
#define IMPROVED_ENUM_LIST ENUMITEM(Closed) \
      ENUMITEM(Mimimize) \
      ENUMITEM(Focus)
#include "defineImprovedEnum.h"
}

childWindow->connect(tgui::ChildWindowConnect::Closed, [] {});

//in your code
connect(tgui::ChildWindowConnect::EnumType connectType, ..);



texus

I see very little advantages of this compared to just defining tgui::ChildWindow::Closed as a string.
String to enum conversion is very nice and can be interesting for other parts in TGUI, but if we already use a string then we don't really need it.
Inheritance of the enum is interesting as well, but I don't think it offers anything extra in this situation.