Skip to content
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

Match Interactable 086 #74

Merged
merged 60 commits into from
May 4, 2023
Merged

Match Interactable 086 #74

merged 60 commits into from
May 4, 2023

Conversation

JaceCear
Copy link
Collaborator

@JaceCear JaceCear commented May 4, 2023

No description provided.

@@ -26,7 +26,7 @@ typedef struct {
} Sprite_NoteParticle; /* size: 0x4C */

/* {anim, variant, tileCount, shouldAllocTileMem(?), (unk | priority)} */
static const u16 gUnknown_080E0140[4][5] = {
static const ALIGNED(4) u16 gUnknown_080E0140[4][5] = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still needed?

Copy link
Collaborator Author

@JaceCear JaceCear May 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, since there's 2 zero-bytes that are removed without explicitly aligning it.
Alternatively I could add 0s to the previous array, but then we wouldn't be able to use ARRAY_COUNT with it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my head, this shows it should be a struct array? As I think struct arrays align 4?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, using a struct sadly doesn't match.

{ 8, 32 },
{ 16, 32 },
{ 32, 64 },
};

void sub_80051E8(Sprite *sprite)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is like ApplyEffects?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That name's too general for a global procedure.
Also, I don't think "effects" is right? How did you infer that name?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not saying that should be the name. This functions is generally called with the sprite RenderAnimationFrame function call.

Maybe you are right that it's not effects. When I say effects I am referring to unk10 which is like a flag of effects which apply to the sprite (though this could also be flags) when this function is called the OamData of the sprite is updated with the result of effects currently applied to the sprite? rotation etc?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.
Well it rather is "attributes" than anything else. OAM is part of the GBA and DS, as well as some other platforms iirc,, so for this, maybe ConvertSpriteFlagsToOamAttributes/ToOamData. Maybe a little shorter than that! :D

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it not Sprite_ApplyOamAttributes(...)


gCurTask->main = Task_807D0C4;
}
#else
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my changes were undone from last night. You should remove the NON_MATCHING blocks from this second function

Copy link
Collaborator

@freshollie freshollie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

/* 0x20 */ u16 quarterWidth;

// unused
/* 0x22 */ u8 padding22[2];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to delete this. The pointer after this will align to 4

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah already tried that before, without issue.
Just wasn't sure whether we want to keep padding for documentation purposes or not! :D

@JaceCear JaceCear merged commit 6c37a49 into main May 4, 2023
github-actions bot pushed a commit that referenced this pull request May 4, 2023
@JaceCear JaceCear deleted the ia_086 branch May 4, 2023 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants