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

Update comments and labels in src/engine/input_name.asm #152

Merged
merged 4 commits into from
Aug 20, 2024

Conversation

Sha0den
Copy link
Contributor

@Sha0den Sha0den commented Aug 9, 2024

I thought that it might be beneficial to incorporate some of the changes that I'm making in poketcg_v2 to the base disassembly, and this file seemed like a good place to start since it contained several unlabeled functions that were very similar to some other functions that were already labeled.

; checks if any buttons were pressed and handles the input.
; returns carry if either the A button or the B button were pressed.
; this function is similar to 'CheckButtonState_DeckNamingScreen'.
CheckButtonState_PlayerNamingScreen:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not compliant to the Style Guide: https://github.com/pret/poketcg/blob/master/STYLE_GUIDE.md

Rather name it PlayerNamingScreen_CheckButtonState


Func_1aa07:
DrawInvisibleCursor_PlayerNamingScreen:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather name it PlayerNamingScreen_DrawInvisibleCursor

; a = which tile to draw
; [wNamingScreenCursorX] = cursor's x position on the keyboard screen
; [wNamingScreenCursorY] = cursor's y position on the keyboard screen
DrawCursor_PlayerNamingScreen:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather name it PlayerNamingScreen_DrawCursor

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also if this isn't used outside the scope of this routine, why use it as a global label?

call WriteByteToBGMap0
or a
ret

Func_1aa23:
DrawVisibleCursor_PlayerNamingScreen:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather name it PlayerNamingScreen_DrawVisibleCursor

; if "End" was selected.
NamingScreen_ProcessInput:
; returns carry if "End" was selected on the keyboard
ProcessInput_PlayerNamingScreen:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather name it PlayerNamingScreen_ProcessInput

; l = y position
; output:
; hl = KeyboardData_PlayerNamingScreen pointer
GetCharInfoFromPos_PlayerNamingScreen:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather name it PlayerNamingScreen_GetCharInfoFromPos

@@ -795,7 +851,7 @@ MACRO kbitem
ENDC
ENDM

KeyboardData_Player:
KeyboardData_PlayerNamingScreen:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather name it PlayerNamingScreen_KeyboardData


Func_1afa1:
DrawInvisibleCursor_DeckNamingScreen:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same styling comments apply here

; a = which tile to draw
; [wNamingScreenCursorX] = cursor's x position on the keyboard screen
; [wNamingScreenCursorY] = cursor's y position on the keyboard screen
DrawCursor_DeckNamingScreen:
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this isn't used outside the scope of this routine, why use it as a global label?

@ElectroDeoxys ElectroDeoxys merged commit 6538bde into pret:master Aug 20, 2024
1 check passed
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