Skip to content

Conversation

TheMoonThatRises
Copy link
Member

@TheMoonThatRises TheMoonThatRises commented Oct 10, 2022

Allows the user to install apps without always injecting PlayTools. This might take a while to complete.

  • Allow user to distinguish between apps with and without PlayTools
  • Someway for the code to differentiate between PlayToolsless apps
  • Allow users to quickly inject and remove PlayTools from apps

@IsaacMarovitz
Copy link
Member

IsaacMarovitz commented Oct 12, 2022

Could you add either a toggle/button in either App Settings or the context menu to allow the user to easily opt in or out of PlayTools on the fly without reinstalling?

Someway for the code to differentiate between PlayToolsless apps

I would suggest adding a isPlayToolsInstalled Bool to the App Settings plist

@Depal1
Copy link
Member

Depal1 commented Oct 12, 2022

As for a way for the user to distinguish whether an app is PlayTools less or not, a bubble with an appropriate icon could be placed in the corner of the app icon, plus an entry for ´isPlayToolsInstalled´ in the info tab.

@TheMoonThatRises
Copy link
Member Author

Could you add either a toggle/button in either App Settings or the context menu to allow the user to easily opt in or out of PlayTools on the fly without reinstalling?

How would PlayTools be removed after it has already been in the executable?

@TheMoonThatRises
Copy link
Member Author

I would suggest adding a isPlayToolsInstalled Bool to the App Settings plist

Also the one thing I worry about adding another thing to the App Settings is that it would reset the user's current settings, which may not be fun for the user.

@IsaacMarovitz
Copy link
Member

Could you add either a toggle/button in either App Settings or the context menu to allow the user to easily opt in or out of PlayTools on the fly without reinstalling?

How would PlayTools be removed after it has already been in the executable?

You would have to use optool to remove the added load command from the dylib, and also remove the PlugIns folder from the .app (This is mostly optional but if you want to return the bundle to its original state, it's necessary)

@IsaacMarovitz
Copy link
Member

Also the one thing I worry about adding another thing to the App Settings is that it would reset the user's current settings, which may not be fun for the user.

On further thought, App Settings are also designed to be transferable from user to user so people can easily share configurations. In that case, it might be worth just creating a plist for PlayTools install status

@Depal1 Depal1 added the squash Indicates whether a PR must be squashed before being merged label Oct 19, 2022
* Removed dependence on storage plist
* Automatically reload app when PlayTools is injected/removed
@TheMoonThatRises TheMoonThatRises marked this pull request as ready for review October 20, 2022 02:26
@IsaacMarovitz
Copy link
Member

Screenshot 2022-10-19 at 22 40 45

I'm not entirely convinced by the placement of the warning in App Settings, maybe it would work better if it was right-aligned, and in line with the title.

@Depal1
Copy link
Member

Depal1 commented Oct 20, 2022

Screenshot 2022-10-19 at 22 40 45

I'm not entirely convinced by the placement of the warning in App Settings, maybe it would work better if it was right-aligned, and in line with the title.

Wouldn't that be conflictive in apps with large names? I just state it as a possibility. I think it is okay as is.

@IsaacMarovitz
Copy link
Member

Wouldn't that be conflictive in apps with large names? I just state it as a possibility. I think it is okay as is.

I think mainly the spacing just looks a little off. Maybe something like this would work better?

196843562-c000d579-a162-48f0-90ae-2c2c4292ded9

@Depal1
Copy link
Member

Depal1 commented Oct 20, 2022

I think mainly the spacing just looks a little off. Maybe something like this would work better?

196843562-c000d579-a162-48f0-90ae-2c2c4292ded9

I think I noticed the spacing issue you mention. Aligning the warning just under the first letter of the app name could be an option, instead of under the app icon. Putting the warning in the lower end wouldn't be noticeable.

@TheMoonThatRises
Copy link
Member Author

I think I noticed the spacing issue you mention. Aligning the warning just under the first letter of the app name could be an option, instead of under the app icon. Putting the warning in the lower end wouldn't be noticeable.

Is this spacing better?
Screenshot 2022-10-20 at 08 43 05

@Depal1
Copy link
Member

Depal1 commented Oct 20, 2022

Is this spacing better?

It is. @IsaacMarovitz What do you think?

@amirsaam
Copy link
Member

text should've less spacing from the warning sign i would say. like:
⚠️.PlayTools
now it's like
⚠️...PlayTools

@Depal1 Depal1 merged commit a1cf8a3 into PlayCover:develop Oct 24, 2022
@TheMoonThatRises TheMoonThatRises deleted the feat/noplaytools branch January 1, 2023 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squash Indicates whether a PR must be squashed before being merged
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants