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 new game message hint #82

Merged
merged 1 commit into from
Nov 14, 2023
Merged

Conversation

Diordany
Copy link
Contributor

The new game message didn't give a hint as to which button should be pressed to confirm or cancel. This message has been present since the original release by id software, and I thought it helped to simply add "y/n" at the end of the message for anyone who isn't familiar with how the modal message works.

Copy link
Contributor

@alexey-lysiuk alexey-lysiuk left a comment

Choose a reason for hiding this comment

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

Other modal messages use (y/n) instead of just y/n.

Also, please avoid white-space only changes. It should be only one line modified instead of four.

@Diordany
Copy link
Contributor Author

Diordany commented Nov 14, 2023

Yes sorry about that @alexey-lysiuk, my IDE's editor automatically changed the other lines. I'll take care of it.

Also, do you want me to change y/n to (y/n) instead?

@alexey-lysiuk
Copy link
Contributor

Also, do you want me to change y/n to (y/n) instead?

Let's wait for another opinion. For me, it should match with the rest of SCR_ModalMessage() calls.

@Diordany
Copy link
Contributor Author

Let's wait for another opinion. For me, it should match with the rest of SCR_ModalMessage() calls.

Alright, got it.

@sezero
Copy link
Owner

sezero commented Nov 14, 2023

Also, do you want me to change y/n to (y/n) instead?

That would be good, yes.

@sezero
Copy link
Owner

sezero commented Nov 14, 2023

And: squash the commits into a single one, please.

@Diordany
Copy link
Contributor Author

I've made the changes @sezero.

@sezero sezero merged commit cac649d into sezero:master Nov 14, 2023
6 checks passed
@sezero
Copy link
Owner

sezero commented Nov 14, 2023

OK, this is in. Thanks.

@Diordany Diordany deleted the feat-newgame-msg branch November 28, 2023 15:24
@Diordany Diordany restored the feat-newgame-msg branch November 30, 2023 10:52
@Diordany Diordany deleted the feat-newgame-msg branch February 19, 2024 14:51
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