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

feat(extension): Rename launcher buttons for clarity #1872

Merged
merged 4 commits into from Oct 16, 2023

Conversation

acastrodev
Copy link
Contributor

@acastrodev acastrodev commented Sep 30, 2023

PR Details

  • Update launcher extension button labels
  • New labels: "Visual Studio Shader {0} Extensions"
  • Add link under VisualStudioExtension label: "An installed Visual Studio is required, which can be found here"

Description

This commit addresses the issue of misleading launcher extension button labels, making it clearer that users are installing an extension, not the IDE itself. The new labels and added clarification help redirect users to the Visual Studio download page to avoid confusion.

Related Issue

Resolves: #1773

Motivation and Context

Improves Launcher UX

No Visual Studio 2019 or 2022 found (hovering over the link shows the tooltip in the bottom part of the screen with the Visual Studio download page URL):

image

When one or another is found:

image

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

   - Update launcher extension button labels
   - New labels: "Visual Studio Shader addons"
   - Add link under VisualStudioExtension label: "An installed Visual Studio is required, which can be found here"

This commit addresses the issue of misleading launcher extension button labels, making it clearer that users are installing an extension, not the IDE itself. The new labels and added clarification help redirect users to the Visual Studio download page to avoid confusion.
@acastrodev
Copy link
Contributor Author

@Doprez I closed the #1862 and recreated it here using a topic branch (the other was blocking my master). :)

@MeharDT
Copy link
Contributor

MeharDT commented Oct 2, 2023

Can we only show the message if Visual Studio isn't installed? There's an example of how to detect an installation here,

public static void FindAndSetMSBuildVersion()

The VS requirements also change by Stride version (i.e., VS 2022 is only needed for Stride 4.1+). We might have to re-word the message to get this across.

@acastrodev
Copy link
Contributor Author

Thanks, @MeharDT! I will take a look on it.

@acastrodev
Copy link
Contributor Author

I'm facing a dilemma and would appreciate your input on it:

We currently have a class called PackageSessionPublicHelper that contains logic for detecting Visual Studio instances. I'd like to centralize this logic within PackageSessionPublicHelper and use the same class in the Strider.Launcher project. However, there's a challenge: PackageSessionPublicHelper resides in the Stride.Core.Assets project, which is not part of the Strider.Launcher solution. In my opinion, we have two potential solutions:

  1. Move PackageSessionPublicHelper from the Stride.Core.Assets project to the Stride.Core project, making it accessible to both the Stride and Strider.Launcher solutions.

  2. Keep PackageSessionPublicHelper in the Stride.Core.Assets project and create a different helper class within the Strider.Launcher project to handle the same logic.

What would you suggest we do in this situation?

Thank you!

@MeharDT
Copy link
Contributor

MeharDT commented Oct 3, 2023

Hi @acastrodev, I think option 1 makes the most amount of sense to me but this might have to be a discussion for the next community meeting.

Another option, can this PR idea be merged into the existing FindAndSetMSBuildVersion() check? When launching Stride it looks for a VS and .NET install and throws an exception if either are missing (the user isn't told what is missing and is only shown a generic error with download links for both).

private static async void Startup(UFile initialSessionPath)

@acastrodev
Copy link
Contributor Author

Hi @MeharDT,

Thank you for your input! I agree that option 1 seems like the approach to go, and discussing it in the next community meeting sounds like a good idea to ensure everyone's alignment.

Regarding your suggestion to merge this PR idea into the existing FindAndSetMSBuildVersion() check, it's an interesting thought. It could potentially simplify the logic and improve the user experience by providing more specific error messages. I would say, we could definitely consider that as an option.

@manio143
Copy link
Member

manio143 commented Oct 3, 2023

Note that PackageSessionPublicHelper has a dependency on Microsoft.Build. If you move it to Stride.Core it means every game created has a runtime dependency on MSBuild.
I think it would be better to create a new project which would contain abstraction over MSBuild that can be shared between Stride.Core.Assets and the launcher without impacting user runtime dependencies.

@acastrodev
Copy link
Contributor Author

acastrodev commented Oct 3, 2023

Thank you, @manio143. I'll consider your point about the MSBuild dependency. Creating a new project for the MSBuild abstraction sounds like a good approach to avoid impacting dependencies. I'll explore this option further and update this PR. Appreciate your insight!

@acastrodev
Copy link
Contributor Author

acastrodev commented Oct 4, 2023

Following up the idea to extract this logic for a new project, my understanding is that it can live under Stride.Core. instead of Stride.Core.Assets.

Assuming Stride.Core. is the way to go, I have some suggestions to name this project as follows:

1 - Stride.Core.Build.Locator (in the same way as Microsoft assembly name Microsoft.Build.Locator)
2 - Stride.Core.Build (More generalist. We can use Stride.Core.Build.Locator namespace anyway)
3 - Stride.Core.MSBuild (Straightforward to make clear it is Microsoft MSBuild related)
4 - Stride.Core.MSBuild.Locator (same as the first option but more specific)

@manio143 and others, please let me know what are your ideas around that. Thanks!

@Ethereal77
Copy link
Contributor

Please note that, in the solution, Stride.Core and related "core" assemblies are implementing common functionality for the "runtime" (i.e. the engine, graphics, physics, etc), while Stride.Core.Design and related "design" assemblies are implementing common functionality for the tooling and editor parts.

This one is more appropriately placed under the "design-time" part. The naming could be whatever is better for legibility, but it must be in the correct category to avoid confusion.

@acastrodev
Copy link
Contributor Author

acastrodev commented Oct 4, 2023

Thanks for the feedback @Ethereal77. I see now we already have a class inside Stride.Core.Design for this purpose — to check Visual Studio versions installed (VisualStudioVersions). So, no big changes required. I will update this PR.

In LauncherWindow.xaml.cs:
- Added a Loaded event handler method TextBlockVisualStudioDownloadPage_Loaded to handle the loading of the TextBlock displaying Visual Studio download information.
- Implemented logic to check for compatible Visual Studio versions and hide the TextBlock if compatible versions are found.

In LauncherWindow.xaml:
- Added the Loaded event handler TextBlockVisualStudioDownloadPage_Loaded to the TextBlock element responsible for displaying Visual Studio download information.

This change improves the user experience by dynamically hiding the Visual Studio download information TextBlock when compatible versions are detected, providing a cleaner interface.
Copy link
Member

@manio143 manio143 left a comment

Choose a reason for hiding this comment

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

Cool, so the dependencies were already referenced. Good job!

@MeharDT
Copy link
Contributor

MeharDT commented Oct 4, 2023

Nice work, well done @acastrodev!

Is there a way to see which version of Stride is actively selected in the launcher? Since you're checking for either VS 2019 or 2022, maybe the text that's displayed could reflect that? As an example if Stride 4.1 is selected it could say something like,

A valid Visual Studio 2022 installation could not be found. Download Now (hyperlink)

For Stride 4.0 and below it would be 2019

@acastrodev
Copy link
Contributor Author

Nice work, well done @acastrodev!

Is there a way to see which version of Stride is actively selected in the launcher? Since you're checking for either VS 2019 or 2022, maybe the text that's displayed could reflect that? As an example if Stride 4.1 is selected it could say something like,

A valid Visual Studio 2022 installation could not be found. Download Now (hyperlink)

For Stride 4.0 and below it would be 2019

Thanks, @MeharDT! And thanks for the feedback!

I trust it is possible, although, if we follow the prerequisites from README it mentions Visual Studio 2022 only. If we are going to explicit offer a link with a wording based in the selected Stride version, I would say we should also need to update the README to reflect it.

Another option is to include "2022" straight ahead in the current wording, being aligned with README instructions.

@MeharDT
Copy link
Contributor

MeharDT commented Oct 6, 2023

Nice work, well done @acastrodev!
Is there a way to see which version of Stride is actively selected in the launcher? Since you're checking for either VS 2019 or 2022, maybe the text that's displayed could reflect that? As an example if Stride 4.1 is selected it could say something like,
A valid Visual Studio 2022 installation could not be found. Download Now (hyperlink)
For Stride 4.0 and below it would be 2019

Thanks, @MeharDT! And thanks for the feedback!

I trust it is possible, although, if we follow the prerequisites from README it mentions Visual Studio 2022 only. If we are going to explicit offer a link with a wording based in the selected Stride version, I would say we should also need to update the README to reflect it.

Another option is to include "2022" straight ahead in the current wording, being aligned with README instructions.

Fair enough, I only mentioned 2019 because of the extension but lets stick to 2022 since it's current for 4.1 and 4.2 (soon).

@Eideren Eideren merged commit cee7491 into stride3d:master Oct 16, 2023
1 check passed
@Eideren
Copy link
Collaborator

Eideren commented Oct 16, 2023

Thanks !

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.

Rename Tool download buttons in launcher.
5 participants