En_Kaizoku and object_kz documented: the pirate fighter miniboss
#1788
Conversation
hensldm
added
documentation
| /* 1 */ KAIZOKU_ANIM_OOT_CONVERSATION, // giving player membership card? | ||
| /* 2 */ KAIZOKU_ANIM_OOT_JUMP, // replaced with _LAND |
There was a problem hiding this comment.
I think we can drop the OOT part of the name and just add a comment that they are unused remnants from that game instead
There was a problem hiding this comment.
Hmm, What should "KAIZOKU_ANIM_OOT_CONVERSATION" be renamed to then? There are 4 other animations used in the game that actually involve talking, and this one does not in this game. Naming the animation CONVERSATION without specifying its not used, could cause confusion.
| this->unk_2EC = this->picto.actor.world.rot.z; | ||
| this->picto.actor.world.rot.z = 0; | ||
| this->colorType = KAIZOKU_GET_TYPE(this); | ||
| KAIZOKU_GET_TYPE(this) = 0; // clear param, which was rot.z, resets skew |
There was a problem hiding this comment.
It may be better to have a SET_TYPE instead of abusing the getter in this way
There was a problem hiding this comment.
How do you feel about reverting the macro alltogether, reverting to naked rot.z manipulation, with the comment alone?
Setter feels opposite: it's not setting a parameter to be used later as a parameter, its clearing the parameter from a hijacked general purpose parameter before it can corrupt the model in-game with irrelevant data.
I guess I make a clear macro instead KAIZOKU_CLEAR_Z_ROT(this); which actually signals the intent.
Co-authored-by: Derek Hensley <[email protected]>
| typedef enum KaizokuTextOffset { | ||
| /* 0 */ KAIZOKU_COVERSATION_INTRO_1, // shout before landing | ||
| /* 1 */ KAIZOKU_COVERSATION_INTRO_2, // after landing | ||
| /* 2 */ KAIZOKU_COVERSATION_WIN, // after losing to player | ||
| /* 3 */ KAIZOKU_COVERSATION_LOSS // after defeating player | ||
| } KaizokuTextOffset; |
There was a problem hiding this comment.
Seems like you could just name these in a way such that the comments (and the disambiguation between INTRO_1 and INTRO_2) are unnecessary, e.g.:
typedef enum KaizokuTextOffset {
/* 0 */ KAIZOKU_COVERSATION_SHOUT_FROM_ABOVE,
/* 1 */ KAIZOKU_COVERSATION_INTRO,
/* 2 */ KAIZOKU_COVERSATION_DEFEAT,
/* 3 */ KAIZOKU_COVERSATION_VICTORY
} KaizokuTextOffset;I renamed the latter two to have them from the actor's perspective. If you really wanted to be explicit about who is defeating who, you could name the latter two like:
/* 2 */ KAIZOKU_COVERSATION_DEFEATED_BY_PLAYER,
/* 3 */ KAIZOKU_COVERSATION_DEFEATED_PLAYERAlso, the enum is called KaizokuTextOffset, but then the values are prefixed with KAIZOKU_CONVERSATION. Should the enum be KaizokuConversation, or should these be prefixed with KAIZOKU_TEXT_OFFSET_? I think it's probably the latter, but I'm not sure.
There was a problem hiding this comment.
I want to avoid both being called "defeat" with a small not "by" that confuses them at a glance, for now I'll change it to PLAYER_VICTORY/DEFEAT to match the actionfuncs
I haven't documented an actor in a long time, so I'm rusty and this may need more scrutiny.
A good amount of this documentation was taken from OOTs version of this actor. All of the object is named, all of the functions are named, however there are still several struct values and states undocumented.