Documentation pass for the Target system #1281

AngheloAlf · 2023-06-11T16:28:52Z

Documentation pass for the Target system and general cleanups

Name the actor flags ACTOR_FLAG_TARGETABLE, ACTOR_FLAG_FRIENDLY and ACTOR_FLAG_UNFRIENDLY
Introduce a TargetMode enum
Migrate z_actor bss

Something to note is every time either ACTOR_FLAG_FRIENDLY or ACTOR_FLAG_UNFRIENDLY is used, it is used in conjunction with ACTOR_FLAG_TARGETABLE. We may want to define compound flags for those two, maybe something like this:

#define ACTOR_FLGSET_FRIENDLY (ACTOR_FLAG_TARGETABLE | ACTOR_FLAG_FRIENDLY)
#define ACTOR_FLGSET_UNFRIENDLY (ACTOR_FLAG_TARGETABLE | ACTOR_FLAG_UNFRIENDLY)

(Not convinced by FLGSET tho)

Co-authored-by: engineer124 <engineer124engineer124@gmail.com>

engineer124

Sorry for the delay, I really wanted to try and understand targeting from both Player and Navi/Tatl's perspective as well. I have some more thoughts on these names

include/z64actor.h

src/code/z_actor.c

include/z64actor.h

engineer124 · 2023-07-28T00:04:30Z

include/z64actor.h

+// Allows Tatl to fly over the actor and Z-target it
+#define ACTOR_FLAG_TARGETABLE    (1 << 0)


Is Z-target in this context the same as lockOn, or is it the more general case of targeting? It's possible for tatl to fly to an actor because of this flag, but not be able to lockOn which is controlled/blocked by a different actor flag: ACTOR_FLAG_8000000 or ACTOR_FLAG_CANT_LOCK_ON

Exactly. As you said having this flag enables Tatl to fly to an actor and lock-on it. To only allow flying over the actor but not lock-on then both flags must be set

I was referring more to the comment in that ACTOR_FLAG_TARGETABLE alone isn't enough to lockOn. ACTOR_FLAG_8000000 must also be unset for you to be able to lock on

I rewrote this comment a bit, tell me if it is clear now

src/code/z_actor.c

engineer124 · 2023-07-28T00:05:50Z

src/code/z_actor.c

+f32 sTargetableNearestActorDistanceSq;
+f32 sBgmEnemyDistSq;
+f32 D_801ED8D0;
+s32 sTargetableHighestPriorityPriority;


Is the double Priority intended?

Sadly yes, because of this block in Target_FindTargetableActorForCategory

if (isNearestTargetableActor && (actor->targetPriority < sTargetableHighestPriorityPriority)) { sTargetableHighestPriorityActor = actor; sTargetableHighestPriorityPriority = actor->targetPriority; }

The first variable stores the address of the actor with the highest priority, while the second one tracks the priority of the actor with the highest priority. Would be fine to drop the second Priority?

Would something like:
sTargetablePrioritizedActor
sTargetablePrioritizedPriority
Sound less awkward? idk

src/code/z_actor.c

include/z64actor.h

engineer124 · 2023-07-28T00:08:31Z

src/code/z_actor.c

+void Target_SetColors(TargetContext* targetCtx, Actor* actor, ActorType type, PlayState* play) {
+    targetCtx->fairyPos.x = actor->focus.pos.x;
+    targetCtx->fairyPos.y = actor->focus.pos.y + (actor->targetArrowOffset * actor->scale.y);
+    targetCtx->fairyPos.z = actor->focus.pos.z;


This sets more than colors, it also sets position. Could there be a simpler name than Target_SetFairyPosAndColors?

What about Target_SetFairyState?

Yeah, I think that's fine? I've been starting to look at fairy docs and there is something called fairyState (name might change eventually tho), plus this has Target_ to give it context. So it's probably fine

src/code/z_actor.c

engineer124 · 2023-07-28T00:13:20Z

src/code/z_actor.c

+ * `sTargetableNearestActor`. The higher priority / smaller distance of those actors are stored in
+ * `sTargetableHighestPriorityPriority` and `sTargetableNearestActorDistanceSq`.
+ *
+ * Something something, ACTOR_FLAG_40000000, D_801ED8C4/D_801ED8C0, D_801ED8D8/D_801ED8D0.


Suggested change

* Something something, ACTOR_FLAG_40000000, D_801ED8C4/D_801ED8C0, D_801ED8D8/D_801ED8D0.

* There is not much info to infer anything about ACTOR_FLAG_40000000, D_801ED8C4/D_801ED8C0, D_801ED8D8/D_801ED8D0. All appear unused in any meaningful way

Maybe something like this?

src/overlays/actors/ovl_En_Elf/z_en_elf.c

Co-authored-by: engineer124 <47598039+engineer124@users.noreply.github.com>

engineer124 · 2023-07-28T01:45:55Z

src/code/z_actor.c

+                if (actor == params->player->lockOnActor) {
                    actor->isTargeted = true;


Suggested change

if (actor == params->player->lockOnActor) {

actor->isTargeted = true;

if (actor == params->player->lockOnActor) {

actor->isLockedOn = true;

I think?

src/overlays/actors/ovl_En_Elf/z_en_elf.c

engineer124 · 2023-07-28T02:27:22Z

src/overlays/actors/ovl_En_Elf/z_en_elf.c

@@ -1134,12 +1134,12 @@ void func_8088F214(EnElf* this, PlayState* play) {
        sp34 = 1;
        Actor_PlaySfx_Flagged(&this->actor, NA_SE_EV_BELL_ANGER - SFX_FLAG);
    } else {
-        arrowPointedActor = play->actorCtx.targetContext.arrowPointedActor;
+        fairyActor = play->actorCtx.targetCtx.fairyActor;


Suggested change

fairyActor = play->actorCtx.targetCtx.fairyActor;

targetFairyActor = play->actorCtx.targetCtx.fairyActor;

src/overlays/actors/ovl_En_Bigpo/z_en_bigpo.c

engineer124

Great work!

AngheloAlf and others added 30 commits May 31, 2023 17:05

cleanup

f0e4b82

import bss

b529b9b

sActorHiliteMtx

ca68dda

some cleanups on func_800BB604

c39b5b8

Actor_IsInTargetableRange

611fedd

rematch func_800BB604

388cbc0

Co-authored-by: engineer124 <engineer124engineer124@gmail.com>

Name ACTOR_FLAG_UNFRIENDLY and ACTOR_FLAG_FRIENDLY

1eabb66

Rename some Target_ functions

ecae861

cleanusp

26a2837

TargetMode enum

d26bb0d

Target_800B82EC

25ad540

sNaming

dfc4cec

more

3b328c8

more naming

32dd454

fairyHintPos

faa4fea

rotation

e3a1487

ACTOR_FLAG_TARGETABLE

4c58ea2

Merge remote-tracking branch 'upstream/master' into actorcleanup

4103816

update namefixer

e64bae1

remove trailing comma

66864ea

bss

3e75d8f

Wall of text for Target_800BB604

9e4732a

function naming

26ae6f2

Merge remote-tracking branch 'upstream/master' into actorcleanup

be053c2

cleanups

3bff59d

Target_GetAdjustedDistSq

57a0b4f

NotLeash

6956312

more comments

3be1e7c

minor comment

3faa18f

Merge remote-tracking branch 'upstream/master' into actorcleanup

453f17e

AngheloAlf added 2 commits July 10, 2023 10:06

Merge remote-tracking branch 'upstream/master' into actorcleanup

0621833

Merge remote-tracking branch 'upstream/master' into actorcleanup

89e883a

engineer124 reviewed Jul 27, 2023

View reviewed changes

AngheloAlf added 3 commits July 27, 2023 12:34

Merge remote-tracking branch 'upstream/master' into actorcleanup

01302b6

review

fff489c

format

6997a77

engineer124 reviewed Jul 28, 2023

View reviewed changes

src/overlays/actors/ovl_En_Elf/z_en_elf.c Outdated Show resolved Hide resolved

AngheloAlf and others added 3 commits July 27, 2023 20:21

Update src/code/z_actor.c

aaa60d6

Co-authored-by: engineer124 <47598039+engineer124@users.noreply.github.com>

Update include/z64actor.h

c9b5e0b

Co-authored-by: engineer124 <47598039+engineer124@users.noreply.github.com>

review

26ded14

engineer124 reviewed Jul 28, 2023

View reviewed changes

src/overlays/actors/ovl_En_Bigpo/z_en_bigpo.c Outdated Show resolved Hide resolved

AngheloAlf added 2 commits July 28, 2023 18:15

review

c168763

fix

d321007

engineer124 approved these changes Jul 30, 2023

View reviewed changes

engineer124 removed the Needs-second-approval Second approval label Jul 30, 2023

AngheloAlf added 6 commits August 6, 2023 10:13

Merge remote-tracking branch 'upstream/master' into actorcleanup

51f068e

Merge remote-tracking branch 'upstream/master' into actorcleanup

e7183cf

Merge remote-tracking branch 'upstream/master' into actorcleanup

6d8a8fb

fix

7b71e06

Merge remote-tracking branch 'upstream/master' into actorcleanup

a9939bc

Merge remote-tracking branch 'upstream/master' into actorcleanup

de2b79f

jpburnett approved these changes Sep 2, 2023

View reviewed changes

AngheloAlf added Merge-ready All reviewers satisfied, just waiting for CI and removed Needs-first-approval First approval labels Sep 2, 2023

AngheloAlf merged commit 9cceea4 into zeldaret:master Sep 2, 2023
1 check passed

AngheloAlf deleted the actorcleanup branch September 2, 2023 19:16

engineer124 mentioned this pull request Oct 2, 2023

Documentation pass for the Target system zeldaret/oot#1558

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Documentation pass for the Target system #1281

Documentation pass for the Target system #1281

AngheloAlf commented Jun 11, 2023

engineer124 left a comment

engineer124 Jul 28, 2023

AngheloAlf Jul 28, 2023

engineer124 Jul 28, 2023

AngheloAlf Jul 28, 2023

engineer124 Jul 28, 2023

AngheloAlf Jul 28, 2023

engineer124 Jul 28, 2023

engineer124 Jul 28, 2023

AngheloAlf Jul 28, 2023

engineer124 Jul 28, 2023

engineer124 Jul 28, 2023

engineer124 Jul 28, 2023

engineer124 Jul 28, 2023

engineer124 left a comment

		// Allows Tatl to fly over the actor and Z-target it
		#define ACTOR_FLAG_TARGETABLE (1 << 0)

	* Something something, ACTOR_FLAG_40000000, D_801ED8C4/D_801ED8C0, D_801ED8D8/D_801ED8D0.
	* There is not much info to infer anything about ACTOR_FLAG_40000000, D_801ED8C4/D_801ED8C0, D_801ED8D8/D_801ED8D0. All appear unused in any meaningful way

		if (actor == params->player->lockOnActor) {
		actor->isTargeted = true;

	fairyActor = play->actorCtx.targetCtx.fairyActor;
	targetFairyActor = play->actorCtx.targetCtx.fairyActor;

Documentation pass for the Target system #1281

Documentation pass for the Target system #1281

Conversation

AngheloAlf commented Jun 11, 2023

engineer124 left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

engineer124 left a comment

Choose a reason for hiding this comment