Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Tracked by #220
luckytyphlosion opened this issue Aug 17, 2023 · 9 comments

Comments

@luckytyphlosion
Copy link
Member

luckytyphlosion commented Aug 17, 2023

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;
@red031000
Copy link
Member

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"

@red031000
Copy link
Member

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

@luckytyphlosion
Copy link
Member Author

luckytyphlosion commented Aug 18, 2023

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

@luckytyphlosion
Copy link
Member Author

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.

@red031000
Copy link
Member

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

@luckytyphlosion
Copy link
Member Author

Is that really possible to enforce while matching?

@red031000
Copy link
Member

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

@luckytyphlosion
Copy link
Member Author

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

@luckytyphlosion
Copy link
Member Author

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.

@luckytyphlosion luckytyphlosion changed the title Fix all struct names to be consistent with the agreed upon style guide. Decide consensus on styling for recursive/"references each other" structs AND styling for unions. Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants