Gold Skulltula/Skullwalltula (En_Sw, En_Si, Obj_Makekinsuta)

[djevangelia]

Documentation of Gold Skulltula and Skullwalltula.
I didn't go into much into the maths of Gold Skulltula positioning and poly selection. I don't understand the bit fixing in EnSw_Init either (Skullwalltula seem to have all param bits set to 0).

[mzxrules]

might do more later.

[mzxrules]

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]

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

[Dragorn421]

I'd suggest HandleACHit but idk 🤷

[mzxrules]

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]

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]

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

[djevangelia]

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.