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

MultiplayerSynchronizer - Add an option to sync properties only when they are changed #4429

Closed
nathanfranke opened this issue Apr 23, 2022 · 18 comments
Milestone

Comments

@nathanfranke
Copy link

nathanfranke commented Apr 23, 2022

Describe the project you are working on

Real Time Strategy, but this applies to almost all multiplayer game types

Describe the problem or limitation you are having in your project

A lot of network bandwidth is consumed idly by properties that rarely change.

Examples:

  • Whether the game has started
  • What map is being played
  • Player names/info

I could increase the MultiplayerSynchronizer interval (e.g. to 1 second) but that means all changes will be delayed up to 1 second later.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

It is important to have the current system as an option since it is unreliable (faster) and works great for stuff like player position.

I propose an alternate option that only updates when needed and does so reliably.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

This proposal serves as more of a discussion topic so I'm not strictly proposing any UI.

image

If this enhancement will not be used often, can it be worked around with a few lines of script?

The only way to work around this currently is writing setter RPCs rather than using the synchronizer feature, which requires a lot of boilerplate code.

Is there a reason why this should be core and not an add-on in the asset library?

There is already a synchronizer in core, so it makes more sense to extend it.

CC @Faless

@Xrayez
Copy link
Contributor

Xrayez commented Apr 23, 2022

I agree it should be there. I proposed to add this 3 years ago (the feature itself including the CHANGED option), see godotengine/godot#16793 (comment).

@nathanfranke
Copy link
Author

Another use case example:

func start_game() -> void:
	for player in players:
		# Fill in player spawn location if needed
		if player.spawn_location == Vector2.ZERO:
			# This is synced by the MultiplayerSynchronizer
			player.spawn_location = Vector2(randf(), randf())
	
	on_game_started.rpc()

@rpc func on_game_started() -> void:
	# Might not have been synced yet
	print("Setting camera position to ", get_self().spawn_location)

spawn_location probably won't be synced by the time the RPC goes through. If the variable is watched and immediately synced, however, this wouldn't be an issue.

@Faless
Copy link

Faless commented Jul 7, 2022

If the variable is watched and immediately synced, however, this wouldn't be an issue.

Well, that would actually be an issue.
Even if we add watched variables, they would still not be synced immediately, since that would mean generating one reliable packet for every changed property, which most definitely you do not want.

@nathanfranke
Copy link
Author

nathanfranke commented Jul 7, 2022

How would you suggest resolving the issue in my use case?

I don't really see an issue with sending a reliable packet when a variable is changed. If the user doesn't want the transfer to be reliable, they can use the unreliable synchronization.

@Faless
Copy link

Faless commented Jul 7, 2022

I don't really see an issue with sending a reliable packet when a variable is changed. If the user doesn't want the transfer to be reliable, they can use the unreliable synchronization.

The problem is that reliable packets, to be reliable, have to be acknowledged.

So if I change 5 properties in one frame, and we send one reliable packet for every property changed the underlying system must:

  • send the first
  • wait for ack
  • (upon receiving the first ack) send the second
  • wait for ack
  • (upon receiving the second ack) send the third
  • etc.

As you can see, this piles up a lot of latency, and gets out of hand quickly (when you change 10/15 properties in a frame).

(note: libraries do sometime send more then one without waiting, queuing/reordering them on the receiving end, but that number is still going to be very limited to ensure low latency.)

How would you suggest resolving the issue in my use case?

on_game_started.rpc(spawn_position_array)

@nathanfranke
Copy link
Author

nathanfranke commented Jul 7, 2022

So if I change 5 properties in one frame, and we send one reliable packet for every property changed the underlying system must:

* send the first

* wait for ack

* (upon receiving the first ack) send the second

* wait for ack

* (upon receiving the second ack) send the third

* etc.

This is not the case and it should never be.

Some basic test code (Using Godot 3.4.4):

extends Node

func _ready() -> void:
	if "--server" in OS.get_cmdline_args():
		print("Starting server")
		var server := NetworkedMultiplayerENet.new()
		var err := server.create_server(12345)
		assert(err == OK)
		get_tree().network_peer = server
	else:
		print("Starting client")
		var client := NetworkedMultiplayerENet.new()
		var err := client.create_client("my.ser.ver.ip", 12345)
		assert(err == OK)
		get_tree().network_peer = client
		
		yield(get_tree().create_timer(3.0), "timeout")
		
		print("starting at ", OS.get_ticks_msec())
		for i in 1000:
			rpc("hello", i)

remote func hello(i: int) -> void:
	print(i)
	if i == 999:
		rpc("done")

remote func done() -> void:
	print("finished at ", OS.get_ticks_msec())

All 1000 packets are sent to my server in ~130ms (server is 1,100 miles away)

@Faless
Copy link

Faless commented Jul 7, 2022

All 1000 packets are sent to my server in ~130ms (server is 1,100 miles away)

indeed, 130 ms is a lot for <2000 km (assuming you are on a good connection) some queuing is clearly happening.

@Faless
Copy link

Faless commented Jul 7, 2022

While I clearly over-simplified the send-ack-send-ack (that's how TCP work, and I did mention that libraries do optimize it), it obviously need to ack packets, and re-trasmit if acks does not arrive in time, and wait for the correct order to deliver them (even it it can buffer out-of-order packets for a while).

From ENet docs http://enet.bespin.org/Features.html:

ENet provides sequencing for all packets by assigning to each sent packet a sequence number that is incremented as packets are sent. ENet guarantees that no packet with a higher sequence number will be delivered before a packet with a lower sequence number, thus ensuring packets are delivered exactly in the order they are sent.

ENet provides an in-flight data window for reliable packets to ensure connections are not overwhelmed by volumes of packets. It also provides a static bandwidth allocation mechanism to ensure the total volume of packets sent and received to a host don't exceed the host's capabilities. Further, ENet also provides a dynamic throttle that responds to deviations from normal network connections to rectify various types of network congestion by further limiting the volume of packets sent.

@nathanfranke
Copy link
Author

nathanfranke commented Jul 7, 2022

It is not nearly as dramatic as you suggest.

Task 1 RPC 10 RPCs 100 RPCs 1000 RPCs
Average Duration after 1000 trials 35ms 34ms 36ms 63ms

I am only suggesting to add an option for reliable synchronization packets, I am not saying we should force all users to use it. There are many cases in my own game where I would prefer to use unreliable packets (e.g. transforms)

@Faless
Copy link

Faless commented Jul 7, 2022

It is not nearly as dramatic as you suggest.

In this very synthetic test where there are no other packets flowing.

I am only suggesting to add an option for reliable synchronization packets

And I'm not saying we should not add an option for reliable synchronization, I'm saying that such a synchronization should still happen at the end of the frame, and pack multiple variables into a single packet.

If you want to send a single packet with a single variable (which is very rare) just use an RPC.

@nathanfranke
Copy link
Author

nathanfranke commented Jul 7, 2022

End of the frame is not a big deal I guess, I will use await get_tree().process_frame

@Cretezy
Copy link

Cretezy commented Dec 17, 2022

The ability to configure reliability for a synchronizer is pretty core. Having at least an option for reliable/unreliable would be good.

Additionally, it would be nice to be able to configure channels for individual MultiplayerSynchronizers (or optionally, individual properties).

@Norrox
Copy link

Norrox commented Feb 26, 2023

I think having an option to do a reliable sync is good, and an "on change" sync as well.

Would bigger packet buffer for the reliable ones solve the problem with sending many small packages and waiting for response?

@akien-mga
Copy link
Member

Implemented by godotengine/godot#75467.

@agrimminck
Copy link

agrimminck commented Jun 5, 2023

I have tested this "Watched" property, but the my function which is synchronized through it, prints its value on its set function on every frame

@export var players_searching := 0 :
  set(value):
    print(players_searching)
    players_searching = value
    $CanvasLayer/PlayersSearchingNumber.text = str(value)

So I guess this is not working yet

@Calinou
Copy link
Member

Calinou commented Jun 5, 2023

@agrimminck Please open a separate issue on the main Godot repository with a minimal reproduction project included.

PS: Code blocks should use triple backticks like this (with an optional language name for syntax highlighting):

```gdscript
code here
```

I edited your post accordingly, but remember to do this in the future 🙂

@agrimminck
Copy link

Will do!

@nathanfranke
Copy link
Author

@agrimminck's issue is tracked here: godotengine/godot#77896

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

No branches or pull requests

8 participants