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.

Sign up for GitHub

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

Integrating with the latest Facepunch Steamworks API and populating steam lobby class. #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Integrating with the latest Facepunch Steamworks API and populating steam lobby class. #5

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3075415
Updating steam lobbies to work with latest facepunch steamworks api a…
chunloklo Dec 28, 2020
572a6f7
fixing few things based on comments
chunloklo Jan 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions Runtime/Integrations/Steam/SteamLobby.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,14 @@ public override int Capacity {

readonly SteamLobbyManager _manager;

public SteamLobby(Steamworks.Data.Lobby lobby, SteamLobbyManager manager) : base() {
public SteamLobby(Steamworks.Data.Lobby lobby, SteamLobbyManager manager, bool connected) : base() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this default to false.

Assert.IsNotNull(manager);
_manager = manager;
Members.Refresh();
_lobby = lobby;
if (connected)
{
Members.Refresh();
}
}

public override int MemberCount => _lobby.MemberCount;
Expand Down Expand Up @@ -63,7 +67,7 @@ internal override void SetMemberMetadata(AccountHandle handle, string key, strin
if (handle.Id != UserId) {
throw new InvalidOperationException("Cannnot set the metadata of a Steam lobby member other than the current user.");
}
_lobby.SetMemberData(new Friend(handle.Id), key, value);
_lobby.SetMemberData( key, value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the extra space before key.

}

internal override void DeleteMemberMetadata(AccountHandle handle, string key) =>
Expand Down Expand Up @@ -96,9 +100,9 @@ internal override unsafe void SendNetworkMessage(AccountHandle target, ReadOnlyS

public override unsafe void SendLobbyMessage(ReadOnlySpan<byte> msg) {
fixed (byte* ptr = msg) {
if (!_lobby.SendChatBytes(ptr, msg.Length)) {
Debug.LogError("Failed to send Steam Lobby Packet.");
}
//if (!_lobby.SendChatBytes(ptr, msg.Length)) {
// Debug.LogError("Failed to send Steam Lobby Packet.");
//}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason this was commented out?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sending chat bytes are no longer supported through facepunch. I'm just going to throw a NotImplemented error then.

}
}

Expand Down
17 changes: 11 additions & 6 deletions Runtime/Integrations/Steam/SteamLobbyManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ public SteamLobbyManager() {
SteamMatchmaking.OnLobbyMemberDisconnected += OnLobbyMemberLeave;
SteamMatchmaking.OnLobbyMemberKicked += OnLobbyMemberRemoved;
SteamMatchmaking.OnLobbyMemberBanned += OnLobbyMemberRemoved;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove extra line here.

SteamMatchmaking.OnLobbyMemberDataChanged += OnLobbyMemberDataChanged;
}

public unsafe void Update() {
Expand Down Expand Up @@ -61,7 +63,7 @@ public async Task<Lobby> CreateLobby(LobbyCreateParams createParams) {
steamLobby.SetData(kvp.Key, kvp.Value.ToString());
}
}
var lobby = new SteamLobby(steamLobby, this);
var lobby = new SteamLobby(steamLobby, this, true);
_connectedLobbies.Add(steamLobby.Id, lobby);
return lobby;
}
Expand All @@ -73,7 +75,7 @@ public async Task<IList<Lobby>> SearchLobbies(Action<ILobbySearchBuilder> builde
if (result == null) return new Lobby[0];
var lobbies = new Lobby[result.Length];
for (var i = 0; i < lobbies.Length; i++) {
lobbies[i] = new SteamLobby(result[i], this);
lobbies[i] = new SteamLobby(result[i], this, false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the default has been set to false, remove this change.

}
return lobbies;
}
Expand All @@ -87,14 +89,14 @@ public LobbySearchBuilder(LobbyQuery query) {
}

public ILobbySearchBuilder Filter(string key, SearchComparison comparison, string value) {
var comp = (LobbyComparison)((int)comparison);
_query.AddStringFilter(key, value, comp);
//var comp = (LobbyComparison)((int)comparison);
//_query.AddStringFilter(key, value, comp);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actively breaks the search builder. Is this now deprecated?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup it's deprecated now. I'm going to fix it to work with the new API then.

return this;
}

public ILobbySearchBuilder Filter(string key, SearchComparison comparison, int value) {
var comp = (LobbyComparison)((int)comparison);
_query.AddNumericalFilter(key, value, comp);
//var comp = (LobbyComparison)((int)comparison);
//_query.AddNumericalFilter(key, value, comp);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here with this commented out section.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above. See the new merge (I'll push it in a bit)

return this;
}

Expand Down Expand Up @@ -124,6 +126,7 @@ public void Dispose() {
SteamMatchmaking.OnLobbyMemberDisconnected += OnLobbyMemberLeave;
SteamMatchmaking.OnLobbyMemberKicked += OnLobbyMemberRemoved;
SteamMatchmaking.OnLobbyMemberBanned += OnLobbyMemberRemoved;
SteamMatchmaking.OnLobbyMemberDataChanged += OnLobbyMemberDataChanged;
}

internal async Task JoinLobby(SteamLobby lobby) {
Expand Down Expand Up @@ -217,6 +220,8 @@ void OnLobbyMemberDataChanged(Steamworks.Data.Lobby steamLobby, Friend friend) {
Debug.LogWarning($"[Steam] Unexpected lobby member update event for lobby: {steamLobby.Id}");
return;
}

Debug.Log("member changed!!!");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is just to debug. Please remove this.

member.DispatchUpdate();
}

Expand Down
2 changes: 1 addition & 1 deletion Runtime/Lobbies/LobbyMember.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class LobbyMember : INetworkConnection, IMetadataContainer, IDisposable {
public static IMessageProcessor MessageProcessor;

static LobbyMember() {
MessageProcessor = new LZFCompressor();
MessageProcessor = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason this was removed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using LZFCompressor instantly crashes Unity. I'm not sure where it goes wrong so I just commented it out.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, that's a sign we have a memory access violation somewhere in our code. I think we can keep this for now, but please leave a TODO(james7132) comment to investigate this. The LZF compression is part of the reason why we have been able to keep our bandwidth usage low despite sending a network message basically every tick in Backroll.

}

public enum ConnectionState {
Expand Down