Small refactoring changes in Stride.GameStudio #1741
Conversation
Jklawreszuk
force-pushed
the
game-studio
branch
3 times, most recently
from
e04c785
to
aae3303
Compare
|
Hey @Jklawreszuk is this PR alright, I'm seeing a big contradiction here
|
sources/editor/Stride.GameStudio/Helpers/ApplicationInitializer.cs
Outdated
Show resolved
Hide resolved
| // Distributed under the MIT license. See the LICENSE.md file in the project root for more information. | ||
| namespace Stride.GameStudio.Helpers | ||
| { | ||
| internal static class ApplicationInitializer |
There was a problem hiding this comment.
So, what's your reasoning here wrt extracting those functions out of program.cs ?
There was a problem hiding this comment.
My intention was to indicate that these methods are needed to initialize the app, so I created a separate class with Init() method creating facade behind all of this.
There was a problem hiding this comment.
Right, but you've just moved the entirety of the logic within Program to a different class. In other words, you renamed Program into ApplicationInitializer and made a new file called Program just for the Main() to call into it.
It'll be harder to follow the git history for that file, and doesn't seem like it provides a lot in exchange ?
I welcome new methods and such to clarify what's actually going on here, but moving everything to another file is hard to justify for me. If other contributors think this is a good idea, feel free to chime in.
There was a problem hiding this comment.
@Eideren Yeah, I guess you're rigth about that. I've reverted this change 👍
| using System; | ||
| using System.Globalization; | ||
| using Stride.Core.Assets.Editor; | ||
| using Stride.Core.Translation; | ||
| using Stride.Core.Translation.Providers; | ||
| using EditorSettings = Stride.Core.Assets.Editor.Settings.EditorSettings; | ||
| // Copyright (c) .NET Foundation and Contributors (https://dotnetfoundation.org/ & https://stride3d.net) and Silicon Studio Corp. (https://www.siliconstudio.co.jp) | ||
| // Distributed under the MIT license. See the LICENSE.md file in the project root for more information. | ||
| namespace Stride.GameStudio.Helpers | ||
| { | ||
| internal static class LanguageInitializer | ||
| { | ||
| public static void SetLanguage() | ||
| { | ||
| TranslationManager.Instance.RegisterProvider(new GettextTranslationProvider()); | ||
| TranslationManager.Instance.CurrentLanguage = EditorSettings.Language.GetValue() switch | ||
| { | ||
| SupportedLanguage.MachineDefault => CultureInfo.InstalledUICulture, | ||
| SupportedLanguage.English => new CultureInfo("en-US"), | ||
| SupportedLanguage.French => new CultureInfo("fr-FR"), | ||
| SupportedLanguage.Japanese => new CultureInfo("ja-JP"), | ||
| SupportedLanguage.Russian => new CultureInfo("ru-RU"), | ||
| SupportedLanguage.German => new CultureInfo("de-DE"), | ||
| SupportedLanguage.Spanish => new CultureInfo("es-ES"), | ||
| SupportedLanguage.ChineseSimplified => new CultureInfo("zh-Hans"), | ||
| SupportedLanguage.Italian => new CultureInfo("it-IT"), | ||
| SupportedLanguage.Korean => new CultureInfo("ko-KR"), | ||
| _ => throw new ArgumentException("Invalid langauge option"), | ||
| }; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
A whole file for just one function seems a bit much. I think multiple calls to this function may create side effects, I think it would fit better as a function local to the callsite, that way we don't have any implication that it does.
There was a problem hiding this comment.
Good point, I've moved method to Program.cs again
|
What's the status on this PR? |
Jklawreszuk
force-pushed
the
game-studio
branch
from
f1390ec
to
ba75496
Compare
|
Sorry, Honestly I am almost forgot about it, but I think now is ready :) |
| AppDomain.CurrentDomain.UnhandledException += CurrentDomain_UnhandledException; | ||
| EditorPath.EditorTitle = StrideGameStudio.EditorName; | ||
| AppDomain.CurrentDomain.UnhandledException += CurrentDomain_UnhandledException; | ||
| EditorPath.EditorTitle = StrideGameStudio.EditorName; |
There was a problem hiding this comment.
Now that StrideGameStudio is in Stride.GameStudio.Helpers you have to include that namespace, same goes for StrideEditorPlugin and co. Right now the gamestudio can't build.
There was a problem hiding this comment.
I guess should work now ✌️
|
Thanks ! |
PR Details
This PR refactors Stride.GameStudio and replaces Xenko images with Stride images in CrashReport form.
Types of changes
Checklist