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

Improved screenshot plugin + screenshot effects in screenshot / after state change fix #4082

Open
wants to merge 25 commits into
base: develop
Choose a base branch
from

Conversation

Lasercar
Copy link
Contributor

@Lasercar Lasercar commented Feb 1, 2025

Does this PR close any issues? If so, link them below.

Closes #3765, fixes #2811, fixes #2284

Briefly describe the issue(s) fixed.

The screenshot preview isn't hidden before a bitmap of the screen is taken, now it has it's alpha set to zero right before the bitmap is taken.
The screenshot plugin in general.

Ok, that should be the final version. All the issues are fixed now.

Include any relevant screenshots or videos.

First version
ayo.it.s.kinda.fixed.mp4

screenshot-2025-02-02-01-24-27
screenshot-2025-02-02-01-24-28
screenshot-2025-02-02-01-24-29
screenshot-2025-02-02-01-24-30

This doesn't fix screenshot spamming completely, so I'm going to work on this a bit more to see if I can get it to not still break, maybe by making it hold off on showing the preview and cursor again if the screenshot button is constantly pressed.

Second version
2025-02-02.16-59-12.mp4

screenshot-2025-02-02-03-25-40
screenshot-2025-02-02-16-59-29
screenshot-2025-02-02-16-59-30 (2)
screenshot-2025-02-02-16-59-30
screenshot-2025-02-02-16-59-31 (2)
screenshot-2025-02-02-16-59-31
screenshot-2025-02-02-16-59-32 (2)
screenshot-2025-02-02-16-59-32

Third version
2025-02-02.20-53-42.1.mp4

screenshot-2025-02-02-20-54-03
screenshot-2025-02-02-20-54-04 (2)
screenshot-2025-02-02-20-54-04
screenshot-2025-02-02-20-54-05
screenshot-2025-02-02-20-54-05 (2)

Fourth version - Epilepsy Warning

Epilepsy Warning (just in case you missed the first one)

Ayo.finally.fixed.mp4

22 Screenshots (most are taken on the same second), the encoding and saving of each shot is spaced out one second:
screenshot-2025-02-06-23-42-24
screenshot-2025-02-06-23-42-24 (2)
screenshot-2025-02-06-23-42-25
screenshot-2025-02-06-23-42-25 (2)
screenshot-2025-02-06-23-42-25 (3)
screenshot-2025-02-06-23-42-25 (4)
screenshot-2025-02-06-23-42-25 (5)
screenshot-2025-02-06-23-42-25 (6)
screenshot-2025-02-06-23-42-25 (7)
screenshot-2025-02-06-23-42-25 (8)
screenshot-2025-02-06-23-42-25 (9)
screenshot-2025-02-06-23-42-26
screenshot-2025-02-06-23-42-26 (2)
screenshot-2025-02-06-23-42-26 (3)
screenshot-2025-02-06-23-42-26 (4)
screenshot-2025-02-06-23-42-26 (5)
screenshot-2025-02-06-23-42-26 (6)
screenshot-2025-02-06-23-42-27
screenshot-2025-02-06-23-42-27 (2)
screenshot-2025-02-06-23-42-27 (3)
screenshot-2025-02-06-23-42-27 (4)
screenshot-2025-02-06-23-42-27 (5)

Final version - Epilepsy Warning

Epilepsy Warning (just in case you missed the first one)

2025-02-16.22-29-14.mp4

@github-actions github-actions bot added status: pending triage Awaiting review. size: small A small pull request with 10 or fewer changes. pr: haxe PR modifies game code. and removed size: small A small pull request with 10 or fewer changes. labels Feb 1, 2025
@AbnormalPoof AbnormalPoof added type: minor bug Involves a minor bug or issue. size: small A small pull request with 10 or fewer changes. labels Feb 1, 2025
@Hundrec
Copy link
Collaborator

Hundrec commented Feb 1, 2025

You did the meme!

Thanks for working on this :)

@github-actions github-actions bot added size: large A large pull request with more than 100 changes. and removed size: small A small pull request with 10 or fewer changes. labels Feb 1, 2025
@Lasercar
Copy link
Contributor Author

Lasercar commented Feb 1, 2025

So I've got it to save to an array while the screenshots are spammed, and then when the spam stops, to save out all the images and make sure that they're unique, but I haven't gotten the cursor and preview to stop appearing in the screenshots, and for the cursor to go back to being hidden/shown before the screenshot spam.

And I've just noticed an issue where the timer for ending the screenshot spam mode breaks after changing states.

So I'm going to give up for now, I've spent why too much time trying to fix those two first issues.
older commit stuff

@AbnormalPoof AbnormalPoof added status: needs revision Cannot be approved because it is awaiting some work by the contributor. and removed status: pending triage Awaiting review. labels Feb 1, 2025
@charlesisfeline
Copy link

