godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.12k stars 69 forks source link

Add a `@sync` annotation to synchronize properties without exposing them in the inspector #8662

Open FreshDaikon opened 9 months ago

FreshDaikon commented 9 months ago

Describe the project you are working on

Working on multiplayer game, making use of the new networking features such as MultiplayerSpawner and MultiplayerSynchronizer.

Describe the problem or limitation you are having in your project

Currently, the easiest way to hook up properties to be synced with a MultiplayerSynchronizer is to mark the property with @Export / [Export] making it visible to the MultiplayerSynchronizer by path.

However, can create a lot of clutter on Node properties, and can at worst be confusing to designers when working with the node.

Additionally, the current approach creates a update-dependency, where in when properties are changed/renamed/removed from a script, the MultiplayerSynchronizer looking for these properties breaks, but this isn't checked/creating errors in the editor, and instead only produces a error at game runtime.

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

Add a way to mark export properties differently, when their only purpose is to be synced across the network and not meant to be edited in the editor.

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

The simplest version of this proposal simply asks that properties can be marked with @Sync/[Sync] making them available to a MultiplayerSynchronizer without showing up in the properties list of the node - or showing up in the property list but greyed out/uneditable.

GDScript

@Sync var MySyncFloat ;

C#

[Sync] 
public float  MySyncFloat { get; set; };

The below suggestions are not really necessary's for the above to suggestion to work and be appreciated, but more just musings on how this could possibly be expanded in the future :

Such an attribute could hold various initiation values for the property replication that are then the initial values for the property when added to the multiplayerSynchronizer :

GDScript

@sync(true, "on_change") var mySyncVar ;

C#

[Sync(Spawn=true, Replicate=ReplicationMode.OnChange )]
public float MySyncFloat { get; set; };

It's even possible to conceive of a system where properties marked properly are just automatically added to a global sync object on creation with the proper visibility and ownership set based on the nodes multiplayer configuration - bypassing the need for an additional Node that is currently only there to accomplish this task.

Of course, being able to add and sync pretty much any property is still very powerful and useful in most cases, but having a way to more directly specify what the intended use of a property is could be helpful when there are lots of properties to keep track of on an object.

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

Currently, there are no workarounds that directly can make it clear that a property will be synced as far as i can tell.

  1. You can setup the MultiplayerSynchronizer wholly in code by setting up the config in code, but it still requires the properties to be accessible via @export / [Export] meaning they will still show up in the property list.
  2. Using validate_properties to hide properties also seems to break or make properties unavailable to the MultiplayerSynchronizer.
  3. Using export groups can hide the properties somewhat or make the implementation clearer to the users of the node.

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

The new Multiplayer systems are core, and this suggestion seeks to simply improve on the current implementations usability.

AThousandShips commented 9 months ago

The relevant syntax for GDScript should also be added here if it's not specific to C#

AThousandShips commented 9 months ago

How would this be achieved? Would it generate a new synchronizer with the relevant properties? AFAIK you can't do synchronisation without a synchronizer node to handle it

Also a synchronizer has a limit on the total synchronized properties, so how would this be handled if it's automatic by just associating it with a specific node?

As I recall and in my looking at the code this would be incompatible with the current setup which involves explicitly adding synchronized properties, and that feels like a more well structured way to do it, it also puts the responsibility to do this on the scene that actually contains it, instead of on the script

What happens if you add a node with this annotation to a scene? How is it applied and updated?

Mickeon commented 8 months ago

Currently, the easiest way to hook up properties to be synced with a MultiplayerSynchronizer is to mark the property with @Export / [Export] making it visible to the MultiplayerSynchronizer by path.

This is a very silly limitation of the way properties are displayed in the selection. In reality, you can write any valid NodePath to the property box. If it exists it'll synchronise just fine at runtime.

image

m21-cerutti commented 3 months ago

Maybe this could be a helper ? like a button "Add user script properties from node" where you can have a lot less properties to select and is less confusing for designers ? I think the problem is more an UX experience on the property selection partially. We could not expand all properties by default and see types first, or add checkbox "only user script variables", or even use groups annotation inside the selection editor for more lisibility. That would be for the "confusing" editor part.

For the validation part, having a validation in editor if the property not exist anymore should do also the trick, and is more explicit than implicitly have @sync annotation (you would have the same problem and need to dig the code to see if the property you wanted to sync exist or not if there is a bug).

And for the the inelegant way to need to export variables to have them in the selection editor, we have the solution of _validate_property with PROPERTY_USAGE_NO_EDITOR to not edit them in the inspector. For that, the annotation could be interesting to reduce the boilerplate code and set the good PropertyUsageFlags, but should not drive the behaviour and just nicely expose it instead. Like recommendations or warnings ? Or even better, some button "add XXXproperty" to directly expose them optionnally in the list.