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

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

Conversation

chunloklo
Copy link

Updating steam lobbies to work with latest Facepunch Steamworks api.
Makes sure that lobby data is set for steamlobbies and that member data is updated.

@@ -27,10 +27,14 @@ internal class SteamLobby : Lobby {

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.

}
//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.

@@ -63,7 +67,7 @@ internal class SteamLobby : Lobby {
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.

@@ -27,6 +27,8 @@ internal class SteamLobbyManager : ILobbyManager {
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.

@@ -73,7 +75,7 @@ internal class SteamLobbyManager : ILobbyManager {
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.

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.

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)

@@ -217,6 +220,8 @@ class LobbySearchBuilder : ILobbySearchBuilder {
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.

@@ -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.

@james7132
Copy link
Member

OK this looks good enough, sans the SendChatBytes change. This is just pending an updated Facepunch UPM package to merge this in.

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

Successfully merging this pull request may close these issues.

None yet

2 participants