Skip to content

Experimental Feature: Game Screenshots#242

Merged
rileytestut merged 7 commits into
rileytestut:experimental_featuresfrom
LitRitt:experimental_feature/game_screenshots
Apr 28, 2023
Merged

Experimental Feature: Game Screenshots#242
rileytestut merged 7 commits into
rileytestut:experimental_featuresfrom
LitRitt:experimental_feature/game_screenshots

Conversation

@LitRitt

@LitRitt LitRitt commented Apr 25, 2023

Copy link
Copy Markdown
Contributor

No description provided.

@rileytestut rileytestut left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for breaking these PRs up! Overall looks good to me, just have one note about the implementation of isAuthorized that could result in minor bug, but once that’s fixed I’d be happy to merge 🙂

Also I appreciate you putting the PHPhotoLibrary stuff in an extension! The more isolated changes are from the rest of the app, the better 👍

Comment thread Delta/Extensions/PHPhotoLibrary+Authorization.swift Outdated
Comment thread Delta/Emulation/GameViewController.swift Outdated
- Ensures that the screenshot is saved when the user is first prompted for access to Photos
- More elegant extension code

@rileytestut rileytestut left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nice approach, passing in a closure that's only run if authorized is a good idea.

Thanks for these changes, everything looks good to me! Will merge this in along with #241 once everything is merged into develop branch 👍

@unknown default:
return false
}
PHPhotoLibrary.requestAuthorization(for: .addOnly, handler: { success in

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nice, forgot about the .addOnly option! Good call.

return false
}
PHPhotoLibrary.requestAuthorization(for: .addOnly, handler: { success in
switch success

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Minor nit, but I think result might be a better name than success since it's an enum not a Bool. But I'll still merge anyway since doesn't really matter, just for future notice.

@rileytestut rileytestut merged commit 6bdc05f into rileytestut:experimental_features Apr 28, 2023
luf1k pushed a commit to playwithlu/Delta that referenced this pull request Jan 22, 2025
* Adds extension to handle interactions with Photos app
- isAuthorized to check/ask for authorization
- saveUIImage to save a UIImage to Photos

* Adds a Pause Menu button for taking game screenshots

* Adds @feature and @options for Game Screenshots

* Implements Game Screenshots feature in GameViewController

* Updates project settings

* Passes call to save to Photos as a closure into authorization prompt
- Ensures that the screenshot is saved when the user is first prompted for access to Photos
- More elegant extension code

---------

Co-authored-by: Riley Testut <riley@rileytestut.com>

@takashiou takashiou left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

好👍

@jesn459 jesn459 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Gracias

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