Skip to content

Conversation

AIFSKATE
Copy link

@AIFSKATE AIFSKATE commented Sep 18, 2025

PR Details

  1. The reason this line is barely visible is because it's the default setting when IsEnabled is set to false. The easiest way to fix this is to change the check condition from 1 to 0.
  2. I changed the version description from .NET Core XX to .NET XX. And because the issue says "It isn't .NET Core anymore, it should be just .NET 10 or .NET 8," I also deleted an if branch.
image

Related Issue

#2895

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.
  • I have built and run the editor to try this change out.

1. The reason this line is barely visible is because it's the default setting when IsEnabled is set to false. The easiest way to fix this is to change the check condition from 1 to 0.
2. I changed the version description from .NET Core XX to .NET XX. Because it says "It isn't .NET Core anymore, it should be just .NET 10 or .NET 8," I also deleted an if branch.
@AIFSKATE
Copy link
Author

@AIFSKATE please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@dotnet-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@dotnet-policy-service agree

@VaclavElias VaclavElias added the area-Launcher Stride Launcher label Sep 18, 2025
@VaclavElias
Copy link
Contributor

Thanks @AIFSKATE , visually it looks definitelly better. I will let others to review your updates.

@Eideren Eideren changed the title Try to solve #2895 chore: Improve launcher framework combobox (#2895) Sep 18, 2025
@Eideren
Copy link
Collaborator

Eideren commented Sep 27, 2025

@VaclavElias was your intent to have the combo box be interactible when there is just one framework to choose from, or keep it non-interactible but slightly more visible than it currently is ? I think keeping it non-interactive is better as it conveys to the user that there is nothing for them to change there. Otherwise users might press on the combo box just to check if there is anything else they could set it to when that box doesn't have anything else but the default value.

@VaclavElias
Copy link
Contributor

I wanted only better visibility which is very good to promote .NET version. See the current state. If the combo box isn't needed that would be even better, but that would be better to solve in a separate issue. Which version even allowed to select multiple frameworks?

image

@Eideren
Copy link
Collaborator

Eideren commented Sep 27, 2025

Which version even allowed to select multiple frameworks?

Pre net6.0 I think ? The launcher should support every engine version as long as feasible, so best leave that combo box in and have it slightly more opaque if possible.

@VaclavElias
Copy link
Contributor

I see, so the reason it wasn't visible was because there was only 1 item. What about, if there is only 1 item, it should be only a text?

Comment on lines -16 to +17
if (framework.Framework == ".NETFramework")
return $".NET {framework.Version.ToString(3)}";
else if (framework.Framework == ".NETCoreApp")
return $".NET Core {framework.Version.ToString(2)}";
if (framework.Framework == ".NETCoreApp")
return $".NET {framework.Version.ToString(2)}";
Copy link
Member

Choose a reason for hiding this comment

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

This is required for older versions that supported multiple frameworks.

</Button>
</Grid>
<ComboBox Margin="0,4" Name="FrameworkSelector" IsEnabled="{Binding ActiveVersion.Frameworks.Count, Converter={sskk:Chained {sskk:IsGreater}, Parameter1={sskk:Double 1}}}" ItemsSource="{Binding ActiveVersion.Frameworks}" SelectedItem="{Binding ActiveVersion.SelectedFramework}" SelectionChanged="FrameworkChanged">
<ComboBox Margin="0,4" Name="FrameworkSelector" IsEnabled="{Binding ActiveVersion.Frameworks.Count, Converter={sskk:Chained {sskk:IsGreater}, Parameter1={sskk:Double 0}}}" ItemsSource="{Binding ActiveVersion.Frameworks}" SelectedItem="{Binding ActiveVersion.SelectedFramework}" SelectionChanged="FrameworkChanged">
Copy link
Member

Choose a reason for hiding this comment

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

That's kind of a useless check since it can never be negative. The reason it was greyed out was because there is no need to select anything when only one framework is available.

I'd rather not display anything in that case, i.e. change IsEnabled to IsVisible.

Copy link
Contributor

Choose a reason for hiding this comment

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

What if we display plain text (in white) when there's only one item? It would still be helpful to show the .NET version so that those unfamiliar with Stride can see it.

The behavior would remain the same when multiple items are available, but if there's only one item, the combo box wouldn't be displayed, just plain white text instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Launcher Stride Launcher
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants