flatbuffers icon indicating copy to clipboard operation
flatbuffers copied to clipboard

Cannot add a new field to an existing flatbuffer with reflection

Open marstaik opened this issue 3 years ago • 2 comments

Read the linked discussion post below to catch up...

I've spent some time digging through the code, and although the reflection code seems to have the ability to expand vector memory and update offsets, it seems unable to do the same thing with the vtable to add in new entries. I haven't investigated into the depth of flatbuffers to figure out exactly if its difficult, or if was just missed.

It seems if there isn't a value already set to begin with, that you cant add it at all, which doesn't seem to be mentioned anywhere. The only real reference I can find is https://github.com/google/flatbuffers/blob/74a25536be1d3242f52d98007a6a03934b675cbd/tests/test.cpp#L934

Maybe I'm not going down the right path for this - I'm trying to generate UI to be able to edit/read/view flatbuffers based off of the generated reflection schema. It would probably be easier to use the object api to do so -- if it had reflection support - but I can't find anything saying that it does.

Discussed in https://github.com/google/flatbuffers/discussions/7307

Originally posted by marstaik May 16, 2022 Hi, I am trying to generate a reflection-based ui so I can edit flatbuffers. I don't care if the performance isnt ideal when im BUILDING them in this case, as its a GUI tool, and I understand as strings and other optional tables are added by the user, that the underlying vector has to be resized.

Ive gone through ReflectionTest in test.cpp, and am using a resizing vector with a PIV just like the test. I can set a struct member just fine, but I cant set an optional string, even with the AddFlatBuffer function.

It goes something like this:

	virtual void text_changed( const String& p_str ) override
	{
		// fb
		// "fb" is simply a wrapper object that holds a vector data, and the "piv" to the root inside of it
		// it can be coerced to both Table* and Table* via operator() casts for ease of use

		const flatbuffers::String* str = flatbuffers::GetFieldS( *fb, *field );
		// There's already a string in the table, modify it
		if ( str ) {
			flatbuffers::SetString(
				*schema,
				std::string( p_str.ascii() ),
				flatbuffers::GetFieldS( *fb, *field ),
				&fb->data );
			
			print_line("MODIFIED FB STRING");
			return;
		}

		// we need to add a new one
		flatbuffers::FlatBufferBuilder s_fbb;
		s_fbb.Finish( s_fbb.CreateString( (const char*)p_str.ascii() ) );
		auto string_ptr = flatbuffers::AddFlatBuffer( fb->data, s_fbb.GetBufferPointer(), s_fbb.GetSize() );
		
		bool ret_val = flatbuffers::SetFieldT( *fb, *field, string_ptr );
		
		print_line("ADDED FB STRING");
	}

In this case, ret_val is always false. It seems to fail in SetFieldT >>> table->SetPointer() >>> GetOptionalFieldOffset(field) which returns 0, causing it to fail.

I notice the test example uses a vector of strings, and manually resizes the vector to add in a new string offset, which is different. I do not see a test case that simply adds in a string by itself.

Is there a different way of doing this? Or is it a bug?

marstaik avatar May 16 '22 23:05 marstaik

Flatbuffers is largely immutable by design. We use relative pointers almost everywhere, which may be invalidated by any edits that resize items in the buffer. So a reflection only approach will run into a lot of challenges indexing and fixing all pointers in the buffer with every edit. As you've observed, the right solution to this would be to add reflection support to the object API, though that hasn't happened yet.

CasperN avatar May 23 '22 12:05 CasperN

Why not edit the schema file directly instead of doing it via reflection?

If you added a new field with reflection, no one else will be able to read it, since the codegen for that new field doesn't exist. So I don't really see the point.

dbaileychess avatar Sep 13 '22 06:09 dbaileychess