-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Added beacon name sync #959
Added beacon name sync #959
Conversation
90f06ba
to
3301427
Compare
3301427
to
e3a525b
Compare
d94f4bc
to
baa2e89
Compare
|
||
do | ||
// Some packets should not fire during game session join but only afterwards so that initialized/spawned game objects don't trigger packet sending again. | ||
using (packetSender.Suppress<PingRenamed>()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add this closer to the code that invokes this?
if (!obj.HasValue) | ||
{ | ||
// Not the object we're looking for. | ||
return; | ||
} | ||
Beacon beacon = obj.Value.GetComponent<Beacon>(); | ||
if (!beacon) | ||
{ | ||
// This can be ok if origin of ping instance component was not from a beacon (but from signal or other). | ||
return; | ||
} | ||
if (beacon.GetComponent<Player>()) | ||
{ | ||
// Skip over beacon component on player GameObjects | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log the ones that are error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of these have to be errors. And this information doesn't seem useful most of the time. I wish we had some .Trace()
or just disable logs from this class by default.
public string Name { get; } | ||
|
||
[ProtoMember(3)] | ||
public byte[] BeaconGameObjectSerialized { get; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do all pings have beacons?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, all beacons have pings.
baa2e89
to
109748e
Compare
Suppressed the ping/beacon rename packet on initial sync so that connecting players don't fire beacon rename packets.
Initially I used
PingManager.onRenamed
event but doing it this way would deviate from the standard patching mechanism so I went with a patch instead.