add XMLLoader module to load widget tree from xml file#101
add XMLLoader module to load widget tree from xml file#101brumazzi wants to merge 8 commits intoTankOs:masterfrom
Conversation
|
Thanks for the MR. Looking at the XML file, it seems to be a very cool feature. I'm unsure whether I'd like to have an XML parser in the core library, though. It adds a dependency and complexity for something only a fraction of SFGUI's users need and will use. I'll give this a spin to get a feel for it. :) Btw.: The CI builds fail. |
eXpl0it3r
left a comment
eXpl0it3r
left a comment
There was a problem hiding this comment.
I highly recommend to not use in-source builds. This will make it a lot easier to not accidentally check in build files, like you currently did.
The code style should match the existing code base.
There's something really odd going on with dynamic library loading that shouldn't be there.
examples/cmake_install.cmake
Outdated
examples/Makefile
Outdated
There was a problem hiding this comment.
Don't check in the CMakeFiles directory.
src/SFGUI/UI/XMLLoader.cpp
Outdated
| // this function is used because sf::String can't be parse special characters from string to string32 | ||
| static std::u32string utf8_to_utf32(std::string utf8_str) { | ||
| std::u32string result; | ||
|
|
||
| size_t size; | ||
| uint32_t* utf32 = u8_to_u32(reinterpret_cast<const uint8_t*>(utf8_str.data()), utf8_str.size(), nullptr, &size); | ||
| if(!utf32) return U""; | ||
|
|
||
| result.assign(utf32, utf32 + size); | ||
| free(utf32); | ||
| return result; | ||
| } |
There was a problem hiding this comment.
I don't think we need a whole new library to parse strings.
SFML's sf::Utf<T> utilities can certainly convert from UTF8 to UTF32
src/SFGUI/UI/XMLLoader.cpp
Outdated
|
|
||
| void test(sfg::Widget::Ptr widget){ | ||
| puts("OK"); | ||
| } |
| for(auto child = element->FirstChildElement(); child != nullptr; child = child->NextSiblingElement()){ | ||
| generateWidget(child, newWidget); | ||
| } | ||
| }else if(!elementName.compare("button")){ // create Button widget |
There was a problem hiding this comment.
You should follow the same coding style as the rest of the SFGUI code base
src/SFGUI/UI/XMLLoader.cpp
Outdated
| float min = 0.0, | ||
| max = 0.0, | ||
| step = 2.0, | ||
| value = 0.0; |
There was a problem hiding this comment.
Repeat the type for each variable
src/SFGUI/UI/XMLLoader.cpp
Outdated
| void *handle = dlopen(NULL, RTLD_LAZY); | ||
| if(handle){ | ||
| void (*callback)(sfg::Widget::Ptr); | ||
| callback = (void (*)(sfg::Widget::Ptr)) dlsym(handle, buffAttr->Value()); | ||
| if(callback) callback(newWidget); | ||
| dlclose(handle); |
There was a problem hiding this comment.
Dynamically loading a shared library in a callback is certainly not a good design. Why do we need to load a library at all and what library is that?
dlopen() is Linux only, so this would need to be rewritten as cross-platform, but I still very much doubt we need to dynamically load something here.
There was a problem hiding this comment.
The events in the window.xml file: on-left-click and on-create, receive the parameter as text and perform the binding via dlopen, thus making it possible to define events for the widgets directly from the XML itself (of course, I will make the correction for the window).
3 participants
This "XMLLoader" module reads an XML file and generates the widget structure with the following components: