pret/pokeheartgold

Decide consensus on styling for recursive/"references each other" structs AND styling for unions.

Opened this issue · 10 comments

5. There is no consensus for defining recursive structs.

When defining a recursive struct, the struct must be declared and defined. Below are the options for declaring the struct.

  • Option A1:
struct UnkSPLStruct6;
  • Option A2:
typedef struct UnkSPLStruct6 UnkSPLStruct6;

Below are the options for defining the struct. Option A1 is only compatible with Option B1. Keywords/identifiers in <> are optional.

  • Option B1:
typedef struct UnkSPLStruct6 {
    struct UnkSPLStruct6 * unk_00;
    struct UnkSPLStruct6 * unk_04;
  	// rest of struct omitted
} UnkSPLStruct6;
  • Option B2:
typedef struct UnkSPLStruct6 {
    struct UnkSPLStruct6 * unk_00;
    struct UnkSPLStruct6 * unk_04;
};
  • Option B3:
typedef struct UnkSPLStruct6 {
    UnkSPLStruct6 * unk_00;
    UnkSPLStruct6 * unk_04;
} <UnkSPLStruct6>;
  • Option B4:
struct UnkSPLStruct6 {
    <struct> UnkSPLStruct6 * unk_00;
    <struct> UnkSPLStruct6 * unk_04;
};

6. There is no consensus on whether unions should follow the same rule as structs.

  • Option 1:
// no union typedef
union StrbufForPrint {
    String *wrapped;
    const u16 *raw;
};

typedef struct TextPrinterTemplate {
	// mention union here
    union StrbufForPrint currentChar;
    Window *window;

	// rest of struct omitted
} TextPrinterTemplate;
  • Option 2:
// union typedef
typedef union StrbufForPrint {
    String *wrapped;
    const u16 *raw;
} StrbufForPrint;

typedef struct TextPrinterTemplate {
	// no mentioning union
    StrbufForPrint currentChar;
    Window *window;

	// rest of struct omitted
} TextPrinterTemplate;

sometimes it is necessary to have the typedef and struct definition separately, usually when the structs contain pointers to eachother, in that case the de facto standard is the following

Struct1.h

#include "Struct2.h"

struct Struct1 {
    Struct2 struct2;
};

Struct2.h

typedef struct Struct1 Struct1;

typedef struct Struct2 {
    Struct1 struct1;
} Struct2;

ofc what struct contains the typedef is somewhat arbritrary, but it's usually based on which one is "higher level" or "more specific"

as for unions, imo they should follow the same rules as structs

sometimes it is necessary to have the typedef and struct definition separately, usually when the structs contain pointers to eachother, in that case the de facto standard is the following

Struct1.h

#include "Struct2.h"

struct Struct1 {
    Struct2 struct2;
};

Struct2.h

typedef struct Struct1 Struct1;

typedef struct Struct2 {
    Struct1 struct1;
} Struct2;

ofc what struct contains the typedef is somewhat arbritrary, but it's usually based on which one is "higher level" or "more specific"

I already mentioned this in point 5

as for unions, imo they should follow the same rules as structs

I think the issue with unions is that knowing that it's a union is very useful context, because the way that you operate on unions is much differently than structs. One possibility is that they follow the same rules as structs, but have to be suffixed with Union.

generally I don't think you should really be passing unions around by themselves, all unions should be within a struct, even if it is the only thing in the struct imo

Is that really possible to enforce while matching?

I think so? not sure metrowerks makes that much of a distinction, if it's not actually possible then imo it should be explicit in the type name

GymmickUnion is at least one example of passing around a union itself. So it does exist in the code.

I have realized that labelling all unions with a Union suffix may not be the right choice. Consider this union for example.

typedef union Quaternion_AsMtxF44 {
    struct {
        f32 _00, _01, _02, _03;
        f32 _10, _11, _12, _13;
        f32 _20, _21, _22, _23;
        f32 _30, _31, _32, _33;
    };
    f32 m[4][4];
    f32 a[16];
} Quaternion_AsMtxF44;

This is literally just an MtxFx44, except it uses floats instead of fixed point numbers. The extra fields are "overloads" of accessing the data in different interpretations, since one may want to view the matrix via its individual points, as a 4x4 array, or as a raw list of points. But it is not like a union where it has entirely different data depending on the context. Therefore, I propose that "overload" unions can stay as unions and do not need to be suffixed with Union, while "context" unions must be suffixed with Union.

This issue has had no activity for 60 days and will be marked stale. If there is no further activity, it will be closed in 30 days.