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

Entity IDs to be u_int32 instead of 12-character strings. #242

Open
TheYellowArchitect opened this issue Aug 22, 2024 · 3 comments
Open

Comments

@TheYellowArchitect
Copy link
Contributor

TheYellowArchitect commented Aug 22, 2024

@rpc("any_peer", "reliable", "call_remote")
func _request_projectile(id: String, tick: int, request_data: Dictionary):
	var sender = multiplayer.get_remote_sender_id()
...
	_save_projectile(projectile, id, local_data)
	_accept_projectile.rpc(id, tick, local_data)
	_after_fire(projectile)
func _save_projectile(projectile: Node, id: String, data: Dictionary = {}):
	_projectiles[id] = projectile
	projectile.name += " " + id
	projectile.set_multiplayer_authority(get_multiplayer_authority())
	
	if data.is_empty():
		data = _get_data(projectile)
	
	_projectile_data[id] = data

Sending a string of 12 characters on an RPC as ID, instead of an integer, costs a lot of bandwidth (and some slight CPU)

func _generate_id(length: int = 12, charset: String = "abcdefghijklmnopqrstuvwxyz0123456789") -> String:
	var result = ""
	for i in range(length):
		var idx = randi_range(0, charset.length() - 1)
		result += charset[idx]
	return result

I suggest solving this with the following way:


Preset Network IDs

An unsigned integer (4 bytes) has range[0, 4294967296]
But let's simplify it, let's say there is [0, 5000]
Say there are 5 players max in the game. If you split the above range in 5 parts, each player occupies the ranges
Player1: [0-1000]
Player2: [1001-2000]
Player3: [2001-3000]
Player4: [3001-4000]
Player5: [4001-5000]

If Player2 spawns a projectile, that projectile's id is 1001. So, locally it is 1001. But the server knows at that exact same moment without even having received player2's new projectile, that whatever next projectile is of player2 at any future tick, it will have id 1001. Ofc, the next projectile of player2 will be 1002, and the next after it 1003.

So just by the ID, it is clear who spawned it, and ofc its very cheap on memory and bandwidth.

Now, since we are using an integer, we are not capped at range[0, 5000] (1000 projectiles/IDs per player), but more like 100000 so that's more than enough.

@TheYellowArchitect TheYellowArchitect changed the title Projectile ID being string costs performance Entity IDs to be u_int32 instead of 12-character strings. Aug 22, 2024
@elementbound
Copy link
Contributor

Sending a string of 12 characters on an RPC as ID, instead of an integer, costs a lot of bandwidth (and some slight CPU)

Define a lot :) We are talking 12 bytes instead of 4, and depending on how often players can fire ( depending on game ), this can be anywhere between pretty much 0 bytes, to 120 bytes per second. For any weapon faster than that, it would make more sense to send messages like "I've started firing" and "I've stopped firing".


I'm not sure you are familiar with the original intent behind the code? Not judging here, IIRC I haven't documented this part.

So the idea is, each peer can generate their own, unique ID for any given fire request. I'm doing pretty much what nanoid does. With 12 characters, the chance of a collision is astronomically low, meaning that no two peers end up generating the same ID.

Clashing ID's could end up rewriting and re-reconciling projectiles already in motion.

Let me know if that makes sense! And based on the above, I'm open to suggestions to how else this could be handled. IMO a pro of the current approach is that it's conceptually pretty simple - peers don't need to sync anything or even know about eachother in any way.

@TheYellowArchitect
Copy link
Contributor Author

TheYellowArchitect commented Sep 2, 2024

func _generate_id(length: int = 12, charset: String = "abcdefghijklmnopqrstuvwxyz0123456789") -> String:
	var result = ""
	for i in range(length):
		var idx = randi_range(0, charset.length() - 1)
		result += charset[idx]
	return result

To generate the 12 characters, you create 12 strings. In GDscript, there is no Stringbuilder so every new string (e.g. adding a new character) creates a new copy of a String (I cannot link you this to confirm you, but I remember finding this out a week ago, as I was searching why my logger was lagging in GDScript)
This ID creation is inside a forloop as well. All in all, it is extremely inefficient in performance. Also having an ID not be an integer but a string is not ideal (e.g. int comparison is faster)
Not to mention that astronomically low chance is still a chance. The ID has length 12 to make that chance astronomical.

The aforementioned method with player ids has no collision chance, is simpler to read, and has O(1) performance.

Clashing ID's could end up rewriting and re-reconciling projectiles already in motion.

For current way or the recommended player id way? For player ids, rewriting and reconciling shouldn't be a problem, as it always goes +1

We are talking 12 bytes instead of 4,

That is 300% times bigger. For something like a replay manager - if implemented crudely by read&writing all states - every byte matters to not have huge replay files.

nanoid

Nice website, thanks for sharing. The string algorithm makes sense now, as it means the collisions are impossible. You weren't kidding about the astronomically low - could as well say 0%.

@TheYellowArchitect
Copy link
Contributor Author

To quote #270

Ideally, the RNG should be seeded with the same value for all peers as it would guarantee they generate the same IDs in the same order.

This would give some guarantee to ID uniqueness (in fact if we generate a used ID something has gone very wrong) while also giving some more server validation possibilities (eg: server would also generate an ID when a weapon is fired and it should match).

Using a shared seed is a good alternative, though instead of using player ID, it uses session ID of the same class, or a random seed value of the same class. Since network weapon and this ID generation function is at Network.extras I think SessionData 's player ID and session ID being used to generate ID, solve the problem.

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

No branches or pull requests

2 participants