Documentation pass for the Target system #1558

engineer124 · 2023-10-02T00:57:45Z

This primarily focuses on the parts of the Targeting system handled in z_actor. All these docs are based on discussions we had in MM around this PR led by AngheloAlf: zeldaret/mm#1281

fig02

not a complete review, just some high level initial thoughts

include/z64actor.h

src/overlays/actors/ovl_player_actor/z_player.c

Dragorn421

😏 🏹 🎯
this system sure is complicated... and there's still a chunk in player bruh

include/z64actor.h

src/code/z_actor.c

engineer124 · 2023-10-03T12:10:25Z

😏 🏹 🎯 this system sure is complicated... and there's still a chunk in player bruh

Yup 😏 here's some wip rough docs of the rest, including the player targeting code:
https://github.com/engineer124/oot/compare/zTarget...engineer124:oot:zTargetFull?expand=1

src/code/z_actor.c

engineer124 · 2023-11-20T03:34:28Z

src/code/z_actor.c

-void func_8002BE98(TargetContext* targetCtx, s32 actorCategory, PlayState* play) {
-    TargetContextEntry* entry;
-    NaviColor* naviColor;
+void Target_InitReticle(TargetContext* targetCtx, s32 actorCategory, PlayState* play) {


Also went for Target_InitReticle instead of Target_InitLockOn and did some more LockOn -> Reticle in general.

Would be nice if this name would mirror Target_SetNaviState as well?

fig02 · 2023-11-22T10:11:12Z

include/z64actor.h

+    /* 0x28 */ Color_RGBAf naviOuterColor;
+    /* 0x38 */ Actor* naviHoverActor; // The actor that Navi hovers over
+    /* 0x3C */ Actor* lockOnActor;
+    /* 0x40 */ f32 naviMoveProgressFactor; // Controls Navi so she can smootly transition to the target actor
+    /* 0x44 */ f32 reticleRadius; // Control the reticle coming in from offscreen when you first target
+    /* 0x48 */ s16 reticleFadeAlphaControl; // Set and fade the reticle alpha. Also controls the reticle drawing with non-zero values.
+    /* 0x4A */ u8 naviActorCategory;
+    /* 0x4B */ u8 rotZTick;
+    /* 0x4C */ s8 lockOnIndex;
+    /* 0x50 */ LockOnReticle lockOnReticles[3];
+    /* 0x8C */ Actor* forcedTargetActor; // Never set to non-NULL
+    /* 0x90 */ Actor* bgmEnemy; // The nearest enemy to player with the right flags that will trigger NA_BGM_ENEMY
+    /* 0x94 */ Actor* arrowPointedActor;
+} TargetContext; // size = 0x98


Suggested change

/* 0x28 */ Color_RGBAf naviOuterColor;

/* 0x38 */ Actor* naviHoverActor; // The actor that Navi hovers over

/* 0x3C */ Actor* lockOnActor;

/* 0x40 */ f32 naviMoveProgressFactor; // Controls Navi so she can smootly transition to the target actor

/* 0x44 */ f32 reticleRadius; // Control the reticle coming in from offscreen when you first target

/* 0x48 */ s16 reticleFadeAlphaControl; // Set and fade the reticle alpha. Also controls the reticle drawing with non-zero values.

/* 0x4A */ u8 naviActorCategory;

/* 0x4B */ u8 rotZTick;

/* 0x4C */ s8 lockOnIndex;

/* 0x50 */ LockOnReticle lockOnReticles[3];

/* 0x8C */ Actor* forcedTargetActor; // Never set to non-NULL

/* 0x90 */ Actor* bgmEnemy; // The nearest enemy to player with the right flags that will trigger NA_BGM_ENEMY

/* 0x94 */ Actor* arrowPointedActor;

} TargetContext; // size = 0x98

/* 0x28 */ Color_RGBAf naviOuterColor;

/* 0x38 */ Actor* naviHoverActor; // The actor that Navi hovers over

/* 0x3C */ Actor* lockOnActor;