eric already did a fix for this in a seperate branch months ago, idk why he didnt merge it to the latest version tho

@Lasercar
Copy link
Contributor Author

Lasercar commented Feb 2, 2025

eric already did a fix for this in a seperate branch months ago, idk why he didnt merge it to the latest version tho

Wait, really? Did I spend all that time for nothing?

@Hundrec
Copy link
Collaborator

Hundrec commented Feb 2, 2025

Since we have over 100 open PRs (and many more merged), it's very likely that you'll come up with the same idea as someone else.

I recommend doing a thorough search for similar PRs before creating a new one.

@Lasercar
Copy link
Contributor Author

Lasercar commented Feb 2, 2025

Well anyway, here's the new result:

Video + screenshots
2025-02-02.16-59-12.mp4

screenshot-2025-02-02-03-25-40
screenshot-2025-02-02-16-59-29
screenshot-2025-02-02-16-59-30 (2)
screenshot-2025-02-02-16-59-30
screenshot-2025-02-02-16-59-31 (2)
screenshot-2025-02-02-16-59-31
screenshot-2025-02-02-16-59-32 (2)
screenshot-2025-02-02-16-59-32

@Lasercar Lasercar marked this pull request as ready for review February 2, 2025 07:02
@Hundrec
Copy link
Collaborator

Hundrec commented Feb 2, 2025

That's actually hilarious
Does the preview show up if you stop spamming the screenshot button?

@Hundrec Hundrec added status: pending triage Awaiting review. and removed status: needs revision Cannot be approved because it is awaiting some work by the contributor. labels Feb 2, 2025
@EliteMasterEric EliteMasterEric added status: reviewing internally Under consideration and testing. and removed status: pending triage Awaiting review. labels Feb 2, 2025
@Hundrec
Copy link
Collaborator

Hundrec commented Feb 2, 2025

Just checked, this is a unique PR.
Once you address any questions reviewers have, this should be good to go!

@Lasercar
Copy link
Contributor Author

Lasercar commented Feb 2, 2025

That's actually hilarious Does the preview show up if you stop spamming the screenshot button?

Sadly no, but I could do that.

Just checked, this is a unique PR. Once you address any questions reviewers have, this should be good to go!

Yay!

@Lasercar
Copy link
Contributor Author

Lasercar commented Feb 2, 2025

Ok, I've got it to show the preview on the last image in the buffer.

Ayo.It.s.Fixed.mp4

The only thing it needs now is either some sort of delay for when saving each screenshot in the buffer (timers won't work because they reset between states) and/or multi-threading of the screenshot saving, as you can see, it causes the screen to freeze while it processes the images.

@Hundrec
Copy link
Collaborator

Hundrec commented Feb 2, 2025

I appreciate you doing the meme every time

Please let us know when you get this working with a preview at the end!

@Lasercar
Copy link
Contributor Author

Lasercar commented Feb 2, 2025

Please let us know when you get this working with a preview at the end!

Uh, I just did that?

@Hundrec
Copy link
Collaborator

Hundrec commented Feb 2, 2025

Well, your video freezes right now, so we wouldn't want to add that to the game just yet

@Lasercar
Copy link
Contributor Author

Lasercar commented Feb 2, 2025

Oh, right.

I'd need help on fixing that.

I guess not.

@Lasercar
Copy link
Contributor Author

Lasercar commented Feb 2, 2025

Alright, I've got it saving asynchronously. I can uncomment the timer so that it saves each screenshot with a delay, though if I do that, whenever the state changes any screenshots not already saved will be lost. If anyone knows of a way to delay the screenshots without that issue I'll update it.

Video + screenshots
2025-02-02.20-53-42.1.mp4

screenshot-2025-02-02-20-54-03
screenshot-2025-02-02-20-54-04 (2)
screenshot-2025-02-02-20-54-04
screenshot-2025-02-02-20-54-05
screenshot-2025-02-02-20-54-05 (2)

@EliteMasterEric EliteMasterEric force-pushed the screenshot-preview-in-screenshot-fix branch from 8ab70ea to 2a46b74 Compare March 11, 2025 22:04
@EliteMasterEric
Copy link
Member

Force pushed, hopefully that didn't break anything.

@Lasercar
Copy link
Contributor Author

Force pushed, hopefully that didn't break anything.

It didn't, but it did break by local branch though, so I had to learn how to pull and rebase it to avoid a merge commit message (turns out all you have to do is just add --rebase to the command).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: haxe PR modifies game code. size: large A large pull request with more than 100 changes. status: accepted PR was approved for contribution. If it's not already merged, it may be merged on a private branch. type: enhancement Involves an enhancement or new feature.
Projects
None yet
6 participants