Skip to content

Gold Skulltula/Skullwalltula (En_Sw, En_Si, Obj_Makekinsuta) #2729

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.

Sign up for GitHub

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

Open
djevangelia wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Gold Skulltula/Skullwalltula (En_Sw, En_Si, Obj_Makekinsuta) #2729

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a9bf86d
Gold Skulltula/Skullwalltula (En_Sw, En_Si, Obj_Makekinsuta)
Apr 6, 2026
3e14b81
Update src/overlays/actors/ovl_En_Sw/z_en_sw.c
djevangelia Apr 8, 2026
9ce45cd
Update include/save.h
djevangelia Apr 8, 2026
d7138ef
Update src/overlays/actors/ovl_Obj_Makekinsuta/z_obj_makekinsuta.c
djevangelia Apr 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions include/actor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,11 +660,11 @@ f32 func_8002DCE4(struct Player* player);
int func_8002DD6C(struct Player* player);
int func_8002DD78(struct Player* player);
s32 func_8002DDE4(struct PlayState* play);
s32 func_8002DDF4(struct PlayState* play);
s32 Player_IsClimbingStill(struct PlayState* play);
void Actor_SwapHookshotAttachment(struct PlayState* play, Actor* srcActor, Actor* destActor);
void Actor_RequestHorseCameraSetting(struct PlayState* play, struct Player* player);
void Actor_MountHorse(struct PlayState* play, struct Player* player, Actor* horse);
int func_8002DEEC(struct Player* player);
int Player_IsDeadOrCutscene(struct Player* player);
void Actor_InitPlayerHorse(struct PlayState* play, struct Player* player);
s32 Player_SetCsAction(struct PlayState* play, Actor* csActor, u8 csAction);
s32 Player_SetCsActionWithHaltedActors(struct PlayState* play, Actor* csActor, u8 csAction);
Expand Down
8 changes: 4 additions & 4 deletions include/save.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,10 +492,10 @@ typedef enum LinkAge {
#define CHECK_QUEST_ITEM(item) (gSaveContext.save.info.inventory.questItems & gBitFlags[item])
#define CHECK_DUNGEON_ITEM(item, dungeonIndex) (gSaveContext.save.info.inventory.dungeonItems[dungeonIndex] & gBitFlags[item])

#define GET_GS_FLAGS(index) \
((gSaveContext.save.info.gsFlags[(index) >> 2] & gGsFlagsMasks[(index) & 3]) >> gGsFlagsShifts[(index) & 3])
#define SET_GS_FLAGS(index, value) \
(gSaveContext.save.info.gsFlags[(index) >> 2] |= (value) << gGsFlagsShifts[(index) & 3])
#define GET_GS_FLAGS(mapIndex) \
((gSaveContext.save.info.gsFlags[(mapIndex) >> 2] & gGsFlagsMasks[(mapIndex) & 3]) >> gGsFlagsShifts[(mapIndex) & 3])
#define SET_GS_FLAGS(mapIndex, id) \
(gSaveContext.save.info.gsFlags[(mapIndex) >> 2] |= (id) << gGsFlagsShifts[(mapIndex) & 3])
Comment thread
djevangelia marked this conversation as resolved.
Outdated

#define HIGH_SCORE(score) (gSaveContext.save.info.highScores[score])

Expand Down
4 changes: 2 additions & 2 deletions src/code/z_actor.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1168,7 +1168,7 @@ s32 func_8002DDE4(PlayState* play) {
return player->stateFlags2 & PLAYER_STATE2_3;
}

s32 func_8002DDF4(PlayState* play) {
s32 Player_IsClimbingStill(PlayState* play) {
Player* player = GET_PLAYER(play);

return player->stateFlags2 & PLAYER_STATE2_12;
Expand Down Expand Up @@ -1204,7 +1204,7 @@ void Actor_MountHorse(PlayState* play, Player* player, Actor* horse) {
horse->child = &player->actor;
}

int func_8002DEEC(Player* player) {
int Player_IsDeadOrCutscene(Player* player) {
return (player->stateFlags1 & (PLAYER_STATE1_DEAD | PLAYER_STATE1_29)) ||
(player->csAction != PLAYER_CSACTION_NONE);
}
Expand Down
2 changes: 1 addition & 1 deletion src/overlays/actors/ovl_En_Insect/z_en_insect.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ void EnInsect_Dig(EnInsect* this, PlayState* play) {
if (this->actionTimer <= 0) {
if ((this->insectFlags & INSECT_FLAG_FOUND_SOIL) && this->soilActor != NULL &&
Math3D_Vec3fDistSq(&this->soilActor->actor.world.pos, &this->actor.world.pos) < SQ(8.0f)) {
this->soilActor->unk_152 = 1;
this->soilActor->bugDig = 1;
}
Actor_Kill(&this->actor);
}
Expand Down
49 changes: 31 additions & 18 deletions src/overlays/actors/ovl_En_Si/z_en_si.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,19 @@

#define FLAGS (ACTOR_FLAG_ATTENTION_ENABLED | ACTOR_FLAG_HOOKSHOT_PULLS_ACTOR)

// Params are set by spawning Gold Skulltula
#define ENSI_GS_AREA(this) PARAMS_GET_S(this->actor.params, 8, 5) // mapIndex for current area
#define ENSI_GS_ID(this) PARAMS_GET_S(this->actor.params, 0, 8) // ID flag for this Gold Skulltula

void EnSi_Init(Actor* thisx, PlayState* play);
void EnSi_Destroy(Actor* thisx, PlayState* play);
void EnSi_Update(Actor* thisx, PlayState* play);
void EnSi_Draw(Actor* thisx, PlayState* play);

s32 func_80AFB748(EnSi* this, PlayState* play);
void func_80AFB768(EnSi* this, PlayState* play);
void func_80AFB89C(EnSi* this, PlayState* play);
void func_80AFB950(EnSi* this, PlayState* play);
s32 EnSi_RemoveAC(EnSi* this, PlayState* play);
void EnSi_Action_Idle(EnSi* this, PlayState* play);
void EnSi_Action_HookshotPull(EnSi* this, PlayState* play);
void EnSi_Action_WaitTextbox(EnSi* this, PlayState* play);

static ColliderCylinderInit sCylinderInit = {
{
Expand All @@ -46,7 +50,7 @@ static ColliderCylinderInit sCylinderInit = {
{ 20, 18, 2, { 0, 0, 0 } },
};

static CollisionCheckInfoInit2 D_80AFBADC = { 0, 0, 0, 0, MASS_IMMOVABLE };
static CollisionCheckInfoInit2 sEnSiColliderInfo = { 0, 0, 0, 0, MASS_IMMOVABLE };

ActorProfile En_Si_Profile = {
/**/ ACTOR_EN_SI,
Expand All @@ -65,10 +69,10 @@ void EnSi_Init(Actor* thisx, PlayState* play) {

Collider_InitCylinder(play, &this->collider);
Collider_SetCylinder(play, &this->collider, &this->actor, &sCylinderInit);
CollisionCheck_SetInfo2(&this->actor.colChkInfo, NULL, &D_80AFBADC);
CollisionCheck_SetInfo2(&this->actor.colChkInfo, NULL, &sEnSiColliderInfo);
Actor_SetScale(&this->actor, 0.025f);
this->unk_19C = 0;
this->actionFunc = func_80AFB768;
this->unused_19C = 0;
this->actionFunc = EnSi_Action_Idle;
this->actor.shape.yOffset = 42.0f;
}

Expand All @@ -78,33 +82,36 @@ void EnSi_Destroy(Actor* thisx, PlayState* play) {
Collider_DestroyCylinder(play, &this->collider);
}

s32 func_80AFB748(EnSi* this, PlayState* play) {
s32 EnSi_RemoveAC(EnSi* this, PlayState* play) {

@mzxrules mzxrules Apr 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RemoveAC isn't really an appropriate name.

The way collision_check colliders work is that the actor has to pass them to the collision_check engine by calling CollisionCheck_SetAT/AC/OC every update cycle for them to be "active". Then on next update cycle, the AT-AC collisions and OC collisions are processed, with the results becoming available to the actors through the collider struct.

When CollisionCheck_SetAT/AC/OC is called, the appropriate AT/AC/OC_HIT flags are zeroed on the collider. But you'll often see the flag manually being zeroed immediately after being checked. I believe the reason for this pattern is that there will be cases in some actors where a collider won't always be active, and so you see it get zeroed so that it doesn't get double processed, simplifying logic.

So it's not really that the collider is being "removed". To me it looks like this code might be some incomplete codeblock for detecting hookshot/boomerang collision

@djevangelia djevangelia Apr 8, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I couldn't figure this one out, why remove the AC hit flag. Any name suggestion?

@Dragorn421 Dragorn421 Apr 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest HandleACHit but idk 🤷

if (this->collider.base.acFlags & AC_HIT) {
this->collider.base.acFlags &= ~AC_HIT;
}
return 0;
}

void func_80AFB768(EnSi* this, PlayState* play) {
/**
* Rotate, wait for player to Hookshot or pick up
*/
void EnSi_Action_Idle(EnSi* this, PlayState* play) {
Player* player = GET_PLAYER(play);

if (ACTOR_FLAGS_CHECK_ALL(&this->actor, ACTOR_FLAG_HOOKSHOT_ATTACHED)) {
this->actionFunc = func_80AFB89C;
this->actionFunc = EnSi_Action_HookshotPull;
} else {
Math_SmoothStepToF(&this->actor.scale.x, 0.25f, 0.4f, 1.0f, 0.0f);
Actor_SetScale(&this->actor, this->actor.scale.x);
this->actor.shape.rot.y += 0x400;

if (!Player_InCsMode(play)) {
func_80AFB748(this, play);
EnSi_RemoveAC(this, play);

if (this->collider.base.ocFlags2 & OC2_HIT_PLAYER) {
this->collider.base.ocFlags2 &= ~OC2_HIT_PLAYER;
Item_Give(play, ITEM_SKULL_TOKEN);
player->actor.freezeTimer = 10;
Message_StartTextbox(play, 0xB4, NULL);
Audio_PlayFanfare(NA_BGM_SMALL_ITEM_GET);
this->actionFunc = func_80AFB950;
this->actionFunc = EnSi_Action_WaitTextbox;
} else {
Collider_UpdateCylinder(&this->actor, &this->collider);
CollisionCheck_SetAC(play, &play->colChkCtx, &this->collider.base);
Expand All @@ -114,7 +121,10 @@ void func_80AFB768(EnSi* this, PlayState* play) {
}
}

void func_80AFB89C(EnSi* this, PlayState* play) {
/**
* Pulled by Hookshot towards player, give item when detached
*/
void EnSi_Action_HookshotPull(EnSi* this, PlayState* play) {
Comment on lines +124 to +127

@mzxrules mzxrules Apr 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/**
* Pulled by Hookshot towards player, give item when detached
*/
void EnSi_Action_HookshotPull(EnSi* this, PlayState* play) {
/**
* Pulled by Hookshot or Boomerang towards player, give item when detached
*/
void EnSi_HookshotPull(EnSi* this, PlayState* play) {

Despite the flag names, this should also handle being collected by the boomerang. Additionally, we typically don't use Action to name actionFuncs

@djevangelia djevangelia Apr 8, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add Boomerang comment, forgot about that. I assume the function still can be Hookshot-named like the flag? As changing it to something like FetchToken isn't as clear.

For action functions, I see, is it something decided upon? I'm OK with changing it (especially for En_Si and Obj_Makekinsuta which are so short), but personally I find it so helpful when looking at new actors that the first thing I do is rename all action functions into Actor_Action_address/name to get a clear view of the state flow.

@Dragorn421 Dragorn421 Apr 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah the pattern overall is <ActorName>_<ActionName>, with rare exceptions when the actor is complicated like player

@djevangelia djevangelia Apr 9, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I don't see the positive aspects of cramming everything into ActorName_Function vs separating ActorName_Action_Function and ActorName_Function, where the separation heavily reflects the game logic...
In any case, I'll wait for comments on En_Sw with its many action functions of two different categories.

Player* player = GET_PLAYER(play);

Math_SmoothStepToF(&this->actor.scale.x, 0.25f, 0.4f, 1.0f, 0.0f);
Expand All @@ -126,17 +136,20 @@ void func_80AFB89C(EnSi* this, PlayState* play) {
player->actor.freezeTimer = 10;
Message_StartTextbox(play, 0xB4, NULL);
Audio_PlayFanfare(NA_BGM_SMALL_ITEM_GET);
this->actionFunc = func_80AFB950;
this->actionFunc = EnSi_Action_WaitTextbox;
}
}

void func_80AFB950(EnSi* this, PlayState* play) {
/**
* Wait for textbox to close, then set token as collected and kill actor
*/
void EnSi_Action_WaitTextbox(EnSi* this, PlayState* play) {
Player* player = GET_PLAYER(play);

if (Message_GetState(&play->msgCtx) != TEXT_STATE_CLOSING) {
player->actor.freezeTimer = 10;
} else {
SET_GS_FLAGS(PARAMS_GET_S(this->actor.params, 8, 5), PARAMS_GET_S(this->actor.params, 0, 8));
SET_GS_FLAGS(ENSI_GS_AREA(this), ENSI_GS_ID(this));
Actor_Kill(&this->actor);
}
}
Expand All @@ -153,7 +166,7 @@ void EnSi_Update(Actor* thisx, PlayState* play) {
void EnSi_Draw(Actor* thisx, PlayState* play) {
EnSi* this = (EnSi*)thisx;

if (this->actionFunc != func_80AFB950) {
if (this->actionFunc != EnSi_Action_WaitTextbox) {
func_8002ED80(&this->actor, play, 0);
func_8002EBCC(&this->actor, play, 0);
GetItem_Draw(play, GID_SKULL_TOKEN_2);
Expand Down
2 changes: 1 addition & 1 deletion src/overlays/actors/ovl_En_Si/z_en_si.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ typedef struct EnSi {
/* 0x0000 */ Actor actor;
/* 0x014C */ EnSiActionFunc actionFunc;
/* 0x0150 */ ColliderCylinder collider;
/* 0x019C */ u8 unk_19C;
/* 0x019C */ u8 unused_19C;
} EnSi; // size = 0x01A0

#endif
Loading