SteamMatchmaking - creating, retrieving, listing, joining and leaving lobbies. #704

Benjamin-Dobell · 2019-07-15T17:04:39Z

An implementation of SteamMatchmaking.

Currently supported

Since originally opening the PR, the following has been added:

Joining lobbies.
Leaving lobbies.
Inviting another user to a lobby.
Getting data for a specific lobby rather than from a lobby list (probably only useful if you were invited to a lobby).

What's not implemented in this PR

Assigning a game server to a lobby.
Lobby chat.
Steam's "frenemy" stuff, which is officially deprecated and according to ISteamNetworking documentation doesn't actually do anything 🤷‍♂

Related Issues

Closes #656

Closes #487 ... as the requested functionality is necessary to implement Matchmaking. Specifically, when we create a lobby we need to submit our public IP, so we require persistent access to it. Same is true of IPCountryCode.

Ping #131 (only partially implemented, so not closing).

Benjamin-Dobell · 2019-07-15T17:08:36Z

I'm actually using this in conjunction with a almost complete implementation of Steam's P2P (UDP) protocol that I've developed. Combining this PR, with the rest of SteamKit and my Steam P2P implementation is enough to get dedicated servers working for games that use Steam P2P exclusively for networking. Which is precisely why I implemented this and is confirmed as working for Tabletop Simulator.

I will of course open source the P2P stuff at some point, however my gut feel is that it may need to be a stand-alone project that depends on SteamKit2, rather than part of the core. The reason for this is that unlike the rest of SteamKit2, it's specifically designed to be integrated into existing games' run-loops, which means there's a strong focus on performance and avoiding garbage collection. Its API is a bit hairy as a result.

EDIT: Oh, and related to above, in case anyone is interested. I've also back-ported SteamKit2 to .NET 3.5 (https://github.com/Benjamin-Dobell/SteamKit/tree/net35). It's a few months behind upstream master, but surprisingly usable.

azuisleet · 2019-07-15T17:13:35Z

The scope of this seems OK, as additional lobby functionality can be further built on top.

I don't see anything unusual after a quick review, but I do want to double check the usage and rationale of lobby.Clone() in those callback handlers (for the thread safety of lobby.Members mutations).

azuisleet · 2019-07-15T17:19:06Z

Would it make sense to have Members be an immutable list of some sort? It shouldn't be necessary to clone Lobby, as reference assignment should be atomic. It would also prevent accidental mutation of the Members list by any other code.

Benjamin-Dobell · 2019-07-15T17:20:12Z

I don't see anything unusual after a quick review, but I do want to double check the usage and rationale of lobby.Clone() in those callback handlers.

Excellent catch on such a quick review!

I actually just changed it to be "thread-safe by default". In the past the Clone() method existed, but wasn't used anywhere. Instead there was documentation stating that Lobby (specifically Lobby.members) can be mutated out from under you, potentially leading to a crash if you were to simultaneously access it whilst it was being mutated - so you could call Clone() in your handler (running on the same thread as SteamMatchmaking) to avoid that if you wanted a copy of the data.

That approach seemed a little to error prone, so the objects returned in handlers are now immutable and won't ever be mutated. Instead of mutating lobbies we first Clone(), mutate, then update our cached lobby with a new one.

Granted, given Clone() is only used internally it may make sense to either:

Make it internal.
Replace it with a private method on SteamMatchmaking.

Benjamin-Dobell · 2019-07-15T17:23:05Z

Would it make sense to have Members be an immutable list of some sort.

It is actually essentially immutable as currently implemented. IReadOnlyList in the public API, it's just internally we clone, then mutate. However, that's realistically a left-over of my previous rationale that I backtracked on.

I think I should just delete the Clone method and explicitly create a new Lobby each time a member is added or removed. It's done in two places, so a convenience method would be in order still, but I can make that a private member of SteamMatchmaking itself.

Would that be desirable?

Benjamin-Dobell · 2019-07-15T17:28:18Z

SteamKit2/SteamKit2/Steam/Handlers/SteamMatchmaking/Lobby.cs

+                            writer.Write( ( byte )0 );
+                        }
+
+                        ;


Ugh, I'll fix this when I address any other feedback.

azuisleet · 2019-07-15T17:32:43Z

Ah, I see what you mean about the interface being IReadOnlyList, but the implementation itself isn't an immutable collection.

I'm suspicious about the clone - mutate pattern. I don't think it's necessary or desirable to clone or create a new Lobby every time.

I think the implementation should simply create a truly immutable Members list, and assign a new immutable list to Members every time. That way a reference to a Lobby will always be correct, and iterators over Members are never invalidated (because they would always be a reference to the old or new immutable collection)

azuisleet · 2019-07-15T17:38:34Z

Although in this case, lobbies are never mutated in place, so perhaps it may be an unnecessary optimization, and just creating a new Lobby may be appropriate.

Benjamin-Dobell · 2019-07-15T17:48:45Z

I think the implementation should simply create a truly immutable Members list, and assign a new immutable list to Members every time. That way a reference to a Lobby will always be correct, and iterators over Members are never invalidated (because they would always be a reference to the old or new immutable collection)

Ideally I'd rather not mutate any objects returned to users. Is there a precedence for this?

The issue would be with users somewhat naively writing:

// ..lobby is some Lobby retrieved a while back and now being used on another thread.

int count = lobby.Members.Count;
for ( int i = 0; i < count; i++ )
{
    var member = lobby.Members[ i ]; // BOOM!
    // We replaced `Members` with a smaller list mid-iteration because a user left the lobby.
}

I think returning purely immutable objects is safest. However, I definitely think making Clone() public is a mistake - an oversight from my previous line of thinking.

I could instead have something like this in SteamMatchmaking:

private void UpdateLobbyMembers(uint appId, Lobby lobby, IReadOnlyList<Lobby.Member> members)
{
    var updatedLobby = new Lobby(
        lobby.SteamID,
        lobby.lobbyType,
        // ..etc
        members,
        lobby.distance,
        /// ...etc
    );

    var appLobbies = lobbies.GetOrAdd( appId, k => new ConcurrentDictionary<SteamID, Lobby>() );
    appLobbies[ lobby.SteamID ] = updatedLobby;
}

Then call that from HandleUserJoinedLobby and HandleUserLeftLobby with a newly constructed List.

Then I can kill this entire concept of Clone() and it's all just logic internal to SteamMatchmaking.

P.S. Off to bed now, will pick this up tomorrow 😄 Thanks for your time thus far!

azuisleet · 2019-07-15T17:51:04Z

I see the case in that example where they didn't hold an iterator. I think we agree that the Lobby should be wholly immutable, so it's just a matter of details at this point.

Benjamin-Dobell · 2019-07-15T18:14:53Z

SteamKit2/SteamKit2/Steam/Handlers/SteamMatchmaking/Callbacks.cs

+        /// <summary>
+        /// This callback is fired in response to <see cref="SetLobbyData"/>.
+        /// </summary>
+        public class SetLobbyDataCallback : CallbackMsg


Note to self: This and all other callbacks that follow should be sealed.

azuisleet · 2019-07-15T18:19:38Z

To clarify one last point, I think this issue also exists in the implementation as-is. Wouldn't an existing reference to Members have this same issue of mutation by the handler, because I don't think the IReadOnlyList interface provides any guarantees, as the underlying implementation is a mutable List?

Edit: My mistake, Clone is actually creating a new list. That's what I get for not looking.

Ne3tCode · 2019-07-15T19:47:39Z

I don't know C# very well, but it does not seem to work as intended 🤔

internal List<Member> members;
internal Dictionary<string, string> metadata;

internal Lobby( ... Dictionary<string, string> metadata, List<Member> members, ... )
{
    ...
    metadata = metadata ?? new Dictionary<string, string>();
    members = members ?? new List<Member>();
    ...
}

Benjamin-Dobell · 2019-07-16T00:50:32Z

Haha, yeah no knowledge of C# required to notice that's wrong! That's what I get for last second refactoring. Thanks, will fix it shortly.

yaakov-h

I'm not particularly comfortable with the approach taken to thread safety here. It seems like it's trying to do the right thing, but it has internal race conditions doesn't quite enforce immutability.

I'd much rather use System.Collections.Immutable (and perhaps in other places too) to keep things immutable and thread-safe where we can, without a [noticeable] performance impact.

yaakov-h · 2019-07-16T10:21:56Z

SteamKit2/SteamKit2/Steam/Handlers/SteamMatchmaking/Lobby.cs

+                /// <returns>true, if obj is <see cref="Member"/> with a matching SteamID. Otherwise, false.</returns>
+                public override bool Equals( object obj )
+                {
+                    if ( obj is Member )


Use obj is Member member to avoid a second type-check below (when casting).

Ah yeah, sorry, this was running with my .NET 3.5 fork of SteamKit (https://github.com/Benjamin-Dobell/SteamKit/tree/net35) with an old Mono compiler not supporting the syntax. Will make the change 👍

EDIT: Granted, this is a bit similar to my dislike of out var, however there's a valid technical reason for this syntax, so I can handle it. Well, also it's not nearly as bad as out var, constructs followed by a block frequently declare variables that are limited to the scope of the trailing block (for loop etc.)

In theory you should be able to use this syntax and still target .NET 3.5... in practice setting up the SDKs like that can be a pain.

Yeah, you're spot on.

I have managed to bump the language level a couple of times with my tool-chain. I can probably pull this off now and drastically clean up my (very hacky) backport.

My original issue was that I needed MDBs (Mono debug symbols), which requires using xbuild instead of msbuild. However, xbuild can't go above Language Level 6(?).

I ended up getting fed-up with the archaic tooling, swapped to msbuild and threw together a trivial Cecil-based CLI tool to convert the symbols from PDB to MDB (https://github.com/Benjamin-Dobell/pdbtomdb) - pdb2mdb doesn't handle the latest "portable" PDB file format. Ah, the joys of the ever changing .NET tooling ecosystem.

<DebugType>full</DebugType> should get you older PDBs if you need.

Ah, sorry, forgot to mention, I'm (generally) on macOS. Older PDBs are Windows only.

yaakov-h · 2019-07-16T10:22:51Z

SteamKit2/SteamKit2/Steam/Handlers/SteamMatchmaking/Lobby.cs

+                {
+                    SteamID = steamId;
+                    PersonaName = personaName;
+                    Metadata = metadata ?? new Dictionary<string, string>();


Can we have a single empty Dictionary instead of one per Member?

We probably should use AsReadOnly, otherwise consumers can cast the object reference back to it's mutable implementation.

Can we have a single empty Dictionary instead of one per Member?

Can do. Just wasn't sure what the precedent for that was withing this code-base. Very happy to make the change as I do similar in my own code-bases.

We probably should use AsReadOnly, otherwise consumers can cast the object reference back to it's mutable implementation.

I'm not typically a fan of doing of stuff like this. If the user is determined to violate our public contract they can always use reflection etc. It's a bit of fruitless battle.

Just realised I'd missed these two pieces of feedback. Addressing both points now.

yaakov-h · 2019-07-16T10:24:15Z

SteamKit2/SteamKit2/Steam/Handlers/SteamMatchmaking/Lobby.cs

+            /// <returns>A new <see cref="Lobby"/> instance that is equivalent to this one.</returns>
+            public Lobby Clone()
+            {
+                return new Lobby(


How about object.MemberwiseClone()?

yaakov-h · 2019-07-16T10:24:42Z

SteamKit2/SteamKit2/Steam/Handlers/SteamMatchmaking/Lobby.cs

+                    new Dictionary<string, string>( metadata ),
+                    MaxMembers,
+                    NumMembers,
+                    new List<Member>( members ), // We're not doing a deep copy as Member is never mutated.


I don't see where we mutate either the dictionary or the list, so why create new ones?

We mutate the list on line 342, granted I've refactored locally. It's purely immutable now.

yaakov-h · 2019-07-16T10:25:14Z

SteamKit2/SteamKit2/Steam/Handlers/SteamMatchmaking/Lobby.cs

+                {
+                    using ( var writer = new BinaryWriter( ms ) )
+                    {
+                        byte[] header = { 0, 0 };


Consider making both of these static+readonly

👍 Good idea. Will do.

yaakov-h · 2019-07-16T10:31:13Z

SteamKit2/SteamKit2/Steam/Handlers/SteamMatchmaking/SteamMatchmaking.cs

+        void HandleCreateLobbyResponse( IPacketMsg packetMsg )
+        {
+            var lobbyListResponse = new ClientMsgProtobuf<CMsgClientMMSCreateLobbyResponse>( packetMsg );
+            CMsgClientMMSCreateLobbyResponse body = lobbyListResponse.Body;


inconsistent use of var in these functions.

Thanks, I've actually already fixed this locally. Actually, I've fundamentally refactored based on some earlier reviews. Will push shortly.

yaakov-h · 2019-07-16T10:33:09Z

SteamKit2/SteamKit2/Steam/Handlers/SteamMatchmaking/SteamMatchmaking.cs

+
+            lobby = lobby.Clone(); // Cloning to avoid mutating the original Lobby, as it was previously returned in a callback (not thread-safe).
+            lobby.members.Add( member );
+            appLobbies[ lobbyId ] = lobby;


Potential race condition here between lines 323 and now.

Hmm, aren't the handlers always called on the one thread (Client thread)?

I'm not totally certain there's a potential race here. I think most HandleMsg based implementations expect to run in a single thread.

They are, but I'd need to look closer at what the thread safety concerns are here and how they're being addressed, since we have comments like // ... (not thread-safe).

yaakov-h · 2019-07-16T10:33:33Z

SteamKit2/SteamKit2/Steam/Handlers/SteamMatchmaking/SteamMatchmaking.cs

+            var userId = new SteamID( body.steam_id_user );
+
+            var appLobbies = lobbies.GetOrAdd( body.app_id, k => new ConcurrentDictionary<SteamID, Lobby>() );
+            var lobby = appLobbies.ContainsKey( lobbyId ) ? appLobbies[ lobbyId ] : null;


This does two dictionary lookups. Better to use TryGetValue for only one lookup.

yaakov-h · 2019-07-16T10:33:58Z

SteamKit2/SteamKit2/Steam/Handlers/SteamMatchmaking/SteamMatchmaking.cs

+            var userId = new SteamID( body.steam_id_user );
+
+            var appLobbies = lobbies.GetOrAdd( body.app_id, k => new ConcurrentDictionary<SteamID, Lobby>() );
+            var lobby = appLobbies.ContainsKey( lobbyId ) ? appLobbies[ lobbyId ] : null;


TryGetValue as above

yaakov-h · 2019-07-16T10:36:34Z

SteamKit2/SteamKit2/Steam/Handlers/SteamMatchmaking/SteamMatchmaking.cs

+
+            lobby = lobby.Clone(); // Cloning to avoid mutating the original Lobby, as it was previously returned in a callback (not thread-safe).
+            lobby.members.RemoveAt( memberIndex );
+            appLobbies[ lobbyId ] = lobby;


Should we maybe return clones in the callback rather than having this racy clone-and-replace dance internally every time?

I've refactored this based on earlier feedback. There is no longer a concept of clone. This logic has moved into a LobbyCache in a vaguely similar fashion to the FriendCache. Will push soon.

Benjamin-Dobell · 2019-07-16T12:40:25Z

Heads up to anyone following this, given the effort I'm going to refactor and test I figured I may as well just add the remaining requests/callbacks. As such, I'll soon be pushing a complete implementation with the exception of:

Additional filter types; simply because I haven't captured any data for them.
Steam's "frenemy" stuff, which is deprecated and apparently useless anyway.

Jvs34 · 2019-07-16T16:25:04Z

Oh I didn't realize you were also working on this, good job so far.
As far as code goes, I can only give you a tip on the lobby filters, I've looked at the proto packets sent by steamworks and it seems like everything is casted as a string and the filter types that I've seen are implemented in the code snippet below.

https://github.com/Jvs34/SteamKit/blob/c2379a80ef10d36c084e56dd5dd2bc1abefb30a3/SteamKit2/SteamKit2/Steam/Handlers/SteamMatchMaking/SteamMatchMaking.cs#L123-L186

Not sure if I'm actually helping here or not, just wanted to pitch in as it seems you've been making more progress that I did.

Benjamin-Dobell · 2019-07-16T16:30:52Z

@Jvs34 Thanks, those filter snippets are indeed very helpful. I only had captures of the 3 filters I've implemented. Will happily add those ones in.

At the moment I'm just trying to sort out chat, which has proved painful to capture because it has a bazillion weird variants (https://partner.steamgames.com/doc/api/steam_api#EChatEntryType). Tempted to forego it for now. Will see.

codecov-io · 2019-07-17T00:21:32Z

Codecov Report

Merging #704 into master will decrease coverage by 1.71%.
The diff coverage is 2.63%.

@@            Coverage Diff             @@
##           master     #704      +/-   ##
==========================================
- Coverage   24.37%   22.65%   -1.72%     
==========================================
  Files          89       93       +4     
  Lines        8800     9560     +760     
  Branches      742      794      +52     
==========================================
+ Hits         2145     2166      +21     
- Misses       6522     7262     +740     
+ Partials      133      132       -1

Impacted Files	Coverage Δ
...SteamKit2/Steam/Handlers/SteamMatchmaking/Lobby.cs	`0% <0%> (ø)`
...mKit2/Steam/Handlers/SteamMatchmaking/Callbacks.cs	`0% <0%> (ø)`
...Kit2/Steam/Handlers/SteamMatchmaking/LobbyCache.cs	`2.43% <2.43%> (ø)`
...eamKit2/SteamKit2/Steam/SteamClient/SteamClient.cs	`44.6% <20%> (-0.62%)`	⬇️
...team/Handlers/SteamMatchmaking/SteamMatchmaking.cs	`3.82% <3.82%> (ø)`
SteamKit2/SteamKit2/Types/SteamID.cs	`80.86% <0%> (+0.36%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97b2a99...b43363b. Read the comment docs.

Benjamin-Dobell · 2019-07-17T00:26:23Z

Not ready to merge

Still a WIP, mostly regarding chat messages, which I'm admittedly a bit confused about at present.

However, still a couple of house keeping things to do as well e.g. KeyValue instead of custom metadata encoding/decoding.

I've force pushed here so I can get some feedback on the direction. Admittedly, that's going to be a pain to diff, so here's the actual change log:

https://github.com/Benjamin-Dobell/SteamKit/commits/matchmaking2

Benjamin-Dobell · 2019-07-17T00:29:27Z

P.S. This is totally untested at present. I'm still working on the aforementioned integration tests. I've pushed purely so I can confirm I'm at least taking this in the right direction design-wise. Granted, the changes are technically less efficient (more ConcurrentDictionary look-ups), but it's a lot more concise and easier to reason about, so I think it's a worthwhile trade-off.

SteamKit2/SteamKit2/Steam/Handlers/SteamMatchmaking/Lobby.cs

Benjamin-Dobell · 2019-07-22T19:17:25Z

This is now ready for review and subsequent merging.

Integration tests

Integration test are available at:

https://github.com/Benjamin-Dobell/SteamKitIntegrationTests

README has instructions. They can be run easily as a Docker container or built and run in an IDE.

However, to specifically run the tests against this PR you'd do:

docker build -f dockerfiles/Dockerfile -t steamkit2_tests .
docker run -e PRIMARY_STEAM_USER=account1 -e PRIMARY_STEAM_KEY=key1 -e SECONDARY_STEAM_USER=account2 -e SECONDARY_STEAM_KEY=key2 -e "STEAMKIT_REPO=https://github.com/Benjamin-Dobell/SteamKit.git" -e STEAMKIT_REF=matchmaking steamkit2_tests

Note that you must provide credentials for two Steam accounts. These can be free (limited) accounts (lobbies are created against DOTA2 which works because it is also free).

What's still not included in this PR

Additional filters.
Assigning a game server to a lobby.
Lobby chat.
Steam's deprecated "frenemy" functionality.

I basically just don't have a decent way to capture additional filters or chat (nor am I particularly interested in the functionality), however they should be reasonably easy to add on.

Merging/Review

Changes have been squashed to facilitate merging and a clean history. However, change-sets since previous reviews are available at:

https://github.com/Benjamin-Dobell/SteamKit/commits/matchmaking2

Benjamin-Dobell · 2019-08-10T14:45:14Z

@yaakov-h Yeah, I felt a bit weird about namespaces/scoping as well e.g. using partial classes for non-generated code seemed a bit unnecessary. I was trying to follow established patterns, however this is certainly one of the more involved handlers. So I think I've stretched the current code layout pattern to its limits.

I'd have absolutely zero issue with someone coming along and reorganising.

yaakov-h · 2019-08-16T02:06:34Z

Thanks a bunch!

If you still want to implement the remaining few items mentioned in the OP, please open a new PR.

If you don't, that's perfectly fine as well. 😄

Tyaap · 2020-03-30T17:09:34Z

Hello,

I am working on a project that requires checking if a lobby with a given Steam ID exists, and if so retrieve its data. I believe this requires the RequestLobbyData() function and LobbyDataUpdate_t callback to check for success. Is it possible for these to be implemented?

Benjamin-Dobell · 2020-03-30T22:27:37Z

@Tyaap Everything you need is already implemented. It's a simple, fairly self explanatory API, as such please refer to the changes in this PR to find the appropriate methods and callbacks.

Tyaap · 2020-03-31T08:54:54Z

Hello Benjamin,

I am aware that lobbies may be checked for existence by calling GetLobbyList and searching the returned list. However there is a problem when the lobby is full: GetLobbyList never returns these lobbies. I have tried adding a filter to request lobbies with >= 0 slots but this does not help. So it seems I can't check if a full lobby exists using this approach. Also I can't use GetLobbyData since this only works for lobbies already retrieved via GetLobbyList. The approach mentioned previously should work, since the Steamworks documentation says that RequestLobbyData does not require a prior call to GetLobbyList. However this function does not appear to be present in SteamKit. Is there something I'm still not aware of that can resolve this problem?

Benjamin-Dobell · 2020-03-31T09:04:45Z

Steamworks documentation applies to Steamworks, not SteamKit. SteamKit is a C# library with C# semantics. What you want to do is already possible, as previously suggested, please refer to the changes made in this PR.

Tyaap · 2020-03-31T13:50:26Z

I have read the code in SteamMatchmaking, which seems to suggest using GetLobbyData. However I have tried using the implementation of GetLobbyData in SteamKit (without a prior call of GetLobbyList) and the client is immediately disconnected from Steam.

Benjamin-Dobell · 2020-03-31T14:18:42Z

@Tyaap How did you retrieve the steam ID for this lobby? Steam will only let you retrieve data for lobbies you should know about. That means those publicly available in the lobby list, or a friend's lobby. Steam's backend won't allow you request the data for an arbitrary lobby.

By the way, there is no such API as "RequestLobbyData". Steamworks's RequestLobbyData() function would just be calling the same API we're calling behind the scenes in our GetLobbyData(), which is called ClientMMSGetLobbyData.

Steamworks RequestLobbyData() will just be wrapping that call with a local check to see if it's a lobby you're allowed to request. If not, I imagine it prevents the request being sent in the first place, rather than disconnecting you.

Now, technically, even if your friend is in a lobby, you can only know this information if you've requested their PersonaState. Steam's back-end is very good at keeping track of what information you should know. So if you haven't requested said friend's persona state, then a disconnection is probably in order.

There is actually some related functionality that is missing from SteamKit. Specifically, PersonaStateCallback does not expose the lobby ID. If you'd like to implement that, it'd be two line (+ 4 lines of documentation) change.

Still though, this is only ever going to work for lobbies that your friends are in; or users which Steam's backend has for some reason allowed you to request the persona state of.

EDIT: By the way, seems as you've been referring to Steamwork's documentation for RequestLobbyData, it does actually cover this i.e.

You can use this to refresh lobbies that you have obtained from RequestLobbyList or that are available via friends.

Tyaap · 2020-03-31T15:31:29Z

Thanks for the information. I am retrieving the lobby list on a regular basis using GetLobbyList, and storing the details. The goal is to keep track of all active lobbies. Sometimes on retrieving a new list a lobby will disappear. It will either disappear because:

It has been deleted.
The lobby is full.
I wish to check which case it is, and retrieve the lobby data in case 2.

For reference, this is the relevant code I'm working on:
https://github.com/Tyaap/SuperLobbyBot/blob/3583d8242ab8a4bff7694895be0fb191cf6e9f05/Steam.cs#L388

Tyaap · 2020-03-31T22:35:01Z

So after reviewing your message, it appears futile to try work around the "disappearance" of full lobbies from the list retrieved from GetLobbyList. The Steamworks documentation states that hiding full lobbies is the "default" behaviour, but gives no indication that this default behaviour can be changed. Are you aware of any part of your Matchmaking implementation which may allow me to retrieve information about full lobbies?

Tyaap · 2020-03-31T22:47:46Z

I have tried this, but no luck: https://github.com/Tyaap/SuperLobbyBot/blob/d207fb2f852b95c40e822bb5d1da10db4a8f9199/Steam.cs#L171

Tyaap · 2020-04-01T13:02:34Z

Update: I have tried using the native Steamworks API (via Steamworks.NET) and have successfully kept track of full lobbies using the RequestLobbyData() function. However the behaviour is somewhat unexpected: the requested lobby data is not retrieved until I call RetrieveLobbyList(). This may be caused by Steamworks.NET but I can't tell.

I have created a proof of concept project: https://github.com/Tyaap/LobbyTrackingPOC/blob/master/Program.cs
I wish to be able to do the same as this with SteamKit, but it is not clear how this can be done.

Benjamin-Dobell · 2020-04-01T13:04:27Z

@Tyaap Just to clarify, you've confirmed the data returned is up-to-date i.e. it's not just serving you cached data (before the lobby was full)?

Benjamin-Dobell · 2020-04-01T13:34:07Z

@Tyaap Assuming you're not receiving stale data, then what I suspect is happening is that Steam is just sending the client a regular lobby update for any lobbies that are no longer in the latest lobby list. Your call to RequestLobbyData() will be unrelated.

Have you subscribed for regular lobby data updates (LobbyDataCallback)?

Tyaap · 2020-04-01T15:28:22Z

I can confirm it is up to date, because the player count is reported as e.g. 4/4. It must have been less than this previously, otherwise it wouldn't have shown up in a prior retrieval of the lobby list. This data is not retrieved unless I call RequestLobbyData().

In SteamKit, I have tried subscribing to LobbyDataCallback. This is only getting triggered if I run GetLobbyData() on a lobby that was retrieved from GetLobbyList(). The test I have done is: run GetLobbyList() and periodically execute GetLobbyData() on each of the returned lobbies. Eventually this resulted in a Steam disconnection, which I believe is because the lobby disappears. I am unable to confirm this though, or determine whether its due to a lobby that truly no longer exists or one that is just full. I've made a test project for that and will upload it shortly.

Tyaap · 2020-04-01T15:49:12Z

https://github.com/Tyaap/GetLobbyDataTest/blob/master/Program.cs

Benjamin-Dobell · 2020-04-01T16:41:34Z

I was just thinking that the idea of Steam disconnecting you simply for requesting data for a lobby you'd previously been told about (but is now full) leading to a disconnection seemed a bit dubious. Because that'd be susceptible to all sorts of race conditions and would likely break standard Steam clients...

This does not appear to be the case.

I don't know why Steam is disconnecting you, however here's a very simple test that demonstrates you can query data for full lobbies you've previously seen in a lobby list, but are not being returned in the latest lobby list.

https://github.com/Benjamin-Dobell/SteamKitIntegrationTests/blob/full-lobby-data/SteamKitIntegrationTests/Program.cs#L38-L98

To run the tests, checkout https://github.com/Benjamin-Dobell/SteamKitIntegrationTests/tree/full-lobby-data (Note: full-lobby-data branch). You can either follow the README to run it in a Docker container, or create a .env file in the project (./SteamKitIntegrationTests) directory with the contents:

PRIMARY_STEAM_USER=your_first_steam_account
PRIMARY_STEAM_PASSWORD=some_secret_password1
SECONDARY_STEAM_USER=your_second_steam_account
SECONDARY_STEAM_PASSWORD=some_secret_password2

or _KEY instead of _PASSWORD if you prefer.

Then either run/debug the singular test from your IDE of choice, or from command line with:

dotnet restore
dotnet build /p:Configuration=Debug /p:EnableSourceLink=false
dotnet test --no-build

Tyaap · 2020-04-01T17:19:11Z

Thank you very much for the test code. :) Unfortunately I can't test it (without further work) as I'm getting AccountLogonDenied due to my Steam Guard. I've had a look and so far haven't found anything within the code snippet that is majorly different to what I have been trying to do, which is very odd. I will have to investigate some more.

Tyaap · 2020-04-01T17:38:20Z

I suspect the disconnects are caused by attempting to get data for a lobby that truly no longer exists. If this is the case, then there is still an issue of being able to differentiate between these two cases in order to avoid being disconnected.

Benjamin-Dobell · 2020-04-01T17:39:30Z

@Tyaap You're being disconnected because you're requesting a deleted lobby, not a full one. Just verified this happens.

However, Steam does send you updates automatically (without you explicitly requesting) for lobbies that you've previously received data for but have since been deleted.

Now, that still seems a bit off to me, because there's potential for a race condition in that you could go to request the data for a lobby, but it's just deleted.

I'm thinking the only way the real Steam client could get around this is that Steam's backend will allow you to make this request, until your client has confirmed receiving the packet that indicates the lobby has been deleted. Then when processing its queue Steam would see a queued outbound message pertains to a deleted resource (lobby), delete the message, and return the data from the lobby cache.

That's actually insanely convoluted, so I'd love to hear I'm wrong and there's a simpler way.

If Steam are doing the above, then we have an huge problem on our hands, as our send and receive logic operate in different threads. Everything is event driven, we really can't process everything serially like that without enormous changes to the architecture of SteamKit 😕

I'd suggest you play around and ensure you're seeing what I am, if you are, you're going to need to open an issue... but ah, if it really is the above, I wouldn't expect it to be fixed anytime soon.

EDIT: Posts crossed.

Tyaap · 2020-04-01T19:40:43Z

Okay I am getting the same results as you for the case of a full lobby and the case of a deleted lobby. :(
This is certainly a major over-complication on Steam's part. It seems almost absurd to implement all that when a success/deleted/denied status code on GetLobbyData would have been sufficient to indicate active/deleted lobbies without revealing info about lobbies not previously received.

Tyaap · 2020-04-01T19:44:58Z

I will open an issue about this. For now I have thought of a workaround although its not pullet proof. I will just assume that any lobbies that disappear with less than e.g. MaxMembers/2 was a deleted lobby, and a lobby with more than that is a full lobby. I will be updating the lobby list every 10 seconds and the lobbies always have a max of 10 players in the game I'm working on, so hopefully the likelihood of these assumptions being incorrect is negligible.

Tyaap · 2020-04-01T19:57:04Z

Do you have code that I can refer to in the new issue, which can quickly reproduce it?

Benjamin-Dobell · 2020-04-01T20:14:02Z

Sure:

Benjamin-Dobell/SteamKitIntegrationTests@bd8fd31

The failing code is commented out, the code that's there is actually guaranteed to work in this circumstance.

Although far from ideal, it's probably worth keeping in mind that the likelihood requesting lobby data just as a lobby is deleted and before LobbyDataCallback is fairly small - well, depending on how many lobbies you're monitoring.

Rather than doing the weirdness you suggested above with guessing about lobby states, it'd probably be easier just to throw your Bot up behind foreman and restart it if it crashes. Could split the Steam logic out into a separate process/container, so if the Steam querying component goes down, your Bot doesn't go down itself.

Well, unless you're worried about being banned by Valve. I imagine making bad requests does raise some eyebrows - even if they are infrequent.

Tyaap · 2020-04-01T20:42:33Z

Okay thanks for the help. I have just read through your code. I was not aware that after retrieving a lobby list we get LobbyDataCallbacks from Steam to update the lobby data, including one that changes the steam ID of the lobby owner to 0 to indicate the destruction of a lobby. I will have to change the issue description again...

Benjamin-Dobell · 2020-04-01T20:46:55Z

Unfortunately, it seems that it's not simply retrieving the lobby list that sets you up for lobby updates. If it was, you'd actually have a 100% crash-free solution on offer, with the ever so slight possibility of mislabelling a deleted lobby as full for a few milliseconds i.e. Never call GetLobbyData().

However, it would seem you only receive deletion updates if you've called GetLobbyData() on the lobby at least once before - and of course, as we've discussed, we can't guarantee it's safe to call even once.

Benjamin-Dobell commented Jul 15, 2019

View reviewed changes

yaakov-h reviewed Jul 16, 2019

View reviewed changes

Benjamin-Dobell force-pushed the matchmaking branch from 36eba69 to 8eb6b97 Compare July 17, 2019 00:21

azuisleet changed the title ~~SteamMatchmaking - support for getting and creating lobbies~~ WIP: SteamMatchmaking - support for getting and creating lobbies Jul 17, 2019

Benjamin-Dobell commented Jul 17, 2019

View reviewed changes

SteamKit2/SteamKit2/Steam/Handlers/SteamMatchmaking/Lobby.cs Outdated Show resolved Hide resolved

yaakov-h added do not merge enhancement labels Jul 17, 2019

Benjamin-Dobell force-pushed the matchmaking branch from 8eb6b97 to 68eb5a9 Compare July 22, 2019 14:24

Benjamin-Dobell changed the title ~~WIP: SteamMatchmaking - support for getting and creating lobbies~~ SteamMatchmaking - support for getting and creating lobbies Jul 22, 2019

Benjamin-Dobell changed the title ~~SteamMatchmaking - support for getting and creating lobbies~~ SteamMatchmaking - creating, retrieving, listing, joining and leaving lobbies. Jul 22, 2019

yaakov-h merged commit 54205ea into SteamRE:master Aug 16, 2019

xPaw mentioned this pull request Nov 2, 2019

Get remote IP for current connection #487

Closed

azuisleet mentioned this pull request Nov 2, 2019

Expose remote endpoint for current connection #775

Closed

Tyaap mentioned this pull request Apr 1, 2020

Disconnection when calling GetLobbyData() on a deleted lobby. #826

Closed

SteamMatchmaking - creating, retrieving, listing, joining and leaving lobbies. #704

SteamMatchmaking - creating, retrieving, listing, joining and leaving lobbies. #704

Conversation

Benjamin-Dobell commented Jul 15, 2019 • edited Loading

Currently supported

What's not implemented in this PR

Related Issues

Benjamin-Dobell commented Jul 15, 2019 • edited Loading

azuisleet commented Jul 15, 2019 • edited Loading

azuisleet commented Jul 15, 2019

Benjamin-Dobell commented Jul 15, 2019 • edited Loading

Benjamin-Dobell commented Jul 15, 2019

Choose a reason for hiding this comment

azuisleet commented Jul 15, 2019

azuisleet commented Jul 15, 2019 • edited Loading

Benjamin-Dobell commented Jul 15, 2019 • edited Loading

azuisleet commented Jul 15, 2019

Choose a reason for hiding this comment

azuisleet commented Jul 15, 2019 • edited Loading

Ne3tCode commented Jul 15, 2019

Benjamin-Dobell commented Jul 16, 2019

yaakov-h left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Benjamin-Dobell Jul 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Benjamin-Dobell commented Jul 16, 2019

Jvs34 commented Jul 16, 2019 • edited Loading

Benjamin-Dobell commented Jul 16, 2019 • edited Loading

codecov-io commented Jul 17, 2019 • edited Loading

Codecov Report

Benjamin-Dobell commented Jul 17, 2019

Benjamin-Dobell commented Jul 17, 2019 • edited Loading

Benjamin-Dobell commented Jul 22, 2019

Integration tests

What's still not included in this PR

Merging/Review

Benjamin-Dobell commented Aug 10, 2019 • edited Loading

yaakov-h commented Aug 16, 2019

Tyaap commented Mar 30, 2020 • edited Loading

Benjamin-Dobell commented Mar 30, 2020

Tyaap commented Mar 31, 2020 • edited Loading

Benjamin-Dobell commented Mar 31, 2020

Tyaap commented Mar 31, 2020 • edited Loading

Benjamin-Dobell commented Mar 31, 2020 • edited Loading

Tyaap commented Mar 31, 2020

Tyaap commented Mar 31, 2020 • edited Loading

Tyaap commented Mar 31, 2020

Tyaap commented Apr 1, 2020 • edited Loading

Benjamin-Dobell commented Apr 1, 2020

Benjamin-Dobell commented Apr 1, 2020

Tyaap commented Apr 1, 2020 • edited Loading

Tyaap commented Apr 1, 2020

Benjamin-Dobell commented Apr 1, 2020 • edited Loading

Tyaap commented Apr 1, 2020

Tyaap commented Apr 1, 2020

Benjamin-Dobell commented Apr 1, 2020 • edited Loading

Benjamin-Dobell commented Jul 15, 2019 •

edited

Loading

Benjamin-Dobell commented Jul 15, 2019 •

edited

Loading

azuisleet commented Jul 15, 2019 •

edited

Loading

Benjamin-Dobell commented Jul 15, 2019 •

edited

Loading

azuisleet commented Jul 15, 2019 •

edited

Loading

Benjamin-Dobell commented Jul 15, 2019 •

edited

Loading

azuisleet commented Jul 15, 2019 •

edited

Loading

Benjamin-Dobell Jul 16, 2019 •

edited

Loading

Jvs34 commented Jul 16, 2019 •

edited

Loading

Benjamin-Dobell commented Jul 16, 2019 •

edited

Loading

codecov-io commented Jul 17, 2019 •

edited

Loading

Benjamin-Dobell commented Jul 17, 2019 •

edited

Loading

Benjamin-Dobell commented Aug 10, 2019 •

edited

Loading

Tyaap commented Mar 30, 2020 •

edited

Loading

Tyaap commented Mar 31, 2020 •

edited

Loading

Tyaap commented Mar 31, 2020 •

edited

Loading

Benjamin-Dobell commented Mar 31, 2020 •

edited

Loading

Tyaap commented Mar 31, 2020 •

edited

Loading

Tyaap commented Apr 1, 2020 •

edited

Loading

Tyaap commented Apr 1, 2020 •

edited

Loading

Benjamin-Dobell commented Apr 1, 2020 •

edited

Loading

Benjamin-Dobell commented Apr 1, 2020 •

edited

Loading

Tyaap commented Apr 1, 2020 •

edited

Loading

Tyaap commented Apr 1, 2020 •

edited

Loading