Godot Marshalls Simplification Proposal

godot-marshalls-simplification-proposal.cpp

// Here are three of the cases:

        case Variant::VECTOR2: {
            Vector2 val;
            if (type & ENCODE_FLAG_64) {
                ERR_FAIL_COND_V((size_t)len < sizeof(double) * 2, ERR_INVALID_DATA);
                val.x = decode_double(&buf[0]);
                val.y = decode_double(&buf[sizeof(double)]);

                if (r_len) {
                    (*r_len) += sizeof(double) * 2;
                }
            } else {
                ERR_FAIL_COND_V((size_t)len < sizeof(float) * 2, ERR_INVALID_DATA);
                val.x = decode_float(&buf[0]);
                val.y = decode_float(&buf[sizeof(float)]);

                if (r_len) {
                    (*r_len) += sizeof(float) * 2;
                }
            }
            r_variant = val;

        } break;
        case Variant::VECTOR3: {
            Vector3 val;
            if (type & ENCODE_FLAG_64) {
                ERR_FAIL_COND_V((size_t)len < sizeof(double) * 3, ERR_INVALID_DATA);
                val.x = decode_double(&buf[0]);
                val.y = decode_double(&buf[sizeof(double)]);
                val.z = decode_double(&buf[sizeof(double) * 2]);

                if (r_len) {
                    (*r_len) += sizeof(double) * 3;
                }
            } else {
                ERR_FAIL_COND_V((size_t)len < sizeof(float) * 3, ERR_INVALID_DATA);
                val.x = decode_float(&buf[0]);
                val.y = decode_float(&buf[sizeof(float)]);
                val.z = decode_float(&buf[sizeof(float) * 2]);

                if (r_len) {
                    (*r_len) += sizeof(float) * 3;
                }
            }
            r_variant = val;
        } break;
        case Variant::QUATERNION: {
            Quaternion val;
            if (type & ENCODE_FLAG_64) {
                ERR_FAIL_COND_V((size_t)len < sizeof(double) * 4, ERR_INVALID_DATA);
                val.x = decode_double(&buf[0]);
                val.y = decode_double(&buf[sizeof(double)]);
                val.z = decode_double(&buf[sizeof(double) * 2]);
                val.w = decode_double(&buf[sizeof(double) * 3]);

                if (r_len) {
                    (*r_len) += sizeof(double) * 4;
                }
            } else {
                ERR_FAIL_COND_V((size_t)len < sizeof(float) * 4, ERR_INVALID_DATA);
                val.x = decode_float(&buf[0]);
                val.y = decode_float(&buf[sizeof(float)]);
                val.z = decode_float(&buf[sizeof(float) * 2]);
                val.w = decode_float(&buf[sizeof(float) * 3]);

                if (r_len) {
                    (*r_len) += sizeof(float) * 4;
                }
            }
            r_variant = val;
        } break;

// We don't really need to be accessing the elements by
// the named property, we can just use a loop. Once that's
// done, there's a lot of repetition in here that we can
// use a macro to simplify. The above can become this:

#define DECODE_LINEAR_REAL_TYPE(struct_type, variant_type, element_count)                        \
        case variant_type: {                                                                     \
            struct_type val;                                                                     \
            if (type & ENCODE_FLAG_64) {                                                         \
                ERR_FAIL_COND_V((size_t)len < sizeof(double) * element_count, ERR_INVALID_DATA); \
                for (int i = 0; i < element_count; i++) {                                        \
                    val[i] = decode_double(&buf[sizeof(double) * i]);                            \
                }                                                                                \
                if (r_len) {                                                                     \
                    (*r_len) += sizeof(double) * element_count;                                  \
                }                                                                                \
            } else {                                                                             \
                ERR_FAIL_COND_V((size_t)len < sizeof(float) * element_count, ERR_INVALID_DATA);  \
                for (int i = 0; i < element_count; i++) {                                        \
                    val[i] = decode_float(&buf[sizeof(float) * i]);                              \
                }                                                                                \
                if (r_len) {                                                                     \
                    (*r_len) += sizeof(float) * element_count;                                   \
                }                                                                                \
            }                                                                                    \
            r_variant = val;                                                                     \
        } break;

        DECODE_LINEAR_REAL_TYPE(Vector2, Variant::VECTOR2, 2)
        DECODE_LINEAR_REAL_TYPE(Vector3, Variant::VECTOR3, 3)
        DECODE_LINEAR_REAL_TYPE(Quaternion, Variant::QUATERNION, 4)

// The same simplification can be made for these three cases:

        case Variant::VECTOR2I: {
            ERR_FAIL_COND_V(len < 4 * 2, ERR_INVALID_DATA);
            Vector2i val;
            val.x = decode_uint32(&buf[0]);
            val.y = decode_uint32(&buf[4]);
            r_variant = val;

            if (r_len) {
                (*r_len) += 4 * 2;
            }
        } break;
        case Variant::VECTOR3I: {
            ERR_FAIL_COND_V(len < 4 * 3, ERR_INVALID_DATA);
            Vector3i val;
            val.x = decode_uint32(&buf[0]);
            val.y = decode_uint32(&buf[4]);
            val.z = decode_uint32(&buf[8]);
            r_variant = val;

            if (r_len) {
                (*r_len) += 4 * 3;
            }
        } break;
        case Variant::COLOR: {
            ERR_FAIL_COND_V(len < 4 * 4, ERR_INVALID_DATA);
            Color val;
            val.r = decode_float(&buf[0]);
            val.g = decode_float(&buf[4]);
            val.b = decode_float(&buf[8]);
            val.a = decode_float(&buf[12]);
            r_variant = val;

            if (r_len) {
                (*r_len) += 4 * 4; // Colors should always be in single-precision.
            }
        } break;

// Which can become:

#define DECODE_LINEAR_TYPE(struct_type, variant_type, element_count, primitive_type)         \
        case variant_type: {                                                                 \
            ERR_FAIL_COND_V(len < sizeof(primitive_type) * element_count, ERR_INVALID_DATA); \
            struct_type val;                                                                 \
            for (int i = 0; i < element_count; i++) {                                        \
                val[i] = decode_##primitive_type(&buf[sizeof(primitive_type) * i]);          \
            }                                                                                \
            r_variant = val;                                                                 \
            if (r_len) {                                                                     \
                (*r_len) += sizeof(primitive_type) * element_count;                          \
            }                                                                                \
        } break;

        DECODE_LINEAR_TYPE(Vector2i, Variant::VECTOR2I, 2, uint32_t)
        DECODE_LINEAR_TYPE(Vector3i, Variant::VECTOR3I, 3, uint32_t)
        DECODE_LINEAR_TYPE(Color, Variant::COLOR, 4, float)

// Additionally, there's some repetition within the two
// macros that we can simplify with another macro. All 6
// of those original cases (106 lines) can be simplified
// to just 30 lines with little repeated logic:

#define DECODE_LINEAR_TYPE_LOOP(struct_type, element_count, primitive_type)                  \
            ERR_FAIL_COND_V(len < sizeof(primitive_type) * element_count, ERR_INVALID_DATA); \
            struct_type val;                                                                 \
            for (int i = 0; i < element_count; i++) {                                        \
                val[i] = decode_##primitive_type(&buf[sizeof(primitive_type) * i]);          \
            }                                                                                \
            r_variant = val;                                                                 \
            if (r_len) {                                                                     \
                (*r_len) += sizeof(primitive_type) * element_count;                          \
            }

#define DECODE_LINEAR_REAL_TYPE(struct_type, variant_type, element_count)   \
        case variant_type: {                                                \
            if (type & ENCODE_FLAG_64) {                                    \
                DECODE_LINEAR_TYPE_LOOP(struct_type, element_count, double) \
            } else {                                                        \
                DECODE_LINEAR_TYPE_LOOP(struct_type, element_count, float)  \
            }                                                               \
        } break;

#define DECODE_LINEAR_TYPE(struct_type, variant_type, element_count, primitive_type) \
        case variant_type: {                                                         \
            DECODE_LINEAR_TYPE_LOOP(struct_type, element_count, primitive_type)      \
        } break;

        DECODE_LINEAR_REAL_TYPE(Vector2, Variant::VECTOR2, 2)
        DECODE_LINEAR_REAL_TYPE(Vector3, Variant::VECTOR3, 3)
        DECODE_LINEAR_REAL_TYPE(Quaternion, Variant::QUATERNION, 4)
        DECODE_LINEAR_TYPE(Vector2i, Variant::VECTOR2I, 2, uint32_t)
        DECODE_LINEAR_TYPE(Vector3i, Variant::VECTOR3I, 3, uint32_t)
        DECODE_LINEAR_TYPE(Color, Variant::COLOR, 4, float)

// (I tested that it compiles, there is technically one other change needed
// which is that decode_uint32 needs to be renamed to decode_uint32_t)

// Additionally, this is possible because these types (Vector2(i), Vector3(i),
// Quaternion, Color) can all have their primitive members accessed using the
// [] array operator. We could simplify the rest of the cases (Rect2, AABB,
// Basis, Transforms, Plane, etc) if we had a standardized method in each
// struct that let us access a primitive value by index. Then we could take
// about 300 lines and turn them into only about 35 lines.
// Or maybe there's some pointer magic that could let us access struct primitive
// members by index without needing to add methods to each struct.

// The primary function of these changes would be to simplify code and
// reduce duplication. However, I will mention another bonus feature,
// if anyone wants to add a new type to Variant, these kinds of changes
// make things much much easier, since you can just add one line.