Closed riidefi closed 10 months ago
Yes, this is pretty much expected. The example is somewhat incomplete, but from what I can make out, here is the problem:
struct JSONMatrixWeight {
rfl::Field<"bone_id", u32> bone_id = 0;
rfl::Field<"weight", f32> weight = 1.0f;
JSONMatrixWeight() = default;
JSONMatrixWeight(const DrawMatrix::MatrixWeight& original)
: bone_id(original.boneId), weight(original.weight) {}
operator DrawMatrix::MatrixWeight() const {
return DrawMatrix::MatrixWeight(bone_id(), weight());
}
};
Because you have added custom constructors, there are now two ways to construct this struct: Either using the default constructor, or through a matrix weight.
However, the library expects to able to construct the struct like this:
auto json_matrix_weight = JSONMatrixWeight {
rfl::Field<"bone_id", u32>(0),
rfl::Field<"weight", f32>(1.0f)};
But you have made this impossible with your custom constructors.
What I would suggest is that you take a look at this:
https://github.com/getml/reflect-cpp/blob/main/docs/custom_classes.md
So instead of having a custom constructor on JSONMatrixWeight
, just do the opposite:
struct JSONMatrixWeight{
rfl::Field<"bone_id", u32> bone_id;
rfl::Field<"weight", f32> weight;
};
struct MatrixWeight{
using ReflectionType = JSONMatrixWeight;
u32 boneId;
f32 weight;
MatrixWeight() : boneId(-1), weight(0.0f) {}
MatrixWeight(u32 b, f32 w) : boneId(b), weight(w) {}
MatrixWeight(const JSONMatrixWeight j): boneId(j.bone_id()), weight(j.weight()) {}
JSONMatrixWeight reflection() const { return JSONMatrixWeight{ .bone_id = boneId, .weight = weight}; }
};
Now you can just directly serialize and deserialize MatrixWeight
, the parser is smart enough to figure this out:
const auto matrix_weight = rfl::json::read<MatrixWeight>(json_str);
Is this an acceptable solution for you? If so, I would close the issue.
If you add custom constructors to your structs, then the original constructors don't work anymore. That's how C++ works, and there is little I can do about this.
Unfortunately, I don't think there is a way that the code snippet you provided could possibly work, no matter what we do.
But again, thank you very much for your input.
Looks like the fix is to explicitly add back the ctors? For instance https://en.cppreference.com/w/cpp/language/direct_initialization
If T is a class type, the constructors of T are examined and the best match is selected by overload resolution. The constructor is then called to initialize the object. otherwise, if the destination type is a (possibly cv-qualified) aggregate class, it is initialized as described in aggregate initialization except that narrowing conversions are permitted, designated initializers are not allowed, a temporary bound to a reference does not have its lifetime extended, there is no brace elision, and any elements without an initializer are value-initialized.
For instance,
struct X {
int a;
int b;
X() = default;
X(int, int);
};
void bar() {
X x{1, 2};
}
compiles as the direct list initialization selects an explicit constructor X::X(int, int)
.
Therefore, I imagine my fix here would be the following,
struct JSONMatrixWeight {
rfl::Field<"bone_id", u32> bone_id = 0;
rfl::Field<"weight", f32> weight = 1.0f;
JSONMatrixWeight() = default;
// Added {
JSONMatrixWeight(auto&& b, auto&& w) : bone_id(b), weight(w) {}
// }
JSONMatrixWeight(const DrawMatrix::MatrixWeight& original)
: bone_id(original.boneId), weight(original.weight) {}
operator DrawMatrix::MatrixWeight() const {
return DrawMatrix::MatrixWeight(bone_id(), weight());
}
};
which has the advantage of not polluting the non-JSON code with JSON conversion operators.
Okay, if you don't want to change your original classes, then how about this?
struct JSONMatrixWeight {
rfl::Field<"bone_id", u32> bone_id = 0;
rfl::Field<"weight", f32> weight = 1.0f;
static JSONMatrixWeight from_matrix_weight(const DrawMatrix::MatrixWeight& original) {
return JSONMatrixWeight{.bone_id = original.boneId, .weight = original.weight};
}
operator DrawMatrix::MatrixWeight() const {
return DrawMatrix::MatrixWeight(bone_id(), weight());
}
};
You could then create a JSONMatrixWeight like this:
JSONMatrixWeight::from_matrix_weight(my_matrix_weight);
The approach of having custom constructors can lead to all kinds of weird side effects and will cause significant maintenance overhead for you, the developer. For instance if you add a new field to MatrixWeight
, there are many places where you have to add it.
For instance, one of these side-effects is that no constructor could accept more variables than the number of fields. To understand why this limitation is necessary, you have to understand that C++ doesn't actually offer reflection for its structs and even things as simple as trying to figure out how many fields there are require significant effort:
https://github.com/getml/reflect-cpp/blob/main/include/rfl/internal/has_n_fields.hpp
That being said, I still think the approach I originally proposed is better, because it would allow the library to fully support your MatrixWeight
class. For instance, this would work as well, but it could never work with the static method:
const auto vec = rfl::json::read<std::vector<MatrixWeight>>(json_str);
Again, thank you very much for this issue. I think I will directly put this in the documentation, because I am sure that you won't be the last one to request this.
I have an idea...give me one second.
I have written a CustomParser
class that can be used if you absolutely do not want to make any changes to the original class.
It is documented here (you will have to pull the library again):
https://github.com/getml/reflect-cpp/blob/main/docs/custom_classes.md
And here is a test case:
https://github.com/getml/reflect-cpp/blob/main/tests/test_custom_class3.hpp
Applying this to your example:
struct JSONMatrixWeight {
rfl::Field<"bone_id", u32> bone_id = 0;
rfl::Field<"weight", f32> weight = 1.0f;
static JSONMatrixWeight from_class(const DrawMatrix::MatrixWeight& original) noexcept {
return JSONMatrixWeight{.bone_id = original.boneId, .weight = original.weight};
}
DrawMatrix::MatrixWeight to_class() const {
return DrawMatrix::MatrixWeight(bone_id(), weight());
}
};
And the custom parser:
namespace rfl {
namespace parsing {
template <class ReaderType, class WriterType>
struct Parser<ReaderType, WriterType, DrawMatrix::MatrixWeight>
: public CustomParser<ReaderType, WriterType, DrawMatrix::MatrixWeight,
JSONMatrixWeight> {};
} // namespace parsing
} // namespace rfl
Now DrawMatrix::MatrixWeight
is fully supported by the library and you didn't have to make any changes to your original class.
This approach would enable you to keep the serialization/deserialization in completely separate files from your original classes. You would only have to include these files before you call rfl::json::read
or rfl::json::write
.
This looks good for my usecase. Thank you so much for going above and beyond here!
It appears adding additional constructors/casting operators is problematic?
Error log
Clang 16.0.5 on Windows 11 x64