/* 0x40 */ f32 naviMoveProgressFactor; // Controls Navi so she can smootly transition to the target actor

/* 0x44 */ f32 reticleRadius; // Control the reticle coming in from offscreen when you first target

/* 0x48 */ s16 reticleFadeAlphaControl; // Set and fade the reticle alpha. Also controls the reticle drawing with non-zero values.

/* 0x4A */ u8 naviHoverActorCategory; // category of the actor Navi is currently hovering over

/* 0x4B */ u8 reticleSpinCounter; // counts up when a reticle is active, used for z rotation of reticle

/* 0x4C */ s8 lockOnIndex;

/* 0x50 */ LockOnReticle lockOnReticles[3];

/* 0x8C */ Actor* forcedTargetActor; // Not used, always set to NULL

/* 0x90 */ Actor* bgmEnemy; // The nearest enemy to player with the right flags that will trigger NA_BGM_ENEMY

/* 0x94 */ Actor* arrowHoverActor; // actor a target arrow is currently hovering over

} TargetContext; // size = 0x98

Open for discussions ofc, but this is the direction id like to see some of these names move in

All of these seem good to me

fig02 · 2023-11-22T10:15:15Z

src/code/z_actor.c

+            // Use multiple triangle sets for the movement effect when the triangles are
+            // getting closer to the actor from the margin of the screen


maybe "Use multiple reticles for the effect of the reticle quickly zooming in on an actor from off screen"

fig02 · 2023-11-22T10:15:59Z

src/code/z_actor.c

