Allow selecting a default account to use with an instance #685

byteduck · 2022-12-27T22:14:59Z

This commit adds the functionality to select a default account to use when launching an instance (#341). If no account is supplied via the accountToUse parameter of Application::launch, then the instance will launched logged into the default account for the instance (or the global default account if none is set for the instance).

The setting is found in the Miscellaneous tab of the instance settings and looks like this:

I couldn't manage to get a screenshot with the dropdown open, but it contains an option for every account configured in Prism. To revert the setting, one can simply uncheck the "Set a default account to use with this instance" box. If the account configured is removed, then the setting is reverted to showing "No default account".

flowln

seems to be working mostly fine, thanks! most the suggested changes are only about some code cleanups :)

we have a .clang-format thingy in the launcher's root directory, please use that on the code you've added, the formatting is somewhat inconsistent :p

launcher/ui/pages/instance/InstanceSettingsPage.cpp

TheKodeToad · 2022-12-29T09:52:49Z

imo the option name ~~name~~ is slightly longer than it should be.

byteduck · 2022-12-29T17:20:57Z

imo the option name ~~name~~ is slightly longer than it should be.

As in, the text seen in the GUI or the key used for the settings store?

byteduck · 2022-12-29T17:21:51Z

we have a .clang-format thingy in the launcher's root directory, please use that on the code you've added, the formatting is somewhat inconsistent :p

Ah yes, my apologies - I was trying to follow the style of the code I saw and didn't notice the .clang-format. Will do!

TheKodeToad · 2023-01-02T17:50:36Z

imo the option name ~~name~~ is slightly longer than it should be.

As in, the text seen in the GUI or the key used for the settings store?

In the GUI. "for this instance" could be seen as redundant.

Signed-off-by: Aaron <10217842+byteduck@users.noreply.github.com>

Co-authored-by: flow <flowlnlnln@gmail.com> Signed-off-by: Aaron Sonin <10217842+byteduck@users.noreply.github.com> Signed-off-by: Aaron <10217842+byteduck@users.noreply.github.com>

Signed-off-by: Aaron <10217842+byteduck@users.noreply.github.com>

byteduck · 2023-01-02T18:18:17Z

imo the option name ~~name~~ is slightly longer than it should be.

As in, the text seen in the GUI or the key used for the settings store?

In the GUI. For this instance could be seen as redundant.

I've reworded it to "Override default account". When unchecked, the instance-specific account selector will show the current global setting for the default account.

byteduck · 2023-01-02T19:47:07Z

Hmm, not sure why the ResourceFolderModel test failed on Windows-MSVC-Legacy only. Nothing I did should affect that, right? I did notice that Windows-MSVC-Legacy failed when I first opened this PR, and when run a second time after signing my commit, it succeeded.

flowln · 2023-01-02T20:37:35Z

Hmm, not sure why the ResourceFolderModel test failed on Windows-MSVC-Legacy only. Nothing I did should affect that, right? I did notice that Windows-MSVC-Legacy failed when I first opened this PR, and when run a second time after signing my commit, it succeeded.

seems to be some issue on develop, i've seen another PR with the same error earlier today, dw about that

flowln

sorry, i kinda forgot i didn't already review this 😅

anyway, the code looks mostly good, there's just a little cornercase and maybe moving some logic to a helper function, otherwise it's all good! thanks!

launcher/ui/pages/instance/InstanceSettingsPage.cpp

byteduck · 2023-01-09T20:34:14Z

sorry, i kinda forgot i didn't already review this 😅

No worries! I'll get the corner case fixed soon :)

…hub.com> I, Aaron <10217842+byteduck@users.noreply.github.com>, hereby add my Signed-off-by to this commit: 160dd09 Signed-off-by: Aaron <10217842+byteduck@users.noreply.github.com>

byteduck · 2023-01-13T07:53:02Z

Seems like some Ubuntu package mirrors are down which caused the checks to fail... I'm encountering this on my own projects too 😬

DioEgizio · 2023-01-13T15:39:16Z

Seems like some Ubuntu package mirrors are down which caused the checks to fail... I'm encountering this on my own projects too 😬

actions/runner-images#6894

flowln

ideally we'd have some place in the UI to indicate that the account for a given instance is overridden. However, that'd probably mean messing with the account selector thingy, so it's probably a bit troublesome for now...

anyway, this looks good to me for the time being, so thank you! :D

Scrumplex

LGTM, thanks!

byteduck force-pushed the instance-accounts branch from a4555f5 to f1fa28c Compare December 27, 2022 22:16

byteduck mentioned this pull request Dec 27, 2022

Setting to allow switching accounts based on instance #341

Closed

1 task

flowln added the enhancement New feature or request label Dec 28, 2022

flowln added this to the 7.0 milestone Dec 28, 2022

flowln reviewed Dec 28, 2022

View reviewed changes

byteduck and others added 7 commits January 2, 2023 11:17

Allow selecting a default account to use with an instance

7e2d78b

Signed-off-by: Aaron <10217842+byteduck@users.noreply.github.com>

Fix conflicting layout name in InstanceSettingsPage

cba3d68

Signed-off-by: Aaron <10217842+byteduck@users.noreply.github.com>

Replace unecessary type check with assertion in InstanceSettingsPage

021e6c0

Co-authored-by: flow <flowlnlnln@gmail.com> Signed-off-by: Aaron Sonin <10217842+byteduck@users.noreply.github.com> Signed-off-by: Aaron <10217842+byteduck@users.noreply.github.com>

Add null check for face in instance account settings selector

e186523

Co-authored-by: flow <flowlnlnln@gmail.com> Signed-off-by: Aaron Sonin <10217842+byteduck@users.noreply.github.com> Signed-off-by: Aaron <10217842+byteduck@users.noreply.github.com>

Properly connect signal in instance settings for account selector

9b8add1

Co-authored-by: flow <flowlnlnln@gmail.com> Signed-off-by: Aaron Sonin <10217842+byteduck@users.noreply.github.com> Signed-off-by: Aaron <10217842+byteduck@users.noreply.github.com>

Remove unecessary delete in InstanceSettingsPage destructor

eefb259

Co-authored-by: flow <flowlnlnln@gmail.com> Signed-off-by: Aaron Sonin <10217842+byteduck@users.noreply.github.com> Signed-off-by: Aaron <10217842+byteduck@users.noreply.github.com>

Reword instance-specific account settings, apply clang-format

ba81ad1

Signed-off-by: Aaron <10217842+byteduck@users.noreply.github.com>

byteduck force-pushed the instance-accounts branch from 5ae4802 to ba81ad1 Compare January 2, 2023 18:17

Merge branch 'PrismLauncher:develop' into instance-accounts

e0e3d4d

byteduck requested a review from flowln January 9, 2023 17:14

flowln approved these changes Jan 9, 2023

View reviewed changes

launcher/ui/pages/instance/InstanceSettingsPage.cpp Outdated Show resolved Hide resolved

Fix instance account selector face for offline accounts

160dd09

byteduck force-pushed the instance-accounts branch 4 times, most recently from 05d6227 to 160dd09 Compare January 13, 2023 04:11

Merge branch 'PrismLauncher:develop' into instance-accounts

b2f48ea

DCO Remediation Commit for Aaron <10217842+byteduck@users.noreply.git…

4e80d1f

…hub.com> I, Aaron <10217842+byteduck@users.noreply.github.com>, hereby add my Signed-off-by to this commit: 160dd09 Signed-off-by: Aaron <10217842+byteduck@users.noreply.github.com>

flowln approved these changes Jan 13, 2023

View reviewed changes

Scrumplex approved these changes Jan 13, 2023

View reviewed changes

flowln merged commit 3de681d into PrismLauncher:develop Jan 13, 2023

byteduck deleted the instance-accounts branch January 13, 2023 20:26

Scrumplex mentioned this pull request Feb 3, 2023

Set default-account to specific instances #812

Closed

1 task

leia-uwu mentioned this pull request May 6, 2023

Account Selection setting for instances #1060

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow selecting a default account to use with an instance #685

Allow selecting a default account to use with an instance #685

byteduck commented Dec 27, 2022 •

edited

flowln left a comment

TheKodeToad commented Dec 29, 2022 •

edited

byteduck commented Dec 29, 2022

byteduck commented Dec 29, 2022

TheKodeToad commented Jan 2, 2023 •

edited

byteduck commented Jan 2, 2023 •

edited

byteduck commented Jan 2, 2023 •

edited

flowln commented Jan 2, 2023

flowln left a comment

byteduck commented Jan 9, 2023

byteduck commented Jan 13, 2023

DioEgizio commented Jan 13, 2023

flowln left a comment

Scrumplex left a comment

Allow selecting a default account to use with an instance #685

Allow selecting a default account to use with an instance #685

Conversation

byteduck commented Dec 27, 2022 • edited

flowln left a comment

Choose a reason for hiding this comment

TheKodeToad commented Dec 29, 2022 • edited

byteduck commented Dec 29, 2022

byteduck commented Dec 29, 2022

TheKodeToad commented Jan 2, 2023 • edited

byteduck commented Jan 2, 2023 • edited

byteduck commented Jan 2, 2023 • edited

flowln commented Jan 2, 2023

flowln left a comment

Choose a reason for hiding this comment

byteduck commented Jan 9, 2023

byteduck commented Jan 13, 2023

DioEgizio commented Jan 13, 2023

flowln left a comment

Choose a reason for hiding this comment

Scrumplex left a comment

Choose a reason for hiding this comment

byteduck commented Dec 27, 2022 •

edited

TheKodeToad commented Dec 29, 2022 •

edited

TheKodeToad commented Jan 2, 2023 •

edited

byteduck commented Jan 2, 2023 •

edited

byteduck commented Jan 2, 2023 •

edited