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 doesn't expose replication_config in Inspector #80717

Open
nezvers opened this issue Aug 17, 2023 · 14 comments
Open

MultiplayerSynchronizer doesn't expose replication_config in Inspector #80717

nezvers opened this issue Aug 17, 2023 · 14 comments

Comments

@nezvers
Copy link

nezvers commented Aug 17, 2023

Godot version

v4.1.1.stable.official [bd6af8e]

System information

Godot v4.1.1.stable - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2070 SUPER (NVIDIA; 31.0.15.3203) - Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz (8 Threads)

Issue description

I was checking out @ic3bug repository https://github.com/ic3bug/SMP and encountered problems opening player scene.
Project version is 4.1. in project file while I have 4.1.1. from official godotengine.org.
Looking through scene file with text editor I saw that MultiplayerSynchronizer has property replication_config that is not in my editor inspector but it is mentioned in documentation.
image

Steps to reproduce

  • use Godot v4.1.1.stable
  • add MultiplayerSynchronizer node to the scene
    -> replication_config is not exposed to editor

Minimal reproduction project

SMP.zip
Project by @ic3bug. player.tscn has packed SceneReplicationConfig resource. I tried to recreate it with player_2.tscn but editor doesn't expose replication_config.

@AThousandShips
Copy link
Member

It's in the bottom area, not as a property

@nezvers
Copy link
Author

nezvers commented Aug 17, 2023

@AThousandShips What bottom area?
It literally says "Properties" in docs so it should be available in the inspector. If not then where exactly it is exposed to the scene?

@AThousandShips
Copy link
Member

AThousandShips commented Aug 17, 2023

It's visible below the 2d)3d view, with the output area etc.

Please don't tag me directly, it's only us here

image

@nezvers
Copy link
Author

nezvers commented Aug 17, 2023

Ah, thanks for pointing out!
Still, out of blue a property resource configuration at the bottom without any mention or indication doesn't feel right.
TileMap's TileSet and AnimationPlayer's Libraries are exposed in the inspector and on top of that the bottom tray is automatically opened.
Now that I mentioned the automatic bottom section activation - all this time it never got shown to me. Now that I manually opened it for the first time it activates every time even after restarting whole Godot editor.

EDIT:
Not only that, the Ic3bug project's player.tscn previously showed this alert
image
But after activating it manually I can open that scene without a problem.
Could there be some kind state switch for the editor?

@AThousandShips
Copy link
Member

It's not supposed to be treated as a resource I believe, that's why it's separated like this

It's by design, not a bug

@nezvers
Copy link
Author

nezvers commented Aug 17, 2023

What's the reasoning/justification for it to be in bottom section? I have no experience setting up multiplayer, so I don't know tiny details. I'm expecting to use bottom section for tools that require more intuitive workflow for setting up, like animations, TileSets, TileMap, Debugging. Assigning property and ticking 3x bools in my eyes is not enough to justify it's location out of expected location in the Inspector.
It is a Reasource and should be treated as such. To make a point I quicly threw a GDscript mockup to show that it should be located in the Inspector.
image

@AThousandShips
Copy link
Member

AThousandShips commented Aug 17, 2023

Because they aren't supposed to be shared, or reset, they are fixed to that one synchronizer, they aren't the same as other resources

It has a dedicated inspector which is much much cleaned and specialised, it doesn't belong in the normal inspector, there wouldn't be anything to show there, so it'd be pointless and confusing imho

@nezvers
Copy link
Author

nezvers commented Aug 17, 2023

I'd argue it is confusing for it not to be in the inspector, hence I opened the issue. Well, automatic bottom section reveal, could have changed that.

Non-copying/ non-sharing doesn't feel like a correct reasoning too. If I duplicate the scene it essentially created a copy of setup. If a NodePath matches I don't see a reason why it couldn't be copied to an other setup. If I'm copying nodes from a one scene that has references to different nodes I'm fully expecting for references to break (AnimationPlayer as example).
Also, I still can copy replication_config from one MultiplayerSynchronizer to other with a @tool script.

With a native code an interface in the Inspector also could be cleaner than creating it with a GDscript.

@SaracenOne
Copy link
Member

SaracenOne commented Oct 10, 2023

Given that it's actually very easy do duplicate a MultiplayerSynchronizer node and that subsequently its replication config gets duplicated too, with no user-facing way to remove the duplicate, I'd say this not being in the inspector is a problem. It's not like the replicator config is any different from, say, an AnimationNode resource in the AnimationTree.

@Faless
Copy link
Collaborator

Faless commented Oct 15, 2023

It's not like the replicator config is any different from, say, an AnimationNode resource in the AnimationTree.

It is indeed inspired by the AnimationPlayer, so my feeling is that it should go in the in the bottom panel (like the Animation button in the AnimationPlayer), and not be directly exposed via the inspector (since you shouldn't be editing it from there).

@AThousandShips
Copy link
Member

I think the solution is to prevent the duplication and ensure the replication is unique, and then prevent direct access to it, it shouldn't be shared or edited directly

@romlok
Copy link
Contributor

romlok commented Dec 13, 2023

One thing not mentioned yet is that, without exposing this in the inspector, it's not currently possible to inspect the real-time state of a replication config of a running project. The bottom panel does not respond to selections of the "remote" scene dock.

I'd been experimenting with programmatically syncing sub-resource properties, and was irked by the lack of ability to easily verify that the replication config was getting set up as I expected!

@Summersay415
Copy link
Contributor

I think it should be exposed, because replicationconfig may be useful to share, and in combination with #86779 you can even extend config and add properties

@jjolmo
Copy link

jjolmo commented Sep 13, 2024

Because they aren't supposed to be shared, or reset, they are fixed to that one synchronizer, they aren't the same as other resources

It has a dedicated inspector which is much much cleaned and specialised, it doesn't belong in the normal inspector, there wouldn't be anything to show there, so it'd be pointless and confusing imho

I understand the reasoning, but misses the fact that could create wrong assumptions when trying to create inherited scenes, as an example, making the behaviour quite confusing. Even if it brings with its own editor it should be exposed as a resource that you can share, or make unique, the same way the tilemaps are also dictionaries with their own panel.

This way, I cannot just create inherited scenes of my enemies, but I need to create a new MPSync, then assign a script to set my custom visibility config, then assign the variables that are shared in parent classes AND the ones that will be specific for that enemy, as an example.

I would envision the replication panel to behave more similar to this mockup:
image

I would agree that parent properties might not be changed, but you should be able to see them. But it should let you create another replication dictionary which inherits from the parent one to add new elements.
You can see that the panel would show properties from inherited classes and properties from its own class.
And of course, they would be libraries that you could use,extend, makeunique etc...

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

7 participants