Skip to content
This repository has been archived by the owner on Nov 16, 2022. It is now read-only.

Fixing initialization errors and unity log errors. Also fixes a bug with int encoding of the frames in backroll connection. #5

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

Conversation

chunloklo
Copy link

Fixing Unity debug formatting errors, various initialization errors, and encoding errors for backroll connection.
Unity doesn't like "{}" when using Debug.Format so I added indices.

@@ -48,6 +48,8 @@ public unsafe class P2PBackrollSession<T> : BackrollSession<T> where T : struct

public bool IsSynchronizing {
get {
// Always synchronized
return false;
Copy link
Member

Choose a reason for hiding this comment

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

Is this correct? I don't think this is true even in the C++ implementation for GGPO.

@@ -325,13 +336,18 @@ public unsafe class P2PBackrollSession<T> : BackrollSession<T> where T : struct
if (_localConnectStatus[queue].Disconnected) return;
int current_remote_frame = _localConnectStatus[queue].LastFrame;
int new_remote_frame = input.Frame;


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 space here.

BitVector.WriteNibblet(msg.bits, i, ref offset);
Debug.Log($"[BackrollConnection][Nibblet][Set] Nibblet Set! Frame {j + msg.StartFrame}, Bit: {i}, On: {current[i]}");
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Please adhere to the project style convention and use K&R style bracketing.

Also is this level of logging advised? This looks like it may flood the log with noisy output. If this was just to help debug the library itself. I strongly suggest removing them.

}

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


// Steam is always synchronized already!
_current_state = State.Running;
//SendSyncRequest();
Copy link
Member

Choose a reason for hiding this comment

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

This is not localized to only a Steam implementation, and thus not a safe assumption. This is safe if HouraiNetworking ensures that the underlying transport is synchronized across all transport implementations.

{
data = (data & 1) | (uint)(value << 1);
};
}
Copy link
Member

Choose a reason for hiding this comment

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

Not sure what this change is doing. Please explain, or leave a comment explaining this.

for (int i = 0; i < _localConnectStatus.Length; i++) {
_localConnectStatus[i].LastFrame = -1;
}

Copy link
Member

Choose a reason for hiding this comment

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

What does this initialization change do? What was broken before?

Copy link

@christopherliu830 christopherliu830 Jan 10, 2021

Choose a reason for hiding this comment

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

Previous code checked PlayerCount on line 87 (_players.Length) before _players was assigned (line 94)
also, SetupConnection tries to get the index of the player in _players so it can't be used during the assignment to _players

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

Successfully merging this pull request may close these issues.

3 participants