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

Remove y/n prompts for save quitting and quitting from the main menu #78379

Merged
merged 3 commits into from
Dec 14, 2024

Conversation

Procyonae
Copy link
Contributor

@Procyonae Procyonae commented Dec 6, 2024

Summary

None

Purpose of change

These prompts don't achieve anything

Describe the solution

Just removes them, leaves the quit without saving (eg on Alt-F4) warning bc losing progress unintentionally as a player would suck

Describe alternatives you've considered

Was originally adding a debug option so contributors could turn all three prompts off

Testing

Additional context

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Dec 6, 2024
@Brambor
Copy link
Contributor

Brambor commented Dec 6, 2024

It is currently here
image

I would move it here
image

Because that is where I look for it.

Also, rename to "Query on ..."

@Brambor
Copy link
Contributor

Brambor commented Dec 6, 2024

Unfortunately doesn't fix my issue:

  1. open crafting GUI
  2. EDIT: open the filter in it too
  3. alt + F4 OR "cross" to quit
  4. Observe: the game still runs
  5. close the filter and the crafting GUI
  6. The game quits

@Procyonae
Copy link
Contributor Author

I would move it here

This isn't exactly an annoying feature unless you're a contributor imo which is why I've stuck it in debug options, it does kind of fit with those other "Query on" ones tho so ig I will

Unfortunately doesn't fix my issue

I mean I weren't aiming to fix any imgui window nonsense

@Procyonae Procyonae force-pushed the DebugOptionRemoveQuitQuery branch from b5705df to 4abbace Compare December 6, 2024 18:17
@Procyonae
Copy link
Contributor Author

Wait crafting GUI isn't imgui facepalms
I don't have that issue it shows the prompt and closes normally for me on today's expimage

@Brambor
Copy link
Contributor

Brambor commented Dec 6, 2024

You are right. It has to be the filter in the crafting GUI. That is not ImGui either AFAIK.

image

@Procyonae Procyonae changed the title Add debug option to skip the y/n prompts for quitting Add option to skip the y/n prompts for quitting Dec 6, 2024
@Procyonae
Copy link
Contributor Author

Procyonae commented Dec 6, 2024

Ye idk I think that's probably a string_editor_window but idk why it would delay the prompt until after you close the entire crafting GUI, pretty weird, either way it has nothing to do with this PR though so I suggest you open an issue

@kevingranade
Copy link
Member

Instead of an option can you do "really quit? Yes/No/Stop Asking"

@Brambor
Copy link
Contributor

Brambor commented Dec 6, 2024

Instead of an option can you do "really quit? Yes/No/Stop Asking"

If this makes it not revertable in options, I disagree.

@Procyonae
Copy link
Contributor Author

Procyonae commented Dec 6, 2024

Instead of an option can you do "really quit? Yes/No/Stop Asking"

Why? Then I have to give it it's own save load handling to store it somewhere even if you're Alt-F4ing (also ye you'd have to config edit to revert it which seems dumb)

@Brambor
Copy link
Contributor

Brambor commented Dec 7, 2024

On Discord I was recommended SuperF4 (for Windows). Works amazing! Just press ctrl+alt+F4 to SIGKILL (I presume).

@kevingranade
Copy link
Member

Why? Then I have to give it it's own save load handling to store it somewhere even if you're Alt-F4ing.

Because were carrying around too many options in the options menu.

@Brambor
Copy link
Contributor

Brambor commented Dec 7, 2024

Why? Then I have to give it it's own save load handling to store it somewhere even if you're Alt-F4ing.

Because were carrying around too many options in the options menu.

I think it is nice to have a section of options dedicated to "Query on". If the issue is it is not manageable, make it a separate section. Would that make it manageable?

@db48x
Copy link
Contributor

db48x commented Dec 7, 2024

Every new configuration option we add makes the maintenance and testing burden higher. In principle it doubles it, because you have to test everything with the option turned on and again with it off, though in practice it’s not quite that bad. I would recommend against adding this option at all; either prompt or don’t prompt. Don’t make it configurable.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Dec 9, 2024
@Procyonae Procyonae force-pushed the DebugOptionRemoveQuitQuery branch from 4abbace to 36eea17 Compare December 11, 2024 16:28
@Procyonae Procyonae changed the title Add option to skip the y/n prompts for quitting Remove y/n prompts for save quitting and quitting from the main menu Dec 11, 2024
@github-actions github-actions bot removed the BasicBuildPassed This PR builds correctly, label assigned by github actions label Dec 11, 2024
@Procyonae Procyonae force-pushed the DebugOptionRemoveQuitQuery branch from 36eea17 to f464be9 Compare December 11, 2024 17:59
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Dec 12, 2024
@Maleclypse Maleclypse merged commit 852bc73 into CleverRaven:master Dec 14, 2024
24 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants