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

Fixed exit from game saves username instead of email #2214

Merged
merged 1 commit into from Mar 14, 2018

Conversation

3 participants
@VPeruS
Contributor

VPeruS commented Mar 13, 2018

No description provided.

@@ -125,7 +125,6 @@ private void run()
userReq.Success += u =>
{
LocalUser.Value = u;
Username = LocalUser.Value.Username;

This comment has been minimized.

@Aergwyn

Aergwyn Mar 13, 2018

Member

This doesn't seem intended?

This comment has been minimized.

@VPeruS

VPeruS Mar 13, 2018

Contributor

Easier to have User as dependency and get Username from him, or use LocalUser bindable they all return same without reassigment

This comment has been minimized.

@Aergwyn

Aergwyn Mar 13, 2018

Member

If the Username is not set it will continually set the API to offline? For example if requests failed because the API is unavailable and it tries to reconnect. Just wondering if that happens.

This comment has been minimized.

@VPeruS

VPeruS Mar 13, 2018

Contributor

No, it will go Offline and stay in it

This comment has been minimized.

@Aergwyn

Aergwyn Mar 13, 2018

Member

Oh I confused something then. Alright, thanks!

This comment has been minimized.

This comment has been minimized.

@Aergwyn

Aergwyn Mar 13, 2018

Member

Yes and HasLogin relies on the Username being present and I remember the API logging me out when it was having problems so I feared it wouldn't reconnect you. But then I also remembered that it was only a visual thing and the Username isn't actually cleared so it wouldn't matter. Hence I confused myself.

@peppy

peppy approved these changes Mar 14, 2018

there's some more changes i'd like to see done, but i'll push these in a follow-up PR.

@peppy peppy merged commit 586ac9c into ppy:master Mar 14, 2018

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@VPeruS

This comment has been minimized.

Contributor

VPeruS commented Mar 14, 2018

You can mention them here if you want me to take care of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment