-
Notifications
You must be signed in to change notification settings - Fork 611
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.
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
WeaponInfo
docs
#1596
base: main
Are you sure you want to change the base?
WeaponInfo
docs
#1596
Changes from 9 commits
658c0b8
29572e2
be4dc01
cbad0ac
b547bbe
4191a64
1ed196a
4a60563
6cb9d24
c4628cf
735f336
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -414,36 +414,36 @@ void EnArrow_Update(Actor* thisx, PlayState* play) { | |
} | ||
|
||
void func_809B4800(EnArrow* this, PlayState* play) { | ||
static Vec3f D_809B4E88 = { 0.0f, 400.0f, 1500.0f }; | ||
static Vec3f D_809B4E94 = { 0.0f, -400.0f, 1500.0f }; | ||
static Vec3f sPosAModel = { 0.0f, 400.0f, 1500.0f }; | ||
static Vec3f sPosBModel = { 0.0f, -400.0f, 1500.0f }; | ||
Comment on lines
+418
to
+419
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "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. Brainstorming based on
🤔 |
||
static Vec3f D_809B4EA0 = { 0.0f, 0.0f, -300.0f }; | ||
Vec3f sp44; | ||
Vec3f sp38; | ||
Vec3f posA; | ||
Vec3f posB; | ||
s32 addBlureVertex; | ||
|
||
Matrix_MultVec3f(&D_809B4EA0, &this->unk_21C); | ||
|
||
if (EnArrow_Fly == this->actionFunc) { | ||
Matrix_MultVec3f(&D_809B4E88, &sp44); | ||
Matrix_MultVec3f(&D_809B4E94, &sp38); | ||
Matrix_MultVec3f(&sPosAModel, &posA); | ||
Matrix_MultVec3f(&sPosBModel, &posB); | ||
|
||
if (this->actor.params <= ARROW_SEED) { | ||
addBlureVertex = this->actor.params <= ARROW_LIGHT; | ||
|
||
if (this->hitActor == NULL) { | ||
addBlureVertex &= func_80090480(play, &this->collider, &this->weaponInfo, &sp44, &sp38); | ||
addBlureVertex &= Player_UpdateWeaponInfo(play, &this->collider, &this->weaponInfo, &posA, &posB); | ||
} else { | ||
if (addBlureVertex) { | ||
if ((sp44.x == this->weaponInfo.tip.x) && (sp44.y == this->weaponInfo.tip.y) && | ||
(sp44.z == this->weaponInfo.tip.z) && (sp38.x == this->weaponInfo.base.x) && | ||
(sp38.y == this->weaponInfo.base.y) && (sp38.z == this->weaponInfo.base.z)) { | ||
if ((posA.x == this->weaponInfo.posA.x) && (posA.y == this->weaponInfo.posA.y) && | ||
(posA.z == this->weaponInfo.posA.z) && (posB.x == this->weaponInfo.posB.x) && | ||
(posB.y == this->weaponInfo.posB.y) && (posB.z == this->weaponInfo.posB.z)) { | ||
addBlureVertex = false; | ||
} | ||
} | ||
} | ||
|
||
if (addBlureVertex) { | ||
EffectBlure_AddVertex(Effect_GetByIndex(this->effectIndex), &sp44, &sp38); | ||
EffectBlure_AddVertex(Effect_GetByIndex(this->effectIndex), &posA, &posB); | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)