`DataModel` usage

[espkk]

I found some of the features of the DataModel bindings not working, e.g. Set method does pointer to sol::object conversion to primitive types, which simply crashes the app. Another issue is that the lua table is not being updated when assigning to data model from rml.
On the other hand, I noticed that

RmlSolLua/src/plugin/SolLuaDataModel.cpp

Line 78 in 7434be8

DataVariable SolLuaObjectDef::Child(void* ptr, const Rml::DataAddressEntry& address)

contains most of the code, but I can't even get there.

Could you clarify how it's supposed to be used?

PS I fixed the above issues locally, now I'm trying to figure out is there a more convenient to deal with nested tables and arrays, that's why I'm also curios about Child call

[espkk]

I think I figured it out, but haven't tested all possible use-cases. Updated PR to support Child as well

[LoneBoco]

What version of RmlUi are you using? The last time I had checked was either 5.1 or 6.0, and things had worked, although I'm not sure if RmlUi 6.1, a sol2 update, or my last round of changes broke things, I'll try to take a look at things, but the project I was building RmlSolLua for is a little out of date, so it might take me a little while to update it to the latest versions of things.

[LoneBoco]

What's complicated is that this advanced usage of RmlUi data bindings is not documented. It was very much a "figure it out" type of situation. Back then I even had to create RmlUI PRs to extend the functionality:
mikke89/RmlUi#412

I think I explain in that issue what I needed ::Child() for and why it was used. It's been a few years since I last looked into this, though.

[espkk]

Thanks for looking into this!

What's complicated is that this advanced usage of RmlUi data bindings is not documented. It was very much a "figure it out" type of situation. Back then I even had to create RmlUI PRs to extend the functionality: mikke89/RmlUi#412

I think I explain in that issue what I needed ::Child() for and why it was used. It's been a few years since I last looked into this, though.

I agree. I wasn't aware of these changes, and I think I ended up doing it differently 😂

What version of RmlUi are you using? The last time I had checked was either 5.1 or 6.0, and things had worked, although I'm not sure if RmlUi 6.1, a sol2 update, or my last round of changes broke things, I'll try to take a look at things, but the project I was building RmlSolLua for is a little out of date, so it might take me a little while to update it to the latest versions of things.

6.1, but I don't think Set could work with either version. If you look at it

RmlSolLua/src/plugin/SolLuaDataModel.cpp

Lines 43 to 64 in 7434be8

	bool SolLuaObjectDef::Set(void* ptr, const Rml::Variant& variant)
	{
	auto obj = static_cast<sol::object*>(ptr);
	if (obj->is<bool>())
	variant.GetInto<bool>(*static_cast<bool*>(ptr));
	else if (obj->is<std::string>())
	variant.GetInto<std::string>(*static_cast<std::string*>(ptr));
	else if (obj->is<Rml::Vector2i>())
	variant.GetInto<Rml::Vector2i>(*static_cast<Rml::Vector2i*>(ptr));
	else if (obj->is<Rml::Vector2f>())
	variant.GetInto<Rml::Vector2f>(*static_cast<Rml::Vector2f*>(ptr));
	else if (obj->is<Rml::Colourb>())
	variant.GetInto<Rml::Colourb>(*static_cast<Rml::Colourb*>(ptr));
	else if (obj->is<Rml::Colourf>())
	variant.GetInto<Rml::Colourf>(*static_cast<Rml::Colourf*>(ptr));
	else if (obj->is<double>())
	variant.GetInto<double>(*static_cast<double*>(ptr));
	else // if (obj->get_type() == sol::type::lua_nil)
	*obj = sol::make_object(m_model->Lua, sol::nil);
	return true;

First it correctly treats ptr closure as sol::object*, but then depending on its type it proceeds to cast the same ptr to different types. Even if object would be directly pointing to the lua VM memory (which I think is UB both from C++ and lua PoV) different lua engines use either discriminated union or NAN-boxing for value types, so even for primitive types it shouldn't be working...
Maybe you never had the datamodel being mutated in that direction (rml->lua)? If you run hello_world.rml from rml examples you can trigger it by entering something in the input field.

Another issue I had is that it only tracks mutables in the datamodel. If you have to overwrite something, it only gets updated in lua world and never comes through datamodel. That's not something which is strictly specified how it should be done, but since it easily lead to an unexpected behaviour it probably makes sense to either support or prevent it from happening.

I used this test in lua with slightly adjusted hello_world.rml to test my changes:

local src = {
  animal = 'DOG',
  show_text = true,
  test = 1,

  -- nested
  nested = {animal = "nested animal"},
  array = {{name = "A"}, {name = "B"}, {name = "C"}},
  array2 = {111,222,333},

  -- events
  event_surprise = function(event) Surprise(event.current_element) end
}
--- ...
datamodel = context:OpenDataModel("animals", src)
--- ...
  trace(datamodel.animal)

  datamodel.animal = choice
  datamodel.nested.animal = choice
  datamodel.array[1] = {name = choice}
  datamodel.array[2].name = choice
  datamodel.array2[2] = math.random()

and all the above correctly updated both lua tables and datamodel

PS
To be honest I'm not aware of the status of this project and are you even interested in maintaining it. I found a fork of your lib used here, but it was already modified for the specific needs of the specific engine, so I decided to use the original code.
If you don't use it anymore in your projects, I guess it would be easier to just keep my changes in a fork. However, if you still use it and interested in PRs, I can clean this up to make it more reviewable and provide more context/tests if needed.

[LoneBoco]

I mainly built this for my hobby project. I do still maintain it and don't plan to stop, but my spare time has been focused on a different project lately, which is why it has rotted a bit. I am perfectly fine with any PRs. I'll try to get a .clang-format added too to make the PRs easier.

[LoneBoco]

I went ahead and pushed a .clang-format to the main branch and formatted all the code with it. I'm sorry for your pending rebase conflicts.

[espkk]

@LoneBoco I've updated a couple of things. It now is ready for review/merge. Still misses any tests and contracts/doc which in theory I can also add