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

Add linter for g_Player.pl_vram_flag #1851

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

JoshSchreuder
Copy link
Contributor

Noticed this in the header in a comment - decided to write a linter and make this an enum. I think it nicely self-documents a bunch of existing code

@JoshSchreuder JoshSchreuder changed the title Add linter for g_Player.pl_vram_flag Add linter for g_Player.pl_vram_flag Oct 30, 2024
src/dra/6D59C.c Outdated Show resolved Hide resolved
src/dra/75F54.c Outdated Show resolved Hide resolved
src/dra/75F54.c Outdated Show resolved Hide resolved
src/dra/75F54.c Outdated Show resolved Hide resolved
src/dra/8A0A4.c Outdated Show resolved Hide resolved
PLAYER.velocityY = -(abs(PLAYER.velocityX) + FIX(3.5));
}
if ((g_Player.pl_vram_flag & 0xF000) == 0x8000) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a bunch more of these so I'll stop pointing out every single one but it would make sense to document the compare values everywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, looks like the current linter will only handle one of masking or equality and not both. I've fixed these cases up manually

Copy link
Owner

@Xeeynamo Xeeynamo left a comment

Choose a reason for hiding this comment

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

I am reading func_8010C36C and I can observe most of these fields are duplicates of the various EFFECT_UNK_ (see *vram_ptr |= g_Player.colliders[i].effects &). Richter follows the same logic (see func_8015EE28). The function func_80109A44 dictates the horizontal speed based on the slope type.

I think we need to gather more information before merging this change. I believe we're very close as some existing info are matching with what you've found. But I do not want to risk to get this merged in, having the massive changes from linter to then discover we could've done this work differently.

@JoshSchreuder JoshSchreuder marked this pull request as draft November 3, 2024 00:27
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.

3 participants