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

Manually sync ammo globably on belt reload. #1120

Merged
merged 1 commit into from
May 14, 2015

Conversation

PabstMirror
Copy link
Contributor

Fix #1119 (setAmmo isn't synced immediately)

Belt relinking is rare enough that doing an extra global event shouldn't be that bad.

@PabstMirror PabstMirror added the kind/bug-fix Release Notes: **FIXED:** label May 12, 2015
@PabstMirror PabstMirror added this to the 3.1.0 milestone May 12, 2015
@nicolasbadano
Copy link
Contributor

I think doing this right might be a bit harder. Due to the MG rate of fire, timing is critical for this setammo commands. That's why I'm letting the local unit calculate how much ammo is actually added upon receiving the event.

The set ammo even global event will arrive later to the rest of the units, that will then set ammo count larger than they should (some of the linked ammo will be consumed on the local client by then).

An improvement would be to broadcast the amount of ammo added on the event, and let each client do the additions themselves. That'd make this precise as long as the mg doesn't deplete the ammo during the time the event is transmiting.

@PabstMirror
Copy link
Contributor Author

The setAmmoSync event comes from the local unit.

Events and fired events should both be synchronized // guaranteed-order.

So every client should see the setAmmoSync event and then fireEvents in the exact same order and always see the exact same ammo count on the unit.

@nicolasbadano
Copy link
Contributor

Events and fired events should both be synchronized // guaranteed-order.

Are we sure about that @jaynus ?

I know about the guaranteed order of events, but I thought for some reason that engine events and publicvariable events had their own independent queues. Obviously I'm not sure.

Anyway, I agree with @PabstMirror that a global event is worth it, and shouldn't have any perf cost.

@jaynus
Copy link
Contributor

jaynus commented May 13, 2015

I know about the guaranteed order of events, but I thought for some reason that engine events and publicvariable events had their own independent queues. Obviously I'm not sure.

They share a network queue, so you can always guarantee reception order as the send order in any given frame. I'm not sure about the server propagation side of this (client->server->clients) and whether it maintains order. However, events going out of a system will always be synchronous - the network stack is single order shared.

nicolasbadano added a commit that referenced this pull request May 14, 2015
Manually sync ammo globably on belt reload.
@nicolasbadano nicolasbadano merged commit 2fb317d into master May 14, 2015
@nicolasbadano nicolasbadano deleted the syncAmmoOnBeltReload branch May 14, 2015 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug-fix Release Notes: **FIXED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linking Belts - reloader will get wrong ammo count
3 participants