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

Document enum PauseState game over parts #2283

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Feacur
Copy link

@Feacur Feacur commented Nov 3, 2024

renamed a set of pause states related to game over sequence

P.S.:
hi, new here 🌞

thank you for the titanic work you've accomplished so far! i've been rummaging around and studying the repo and thought to try and join

also i see there's a quite old kaleido_scope PR, which explicitly didn't touch game over pause states, thus this one should not conflict; much at least

also, as previously, i've rerun the "make" to check integrity
in case it helps to rename them later, if the need ever arises
also i've noticed, that `z64pause.h` doesn't comply fully with the `.clang-format`

temporary changing a couple of setting to
- ColumnLimit: 0
- AlignTrailingComments: false
fixes the issue. i don't want to push unrelated formatting here
Copy link
Collaborator

@Dragorn421 Dragorn421 left a comment

Choose a reason for hiding this comment

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

Nice :)
Yeah this doesn't conflict with/duplicates any pause docs I've done, I didn't touch game over

src/code/z_kaleido_scope_call.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@fig02 fig02 left a comment

Choose a reason for hiding this comment

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

Thanks for this.
Have thoughts on a few names, put some suggestions for discussion

/* 15 */ PAUSE_STATE_15,
/* 16 */ PAUSE_STATE_16,
/* 17 */ PAUSE_STATE_17,
/* 8 */ PAUSE_STATE_GAME_OVER_REQUEST,
Copy link
Collaborator

@fig02 fig02 Nov 4, 2024

Choose a reason for hiding this comment

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

PAUSE_STATE_GAME_OVER_REQUEST

Not sure request totally fits here. To me when I hear request Im expecting a request to be sent somewhere, processed, and then a response sent back. This just sets a couple things and moves on to the next stage instantly.

My gut instinct would be to call this "init", but I see that state 10 is also called init, and it has more symmetry with the pause menu's init. So maybe just "_START", idk. thoughts @Dragorn421

Copy link
Author

@Feacur Feacur Nov 4, 2024

Choose a reason for hiding this comment

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

there's a pattern between them

PAUSE_STATE_WAIT_LETTERBOX // start
PAUSE_STATE_WAIT_BG_PRERENDER // wait single frame

PAUSE_STATE_GAME_OVER_START
PAUSE_STATE_GAME_OVER_WAIT_BG_PRERENDER

but normal pause counterpart is currently called PAUSE_STATE_WAIT_LETTERBOX...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Its called "wait letter box" because it is waiting for this condition to be met
if (Letterbox_GetSize() == 0) {
Youll notice that in the game over variant there is no condition like that, it doesnt have to wait for anything. Its just initializing a few things right away and then continuing.

Copy link
Author

@Feacur Feacur Nov 4, 2024

Choose a reason for hiding this comment

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

may i suggest renaming

PAUSE_STATE_WAIT_LETTERBOX -> PAUSE_STATE_START

so that it matches with potential PAUSE_STATE_GAME_OVER_START?

because normal pause STARTS with the following code

// z_kaleido_setup.c
// if (CHECK_BTN_ALL(input->press.button, BTN_START))
// pauseCtx->state = PAUSE_STATE_WAIT_LETTERBOX;
pauseCtx->state = PAUSE_STATE_START;

current name was based off of the comment

/*  1 */ PAUSE_STATE_WAIT_LETTERBOX, // Request no letterboxing and wait for it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Im not sure symmetry is strictly required here. In the pause case, this state is spending most of its time waiting for the letterbox state to update appropriately. I think it makes sense to name if after this. Meanwhile, the gameover equivalent does not have to wait for anything in this manner.

More opinions welcome though, I believe @Dragorn421 you named the pause ones initially

Copy link
Author

Choose a reason for hiding this comment

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

got it

include/z64pause.h Outdated Show resolved Hide resolved
include/z64pause.h Outdated Show resolved Hide resolved
include/z64pause.h Outdated Show resolved Hide resolved
include/z64pause.h Outdated Show resolved Hide resolved
src/code/z_kaleido_scope_call.c Outdated Show resolved Hide resolved
reverified with
> `check_format.py ...`
> `make ...`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants