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

Change Screenshot Functionality #7431

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

kphoenix137
Copy link
Collaborator

@kphoenix137 kphoenix137 commented Sep 25, 2024

Removes:

  • Red screen
  • 300 ms pause
  • Gamma adjustment from output png/pcx

Adds:

  • Sound effect

@StephenCWills
Copy link
Member

StephenCWills commented Sep 25, 2024

The other downside is that the chat message will linger on screen much longer than the red palette.

EDIT: The other other downside is that it may interfere when taking screenshots of conversations in chat.

EDIT2: And potentially exposing personal info in the screenshot.

@AJenbo
Copy link
Member

AJenbo commented Sep 25, 2024

Cutting it down to just the file name or relative path, or no file name is other options.

We could also use the info box that's also used for shrines and loading rather then the chat, that way it's not affecting the chat.

@kphoenix137
Copy link
Collaborator Author

Cutting it down to just the file name or relative path, or no file name is other options.

We could also use the info box that's also used for shrines and loading rather then the chat, that way it's not affecting the chat.

Absolutely. I think that DiabloMsg might not be a good idea because it's kind of annoying. I think anything new introduced text based to the user should be chat based rather than DiabloMsg for that reason.

@kphoenix137
Copy link
Collaborator Author

Removed the file path from the chat message

Source/capture.cpp Outdated Show resolved Hide resolved
@StephenCWills
Copy link
Member

I think that DiabloMsg might not be a good idea because it's kind of annoying.

Maybe so, but at least it can be dismissed. It does the player no good to keep the screenshot message hanging around in their chat log.

@kphoenix137
Copy link
Collaborator Author

I think that DiabloMsg might not be a good idea because it's kind of annoying.

Maybe so, but at least it can be dismissed. It does the player no good to keep the screenshot message hanging around in their chat log.

We could add a bool that prevents it from being added to the chat log (the one you get with the hotkey)

@FitzRoyX
Copy link

Instead of displaying text, you could play an unused sound effect like vtheft.wav

@kphoenix137
Copy link
Collaborator Author

Instead of displaying text, you could play an unused sound effect like vtheft.wav

Yeah we could do that.

@kphoenix137
Copy link
Collaborator Author

Updated

Copy link
Member

@StephenCWills StephenCWills left a comment

Choose a reason for hiding this comment

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

There's a warning here says you're not using the palette variable anymore in CaptureScreen().

@StephenCWills
Copy link
Member

Since we decided to remove the on-screen message, I was kind of hoping maybe we could use a camera shutter sound effect to make it more obvious. Here is one I found that sounds pretty good to me. It might work with a bit of editing. Released under the cc0 license so it's public domain.

https://freesound.org/people/mrrap4food/sounds/619062/

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.

4 participants