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

Player name is changed to [unassigned] on successful login #442

Closed
GabeHirakawa opened this issue Sep 5, 2017 · 21 comments
Closed

Player name is changed to [unassigned] on successful login #442

GabeHirakawa opened this issue Sep 5, 2017 · 21 comments
Labels

Comments

@GabeHirakawa
Copy link

GabeHirakawa commented Sep 5, 2017

I've ran into an issue where whenever I successfully log into steam the user account that I logged into's name is changed to: [unassigned]. I've tried looking around to see what could possibly find the issue and the only thing I can find is referenced here: https://github.com/Jessecar96/SteamKit2/blob/master/SteamKit2/changes.txt

Local user is now given the persona name [unassigned] before SteamUser.AccountInfoCallback comes in.

However, this is from 2012 and is also not the official repo so I'm not sure how relevant it is. I'm kind of stuck and feel like I'm missing something pretty obvious. Any help with this would be appreciated.

@JustArchi
Copy link
Contributor

@GabeHirakawa
Copy link
Author

@JustArchi So that did seem to the nickname issue, but now it doesn't seem to be putting me online. Any idea?

@JustArchi
Copy link
Contributor

JustArchi commented Sep 5, 2017

If you put an example code then it'd be much better than asking my crystal ball, I'm trying to keep my mana for later 🙂.

@GabeHirakawa
Copy link
Author

@JustArchi My bad, I forgot to copy and paste /facepalm

        private static async void OnAccountInfo(SteamUser.AccountInfoCallback callback)
        {
            string nickname = _steamFriends.GetPersonaName();
            for (byte i = 0; (string.IsNullOrEmpty(nickname) || nickname.Equals("[unassigned]")); i++) {
                await Task.Delay(1000).ConfigureAwait(false);
                nickname = _steamFriends.GetPersonaName();
            }

            if (string.IsNullOrEmpty(nickname) || nickname.Equals("[unassigned]")) {
                return;
            }

            try {
                await _steamFriends.SetPersonaState(EPersonaState.Online);
            } catch (Exception e) {
                Console.WriteLine(e);
            }
        }

@yaakov-h
Copy link
Member

yaakov-h commented Sep 5, 2017

Can you post a reproducible example?

I have a suspicion you're trying to set your persona state too early.

@GabeHirakawa
Copy link
Author

@JustArchi
Copy link
Contributor

If your account is already named as [unassigned] then this will loop forever. You got rid of my timeout condition of i < 5. This is also why I added that code in the first place - SK2 always delivers nickname with maximum of 1 try if by any chance OnAccountInfo() gets triggered too early. Timeout means that your account is already named as [unassigned], so code can't tell the difference between proper nickname and lack of it.

@JustArchi
Copy link
Contributor

JustArchi commented Sep 5, 2017

In other words, wait for maximum of 5 seconds like in my original code, and assume it's OK to call SetPersonaState() afterwards, regardless if loop exited due to proper nickname or timeout.

@GabeHirakawa
Copy link
Author

That wasn't intentional, adding in the i < 5, however, still did not put me online, but did keep the username from changing.

@JustArchi
Copy link
Contributor

Because you're returning in the next if in [unassigned] case. You might want to handle it in a bit different/better way than I do in ASF - I just pointed out the issue you're having that you must wait for SK2 nickname to be registered. However, there is still an edge case of your nickname being [unassigned], then you can't say whether this is your real nickname or SK2 unregistered one, and you should call SetPersonaState() regardless. I'm not doing it in ASF, even though I probably should.

@JustArchi
Copy link
Contributor

@JustArchi
Copy link
Contributor

JustArchi commented Sep 5, 2017

BTW, while we're at it, maybe it'd make sense to somehow improve this part and use null at nickname place if it wasn't initialized yet, so we have a clear difference between SK2 pre-initialization step and user actually named [unassigned]. @yaakov-h thoughts?

Functions that expect nickname to be initialized such as SetPersonaState() could simply throw an exception if nickname is not initialized at the time of call. It'd solve issue like this one.

@yaakov-h
Copy link
Member

yaakov-h commented Sep 5, 2017

Hmm, it might be possible that you're handling the AccountInfoCallback that SteamUser posts, before SteamFriends gets a chance to handle that same message.

If you wait for the WalletInfoCallback instead, you should probably be in the clear.

And yes, we should probably not send [unassigned] to the CMs... @voided any thoughts? I think this has been in there since Vapor.

@GabeHirakawa
Copy link
Author

https://github.com/GabeHirakawa/SteamFriendUtility/blob/168ed870ee15111caba5aa02caee35c08e64e778/SteamFriendUtility/Program.cs#L128

So here is my most recent code. Still not going online.

@yaakov-h I'm still pretty new to this (as probably obvious by now) how would I go about fixing it according to your response?

@JustArchi
Copy link
Contributor

JustArchi commented Sep 5, 2017

This code looks good to me. You have a lot of synchronous functions there - including MainMenu() called in OnLoggedOn(), I wouldn't be shocked if callback manager was simply waiting for that callback to finish before forwarding future ones, including OnAccountInfo(). This has nothing to do with the issue you're having. You're simply blocking callback manager thread, so it can't continue delivering callbacks.

Using another callback for SetPersonaState() is not guaranteed to fix the issue, since callbacks can arrive in any order as they're all fired at the same time, I already met with unpredictable order of them. It's much better to use delay with timeout - at least until we implement something like getting rid of [unassigned] entirely, changing default value of nickname to null and throwing exception in functions executed too early that expect nickname to be initialized.

@GabeHirakawa
Copy link
Author

Hmm, okay.

Since we're on the topic, just for my learning/curious mind to understand. What is the process of the name being changed to [unassigned] and what is the purpose?

@yaakov-h
Copy link
Member

yaakov-h commented Sep 5, 2017

@JustArchi Callbacks are queued, the order should be deterministic... 😕

Sleeping is inherently unreliable.

@JustArchi
Copy link
Contributor

JustArchi commented Sep 5, 2017

Callbacks are queued once they arrive, Steam doesn't give a fuck about proper scheduling them in the connection itself. Especially during post-maintenance times, it's entirely possible that we could get A, B, C callbacks in this order, and on next login B, A, C or C, A, B. This is especially true with all callbacks that are being fired at the same time, usually after successfull OnLogin(). In this case you need sleeping anyway as you can't reliably determine that callback X will always arrive before callback Y if callback Y is not the result of some X action.

Yes, I hate it as well. If that didn't happen, this issue would not pop up, ever, because most of the time OnAccountInfo() is never executed before name initialization. Emphasis on most of the time though.

@yaakov-h
Copy link
Member

yaakov-h commented Sep 5, 2017

FWIW, Sample 4 works fine for me. I'd bet the actual problem here is the blocking inside a of callback handler.

@JustArchi
Copy link
Contributor

JustArchi commented Sep 5, 2017

Sample 4 works for me too. Moreover, my own ASF always worked fine for me, while some minor fraction of the people reported exactly the same issue before I added referenced workaround. I still blame order of callbacks sent by Steam network. There is nothing in SK2 to fix this, apart from making code as much independent of callbacks order as possible.

With #443 we won't need this anymore, which is good.

@yaakov-h
Copy link
Member

With #443 merged, the player name should no longer be changed when setting persona state, and the related race condition should now be fixed.

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

No branches or pull requests

3 participants