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 ability to sync node references #4787

Open
nathanfranke opened this issue Jun 30, 2022 · 0 comments
Open

MultiplayerSynchronizer - Add ability to sync node references #4787

nathanfranke opened this issue Jun 30, 2022 · 0 comments

Comments

@nathanfranke
Copy link

Describe the project you are working on

RTS with a lot of export nodes to link properties (e.g. the owner of an entity is a player node).

Describe the problem or limitation you are having in your project

I am happily migrating my code to utilize the new export nodes: #1048

However, I still need to keep a setter/getter for the NodePath for when the MultiplayerSynchronizer syncs. Currently, trying to sync the node references directly has no behavior (bug?).

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

When Node is serialized to be sent over the network, it should be converted to a NodePath for transit.

#4429 will likely be implemented in two ways:

  1. Keep a cache of the previous value and compare that before a synchronization. In this case, the Node reference itself is kept in cache.
  2. Use some signal to determine when a property is changed, in which case Nodes in particular won't be very different.

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

@export var player: Node

And in the MultiplayerSynchronizer:

image

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

@export var player: Node

var player_path: NodePath:
	set(value):
		player = get_node_or_null(value)
	get:
		return player.get_path() if player != null else NodePath()

Issues with this workaround though:

  1. Requires another ~5 messy lines of code.
  2. For compatibility with MultiplayerSynchronizer - Add an option to sync properties only when they are changed #4429, the workaround will either:
    • Not work at all because player_path's value never actually changes.
    • Work, but be non-optimal because the getter for player_path has a logical branch and calls get_path every time.

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

MultiplayerSynchronizer is already core

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

2 participants