+                    // Draw the 4 lock-on triangles
+                    for (triangleIndex = 0; triangleIndex < 4; triangleIndex++) {


"Draw 4 triangles that make up the reticle", or something to that effect

fig02 · 2023-11-22T10:16:54Z

src/code/z_actor.c

+            if (lockOnActor->id == ACTOR_EN_BOOM) {
+                // Avoid drawing the lock on triangles on the boomerang


"Don't draw reticle when locked onto the boomerang"

fig02 · 2023-11-22T10:18:05Z

src/code/z_actor.c

        } else {
-            targetCtx->unk_4B = (targetCtx->unk_4B + 3) | 0x80;
-            targetCtx->unk_44 = 120.0f;
+            // 0x80 is or'd to avoid getting this value be set to zero


"0x80 is or'd to avoid a value of zero", or something. as is the sentence doesnt read right

fig02 · 2023-11-22T10:18:40Z

src/code/z_actor.c

+        // Linear scaling, yaw being 90 degree means it will return the original distance, 0 degree will adjust to 60%
+        // of the distance


degree -> degrees

fig02 · 2023-11-22T10:22:04Z

src/code/z_actor.c

+void Target_FindTargetableActorForCategory(PlayState* play, ActorContext* actorCtx, Player* player, u32 actorCategory) {
+    f32 distSq;


So just to make sure, when this system calls an actor "targetable", that means it is either lock-able, or hover-able?

I feel like this is going to be a common place of confusion moving forward, with most people assuming "targetable" means "lockable", cause thats how the term is commonly used. But I guess theres not much we can do about it. I dont have any ideas for a different term.

Looking at it again, it's actually very specifically used for "hover-able".

Target_FindTargetableActorForCategory is used specifically for Target_GetTargetActor and no where else, and Target_GetTargetActor is used to get the actor that's set to targetCtx->arrowHoverActor. Afterwards, actor in that function gets overwritten with something else so it's not used for any other purpose. It's just that arrowHoverActor then gets used for other purposes later to allow an actor to be locked-on

if ((player->unk_664 != NULL) && (player->unk_84B[player->unk_846] == 2)) { targetCtx->arrowHoverActor = NULL; } else { Target_GetTargetActor(play, &play->actorCtx, &actor, player); targetCtx->arrowHoverActor = actor; }

So maybe:
Target_GetTargetActor -> Target_GetArrowHoverActor
Target_FindTargetableActorForCategory -> Target_FindArrowHoverActorForCategory

But then

Actor* sTargetableNearestActor; Actor* sTargetablePrioritizedActor; f32 sTargetableNearestActorDistSq; s32 sTargetablePrioritizedPriority;

Should all change to ArrowHoverActor?

I was thinking, maybe we shouldnt be so set on this being the "arrow" actor, and more on the fact that its the next lock-able actor?
Does the arrow appear above things that are not lockable?

video helps get the point across better. this "current" and "next" lockable actor interaction is a pretty important concept to get across, and I dont think the current "arrow" naming helps with this. but Ive not investigated all of the code to see if the way it ends up working in practice translates to these functions well

2023-11-24.23-36-14.mp4

engineer124 added 6 commits October 2, 2023 10:31

Begin Target Docs

29d366f

more cleanup

db98ae9

small fix

b91cc93

seperate extra navi color

5e0c9bd

rm comment

0dc9357

remove unused var

77138d0

fig02 reviewed Oct 2, 2023

View reviewed changes

include/z64actor.h Outdated Show resolved Hide resolved

include/z64actor.h Outdated Show resolved Hide resolved

src/overlays/actors/ovl_player_actor/z_player.c Outdated Show resolved Hide resolved

engineer124 added 4 commits October 3, 2023 10:14

PR Review

28d8885

fix comment

eccfb54

adjust comment

1ac99c4

revert unk_664

4ddce98

Dragorn421 reviewed Oct 3, 2023

View reviewed changes

include/z64actor.h Outdated Show resolved Hide resolved

include/z64actor.h Outdated Show resolved Hide resolved

src/code/z_actor.c Outdated Show resolved Hide resolved

src/code/z_actor.c Show resolved Hide resolved

AngheloAlf reviewed Oct 3, 2023

View reviewed changes

src/code/z_actor.c Outdated Show resolved Hide resolved

src/code/z_actor.c Outdated Show resolved Hide resolved

fig02 added Needs first approval Needs second approval labels Oct 3, 2023

Dragorn421 added the Waiting for author There are requested changes that have not been addressed label Oct 15, 2023

engineer124 added 3 commits November 20, 2023 14:08

merge main

81dbb96

PR Review

6fcd74e

typo

20472fb

engineer124 commented Nov 20, 2023

View reviewed changes

fig02 removed the Waiting for author There are requested changes that have not been addressed label Nov 21, 2023

fig02 reviewed Nov 22, 2023

View reviewed changes

engineer124 added 2 commits November 25, 2023 15:04

Merge branch 'main' into zTarget

f595d0d

partial PR Review

86a68fb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Documentation pass for the Target system #1558

Documentation pass for the Target system #1558

engineer124 commented Oct 2, 2023

fig02 left a comment

Dragorn421 left a comment

engineer124 commented Oct 3, 2023

engineer124 Nov 20, 2023

fig02 Nov 22, 2023

engineer124 Nov 25, 2023

fig02 Nov 22, 2023

fig02 Nov 22, 2023

fig02 Nov 22, 2023

fig02 Nov 22, 2023

fig02 Nov 22, 2023

fig02 Nov 22, 2023

engineer124 Nov 25, 2023

fig02 Nov 25, 2023

fig02 Nov 25, 2023

		// Use multiple triangle sets for the movement effect when the triangles are
		// getting closer to the actor from the margin of the screen

		// Draw the 4 lock-on triangles
		for (triangleIndex = 0; triangleIndex < 4; triangleIndex++) {

		if (lockOnActor->id == ACTOR_EN_BOOM) {
		// Avoid drawing the lock on triangles on the boomerang

		// Linear scaling, yaw being 90 degree means it will return the original distance, 0 degree will adjust to 60%
		// of the distance

		void Target_FindTargetableActorForCategory(PlayState* play, ActorContext* actorCtx, Player* player, u32 actorCategory) {
		f32 distSq;

Documentation pass for the Target system #1558

Are you sure you want to change the base?

Documentation pass for the Target system #1558

Conversation

engineer124 commented Oct 2, 2023

fig02 left a comment

Choose a reason for hiding this comment

Dragorn421 left a comment

Choose a reason for hiding this comment

engineer124 commented Oct 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment