`WeaponInfo` docs #1596

Dragorn421 · 2023-12-11T15:11:38Z

Some docs on WeaponInfo and surroundings

Cf Player_UpdateWeaponInfo for an explanation of what this is (see if the doc is good enough)

Major change is renaming "tip" and "base" to "posA" and "posB" respectively.

This is because tip&base only works for melee weapons (swords, stick, hammer), where the two positions do correspond to a tip and a base:

However this falls apart for range weapons which also use WeaponInfo. For example hookshot uses positions that are basically "left and right" (or "top and bottom" idk)

Link to discord discussion (really me venting that bleh posA and posB) https://discord.com/channels/688807550715560050/688851317593997489/1175953325733199922

fig02 · 2023-12-11T15:45:52Z

src/overlays/actors/ovl_En_Arrow/z_en_arrow.c

+    static Vec3f sPosAModel = { 0.0f, 400.0f, 1500.0f };
+    static Vec3f sPosBModel = { 0.0f, -400.0f, 1500.0f };


Currently we dont have a convention for naming these types of vectors. So maybe its something we need to solve outside of this PR. But, suffixing "model" isnt all that common so far for these.

Most often youll see "offset" used. As in, posA is 400 units in the y direction and 1500 units in the z direction from the base position of whatever the relevant entity is.

"offset" makes it sound like it's just an addition though, when in fact the transformation also involves scaling and rotation (or any other linear transform but typically TRS)

It seems more typical/robust to me to specify the coordinates system a position is expressed in, here the "left hand limb (model) space"

I don't like the jumbled mess of words it comes out as either, but I don't really see a better option and apparently this is how I named similar coordinates in this file in the past (e.g. sPlayerFocusHeadLimbModelPos in #1230), it wasn't challenged then (just to say that other stuff would need changing too)

Brainstorming based on sPlayerFocusHeadLimbModelPos

sPlayerFocusPosHeadLimbSpace

sPlayerFocusPos_HeadLimbSpace

sPlayerFocusPos_HeadLimb

🤔

fig02 · 2023-12-11T15:49:25Z

src/code/z_player_lib.c

+// Positions for the tip of melee weapons, in the left hand limb's own model space.
+Vec3f sMeleeWeaponTipLeftHandLimbModelPos0 = { 5000.0f, 400.0f, 0.0f };
+Vec3f sMeleeWeaponTipLeftHandLimbModelPos1 = { 5000.0f, -400.0f, 1000.0f };
+Vec3f sMeleeWeaponTipLeftHandLimbModelPos2 = { 5000.0f, 1400.0f, -1000.0f };

-Vec3f D_801260A4[3] = {
-    { 0.0f, 400.0f, 0.0f },
-    { 0.0f, 1400.0f, -1000.0f },
-    { 0.0f, -400.0f, 1000.0f },
-};
+// Positions for the base of melee weapons, in the left hand limb's own model space.
+Vec3f sMeleeWeaponBaseLeftHandLimbModelPos0 = { 0.0f, 400.0f, 0.0f };
+Vec3f sMeleeWeaponBaseLeftHandLimbModelPos1 = { 0.0f, 1400.0f, -1000.0f };
+Vec3f sMeleeWeaponBaseLeftHandLimbModelPos2 = { 0.0f, -400.0f, 1000.0f };


While the comments help greatly, the variable names themselves are quite confusing for me currently. I initially couldnt tell if this was the position of the left hand or what.

Using "offset" from the previous comment, it would be something like
sMeleeWeaponTipOffsetFromLeftHand0. This tells me that its the position of something relative to something else (and the fact that its in model space is imo clear by its usage instantly, and doesnt need to be in the name)

ariahiro64 · 2024-04-08T00:43:20Z

why not just do sMeleeWeaponLengthPos0 then explain its from the tip in a comment

krm01

I think changing from tip/base to accommodate the hookshot is a mistake. Honestly tip/base still works, and is just plain easier to reason about. From a modding perspective it's immediately obvious which one I need to use, and have used several times. I think changing it to posA/posB is a strict downgrade and obfuscates the WeaponInfo struct further.

also... these names sMeleeWeaponTipLeftHandLimbModelPos0 are just way too much in my opinion, but I'm not as against it as I am the tip/base thing. I think sWeeaponTipOffset0 , etc. is better and simpler. same for the other ones, calling these offsets makes a lot of sense to me. this idea of model space offsets makes a lot a lot a lot of sense to me. I would be happy to see all such Vec3fs used in this way named "offset".

Dragorn421 · 2024-09-22T19:19:33Z

How does tip/base still work for hookshot? They're used by hookshot as "one side" and "the other side"

krm01 · 2024-09-22T19:36:32Z

Maybe it's not perfect, but I also don't think it's wrong, and having to go all the way back to just "pos" feels like an overcorrection. Like... yeah of course it's a pos, we knew that much from the type basically.

fig02 · 2024-09-22T21:26:56Z

Its more than just the hookshot. What is the "tip" of the boomerang, for example.
Even if you want to stretch it and say "tip" is the part thats "facing forward", I'm pretty sure for the boomerang the "tip" is the side closest to link in this case, if I'm reading things right

cadmic · 2024-09-22T21:30:52Z

For hookshot, you can think of it as shooting out a tiny weapon, oriented vertically so the top is the "tip" and the bottom is the "base". Same with boomerang, the leading edge of the hitbox is the true "weapon", although I don't know which way it's oriented (and I wouldn't know if it's named posA/posB either)

fig02 · 2024-09-22T22:02:20Z

Heres the boomerang, with the "tip" that has a green arrow pointing at it (bottom right vertex) and the "base" in red (top right vertex)

And the hookshot, the "tip" in green (top right vertex) and "base" in red (bottom right vertex)
(bad image, see comment below)

These points are set arbitrarily and cannot be named after anything really, which is why they are generically named as "A" and "B". They will need to have meaningful names within the actors themselves.

fig02 · 2024-09-22T22:06:40Z

Sorry that image of the hookshot is a bit misleading cause of how the arrows are oriented/positioned. Heres a similar angle as the boomerang. They are at least both in the front, but they are still flipped between the two use cases

cadmic · 2024-09-22T22:13:03Z

Why can't we give them a suggestive name anyway? I think the base/tip analogy works well if you imagine a little sword sweeping out the path the hookshot or boomerang. It's at least not worse than posA/posB for hookshot etc (note posA/posB also implies some orientation), and it's much better for things like sword and deku stick, so overall I think base/tip are better names.

fig02 · 2024-09-22T22:13:53Z

They are pointing at complete opposite points between hookshot and boomerang, how is tip and base helping here

cadmic · 2024-09-22T22:18:45Z

They don't really, I guess you could imagine the boomerang as being thrown upside-down. But posA and posB also don't help. But base/tip helps a ton when dealing with sword etc, so all things considered I think the names base/tip are more helpful. I don't think we should prefer an always bad name over a name that is bad only some of the time ("bad" in the sense of "not communicating anything to the reader").

fig02 · 2024-09-22T22:23:55Z

The weapon info system is used for more than melee weapons, so I think it should accommodate for that fact. Being generic when things are generic only really makes sense. Tip and base in the other contexts they are used are just wrong.

The pieces relevant only to melee weapons can still be named after tip and base, like the vector that holds their offsets.

fig02 · 2024-09-22T22:38:41Z

Just for completeness sake, here is arrow which match the hookshot in orientation.

fig02 · 2024-09-22T22:48:02Z

@krm01 do you still want to keep tip and base given the information above? You said Honestly tip/base still works, and is just plain easier to reason about But Im assuming you thought they were pointing to the front and back?

cadmic · 2024-09-22T22:48:03Z

I don't think they're "just wrong" for hookshot/bow/boomerang, it would be wrong if for example the hammer used "tip" for the handle and "base" for the head. For those weapons you can think of "base" and "tip" as being generic instead since the orientation doesn't matter

fig02 · 2024-09-22T22:51:38Z

The orientation does matter. I would only be on board if it were like so

cadmic · 2024-09-22T23:05:04Z

I see, that's not how "quad" weapons work though, it sweeps out a path along weapon using the previous base/tip and the current base/tip.

For hookshot, you can think of it as shooting out a tiny weapon, oriented vertically so the top is the "tip" and the bottom is the "base". Same with boomerang, the leading edge of the hitbox is the true "weapon"

I guess you really don't like this analogy? I've internalized it pretty well after working on tricks to hit stuff with a weirdshot or a boomerang throw

krm01 · 2024-09-24T19:54:22Z

Yea after reading the further discussion I still like base / tip because of how useful it is for the sword... other actor's reusing this in a slightly different way isn't enough to make me want to give that up. and as cadmic pointed out, which i agree with, it's easy to simply think of the hookshot/boomerang as a little sword sweeping out.

the sword base / tip just has so many common uses... I reference it all the time for various things, checking the burning stick tip in range, etc... to me, being slightly more "correct" by being more generic and less meaningful for its use in hookshot/boomerang isn't worth the hit to usability/readability for the common use case of dealing with Link's melee weapons.

fig02 · 2024-09-24T20:00:48Z

I think the other uses are being disregarded as small use cases/hacks when 3 different weapons is a significant amount. And I still dont follow how we can call it tip and base when those other weapons dont have a tip or base and dont use it in that manner. Even if you look at the leading edge as a small sword, the tip and base are not used in a meaningful way.

But I am just repeating myself at this point and dont think it will add anything to the conversation.
Some more opinions weighing in would help

AngheloAlf · 2024-09-24T20:07:11Z

What about using an union?
Something like this comes to mind:

typedef struct WeaponInfo {
    /* 0x00 */ s32 active;
    union {
        struct {
        /* 0x04 */ Vec3f tip; // furthest from the player hand
        /* 0x10 */ Vec3f base; // near the player hand
        }  melee;
        struct {
        /* 0x04 */ Vec3f a;
        /* 0x10 */ Vec3f b;
        } other;
    } pos;
} WeaponInfo; // size = 0x1C

fig02 · 2024-09-24T20:18:26Z

trying to lean toward compromise is a nice thought, but dont think it will really help with anything in this case

The main debate here is over what these members will be called within func_80090480. Regardless of these field names, you are free to name the vectors within the actors themselves as tip and base (which is what I have been suggesting).

If a union was used thered be pretty much no place where you would access them as tip and base, because you would want to use the generic member vars in func_800906D4, which is the generic weaponInfo processing function. The values are passed as arguments to this function.

fig02 · 2024-09-24T20:23:16Z

Phrased another way, I have no problem with tip and base terminology being used in func_800906D4, the function that sets this stuff for melee weapons. Within this function, there is no place where the union would see any use. All of the things called "tip" and "base" are newTipPos and newBasePos which are separate from this structure.

WeaponInfo docs

658c0b8

Dragorn421 added Needs first approval Needs second approval labels Dec 11, 2023

fig02 reviewed Dec 11, 2023

View reviewed changes

Dragorn421 added 2 commits February 5, 2024 22:27

Merge branch 'main' into doc_weaponinfo

29572e2

format

be4dc01

Dragorn421 added 6 commits August 19, 2024 14:12

Merge branch 'main' into doc_weaponinfo

cbad0ac

Merge branch 'main' into doc_weaponinfo

b547bbe

disasm metadata

4191a64

Merge branch 'main' into doc_weaponinfo

1ed196a

Merge branch 'main' into doc_weaponinfo

4a60563

Merge branch 'main' into doc_weaponinfo

6cb9d24

krm01 reviewed Sep 22, 2024

View reviewed changes

Dragorn421 added 2 commits September 25, 2024 10:21

Merge branch 'main' into doc_weaponinfo

c4628cf

Merge branch 'main' into doc_weaponinfo

735f336

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`WeaponInfo` docs #1596

`WeaponInfo` docs #1596

Dragorn421 commented Dec 11, 2023 •

edited

Loading

fig02 Dec 11, 2023

Dragorn421 Dec 14, 2023

fig02 Dec 11, 2023

ariahiro64 commented Apr 8, 2024 •

edited

Loading

krm01 left a comment

Dragorn421 commented Sep 22, 2024

krm01 commented Sep 22, 2024

fig02 commented Sep 22, 2024

cadmic commented Sep 22, 2024

fig02 commented Sep 22, 2024 •

edited

Loading

fig02 commented Sep 22, 2024

cadmic commented Sep 22, 2024

fig02 commented Sep 22, 2024

cadmic commented Sep 22, 2024

fig02 commented Sep 22, 2024

fig02 commented Sep 22, 2024

fig02 commented Sep 22, 2024

cadmic commented Sep 22, 2024

fig02 commented Sep 22, 2024

cadmic commented Sep 22, 2024

krm01 commented Sep 24, 2024

fig02 commented Sep 24, 2024

AngheloAlf commented Sep 24, 2024

fig02 commented Sep 24, 2024

fig02 commented Sep 24, 2024

		static Vec3f sPosAModel = { 0.0f, 400.0f, 1500.0f };
		static Vec3f sPosBModel = { 0.0f, -400.0f, 1500.0f };

WeaponInfo docs #1596

Are you sure you want to change the base?

WeaponInfo docs #1596

Conversation

Dragorn421 commented Dec 11, 2023 • edited Loading

fig02 Dec 11, 2023

Choose a reason for hiding this comment

Dragorn421 Dec 14, 2023

Choose a reason for hiding this comment

fig02 Dec 11, 2023

Choose a reason for hiding this comment

ariahiro64 commented Apr 8, 2024 • edited Loading

krm01 left a comment

Choose a reason for hiding this comment

Dragorn421 commented Sep 22, 2024

krm01 commented Sep 22, 2024

fig02 commented Sep 22, 2024

cadmic commented Sep 22, 2024

fig02 commented Sep 22, 2024 • edited Loading

fig02 commented Sep 22, 2024

cadmic commented Sep 22, 2024

fig02 commented Sep 22, 2024

cadmic commented Sep 22, 2024

fig02 commented Sep 22, 2024

fig02 commented Sep 22, 2024

fig02 commented Sep 22, 2024

cadmic commented Sep 22, 2024

fig02 commented Sep 22, 2024

cadmic commented Sep 22, 2024

krm01 commented Sep 24, 2024

fig02 commented Sep 24, 2024

AngheloAlf commented Sep 24, 2024

fig02 commented Sep 24, 2024

fig02 commented Sep 24, 2024

`WeaponInfo` docs #1596

`WeaponInfo` docs #1596

Dragorn421 commented Dec 11, 2023 •

edited

Loading

ariahiro64 commented Apr 8, 2024 •

edited

Loading

fig02 commented Sep 22, 2024 •

edited

Loading