search

sony / flutter-embedded-linux

Embedded Linux embedding for Flutter
BSD 3-Clause "New" or "Revised" License
1.16k stars 123 forks source link

Refactor: convert view properties structs to classes #315

Closed vially closed 1 year ago

vially commented 1 year ago

Convert FlutterDesktopViewProperties and ViewProperties from structs to classes. The main motivation for this change is to be able to use std::string member variables as view properties.

This is necessary for an upcoming pull-request (#316) that allows configuring the window title and app-id using these view properties.

Warning

:warning: Please review this very carefully. I'm not familiar enough with C++ and the code-base to understand if this might have unintended side-effects. Also, feel free to suggest alternative solutions it these changes are not deemed acceptable.

Note: I agree to delegate all rights related to this PR to Sony.

vially commented 1 year ago

Thanks for the feedback, that makes sense.

the embedder's interface must use C language in order for user-land side to be able to use other languages such as C lang.

Does this requirement apply to both FlutterDesktopViewProperties and flutter::FlutterViewController::ViewProperties or only to FlutterDesktopViewProperties?

As far as i can tell flutter::FlutterViewController::ViewProperties is declared inside a C++ class already (FlutterViewController) so maybe that one is not supposed to be compatible with other languages or C anyway?

If that's the case, do you think it makes sense to convert only flutter::FlutterViewController::ViewProperties into a class and keep FlutterDesktopViewProperties as a struct?

HidenoriMatsubayashi commented 1 year ago

Oh, sorry. I mistook the check. Correctly, I remember using a struct according to Google's coding style. See: https://google.github.io/styleguide/cppguide.html#Structs_vs._Classes

vially commented 1 year ago

I'm closing this pull-request because #316 has been updated to no longer require this refactoring (i.e.: the FlutterDesktopViewProperties and ViewProperties can still be kept as structs).

I think the new implementation from #316 is more in-line with the linked C++ Google Style Guide and it no longer breaks interoperability with other languages (e.g.: plain C) when using the FlutterDesktopViewProperties type.