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

Synchronizing properties of nodes which aren't under the root node #298

Closed
TheYellowArchitect opened this issue Oct 9, 2024 · 14 comments
Closed

Comments

@TheYellowArchitect
Copy link
Contributor

In RollbackSynchronizer.process_settings():

# Gather all rollback-aware nodes to simulate during rollbacks
	_nodes = root.find_children("*")
	_nodes.push_front(root)
	_nodes = _nodes.filter(func(it): return NetworkRollback.is_rollback_aware(it))
	_nodes.erase(self)

This seems like nodes outside root cannot be synchronized.
Is it impossible to synchronize properties outside a root node? Is there some temporary workaround with NodePath properties?

For my use-case, I want to synchronize a node which changes parents (brawler to world, and world to brawler)

@albertok
Copy link
Contributor

I imagine it is impossible to synchronize properties outside the root node as Godot's multiplayer is based on the scene tree reference.

@TheYellowArchitect
Copy link
Contributor Author

(for no misunderstanding, the root node I refer to in the OP is the var root: Node inside RollbackSynchronizer.gd, not the root of the scene tree)

@albertok
Copy link
Contributor

@TheYellowArchitect

Reparenting is probably best done by making a copy and destroying the original.

Or never having that node be in somewhere that can't be permanent would be even better.

SceneReplicationConfig isn't really built for moving NodePaths.

There's stuff like RemoteTransform3D which might make influencing nodes that arent children easier also.

@TheYellowArchitect
Copy link
Contributor Author

TheYellowArchitect commented Oct 16, 2024

I would like to not destroy and remake the original for 2 reasons.

  1. The node I want to synchronize has a unique ID
  2. Because creating that type of node is expensive for no reason

Or never having that node be in somewhere that can't be permanent would be even better.

That is an interesting idea actually. It doesn't fit my case unless I do some hacks, but to expand on it: I have an Item node. To implement this suggestion would be to put an entity ID for who holds it at any moment, and it stays under a permanent WorldItems node, regardless who picks it. Anyway, it causes a lot of code complexity, and is incompatible with my item system which requires the holder to also be its parent. But even such food for thought is helpful for me to think of a problem in more angles, so thanks for the suggestion

SceneReplicationConfig isn't really built for moving NodePaths.

How is netfox related to SceneReplicationConfig?


Having a NodePath property "holder" at the Item would kinda work, but I would have to somehow hook up to its setter and reparent. Its the closest solution I can think of without any changes to netfox.

@albertok
Copy link
Contributor

Sorry I meant MultiplayerAPI object configurations.

Basically because netfox is using rpcs and those rpcs are bound to SceneTree/NodePaths moving stuff is really hard. You'd need everyone to move the node at exactly the same tick which is crazy hard to synchronize. Add rollback to the mix and the complexity is just not worth it.

I think you're on the right track with switching to a possession variable and items living in their own space. Much easier to rollback too :)

@TheYellowArchitect
Copy link
Contributor Author

TheYellowArchitect commented Oct 19, 2024

Requiring rollback with reparenting is not a rare use-case, see moving platforms: #223

@albertok
Copy link
Contributor

You're assuming reparenting is the solution ;)

Reparenting is impractical when using the high level api.

@TheYellowArchitect
Copy link
Contributor Author

TheYellowArchitect commented Oct 20, 2024

no high level api should completely change how I design my game. I need to reparent - a basic godot functionality - and don't want any hacks. If RollbackSynchronizer for example had reparenting functionality it would work out.

The solution would vaguely be some new variables to RollbackSynchronizer for parent. If null, same behaviour happens. If it has any Node, it reparents to that parent for that tick (if not already a child of it). So RollbackSynchronizer would hold an array/dictionary of parents it has had, and on rollback, it reparents to that node.

The above is only "impractical" because RollbackSynchronizer wasnt designed with reparenting in mind, so I have to do a hack for items to work. Instead of doing a hack, I will just make them work like projectiles do in forest-brawl, aka no rollback, which is dissapointing.

@albertok
Copy link
Contributor

albertok commented Oct 20, 2024

It's not a netfox thing, the high level api wasn't built with reparenting support.

godotengine/godot#86501

and fair enough. It's an anchor point for messaging, not a moving target.

If you want to reparent something it can't have any rpcs in it.

If they do decide to support it they'll likely just copy and kill the node behind the scenes.

Also do you really want to be handling rollback reparenting scenarios?

reparent, Go back 4 ticks, reparent to where it was, roll forward, reparent again.

@TheYellowArchitect
Copy link
Contributor Author

If you want to reparent something it can't have any rpcs in it.

This single phrase destroys the design of Godot's netcoding architecture. You couldn't have highlighted the problem with high level api better. I now understand fully what you meant when you said

Reparenting is impractical when using the high level api.


Also do you really want to be handling rollback reparenting scenarios?

Yes, but in the end of the day looks like I will not be doing that at all, since items won't work with rollback 😅

@elementbound
Copy link
Contributor

RollbackSynchronizer wasn't designed to synchronize nodes that move around the scene tree. It was designed to handle a single game object ( the root ), which is treated as an atomic unit comprised of itself and its children node. This is why it manages only the properties of the root node and its children, nothing else. This is also why it's not recommended to nest roots, e.g. node A has a RollbackSynchronizer, and then also has a child node B which also has a RollbackSynchronizer as child.

Once you reparent one of those nodes, you break this atomic unit.

So unfortunately there's no plans to support this with the current design.


That does not necessarily mean that you can't do items with rollback though! Without knowing the specifics of your use case, I'd separate your item functionality based on what needs to be reparented and what doesn't. From there, you can have an item holder variable as suggested by @albertok, and whenever that changes, you can reparent the parts / nodes that need to be reparented ( I assume this includes some visuals? ). IIRC you can synchronize NodePath values as-is, so you wouldn't need to mess around with serialization. But even as a fall-back, you can write getters and setters to do the serialization, and then you can just use the backing variable in your code.

This way, you can have your RPCs and rollback, with reparenting. The compromise is splitting the item logic into multiple nodes.

@elementbound
Copy link
Contributor

Something came to mind recently during another conversation.

We could add a RemoteState node, similar to RemoteTransform3D. You could configure certain properties and a target node, so that those properties are proxied to and from the target node. This way, you can sync the state of the RemoteState node, and you could reparent the target node as you'd like, as long as the RemoteState node doesn't move around the scene tree.

For now this is just an idea and I'll need to check if we can properly intercept set and get calls for arbitrary properties. Regardless, how does this sound @TheYellowArchitect?

@elementbound
Copy link
Contributor

Closing as stale. Feel free to open a feature request for the RemoteState node if needed!

@elementbound elementbound closed this as not planned Won't fix, can't repro, duplicate, stale Dec 19, 2024
@TheYellowArchitect
Copy link
Contributor Author

TheYellowArchitect commented Dec 23, 2024

I apologize for not replying to the above earlier. RemoteState node is a great idea, and a great feature for sure. The reason I hadn't replied is that I am still wondering if either solutions solve my specific use-case, and I haven't coded with items in a while. The Item system I use is https://github.com/peter-kish/gloot but for the item to be owned/usable by the player's avatar, it must be its child pretty much.

Anyway, I will experiment with what is exactly required and whether RemoteState node solves this issue while keeping the above item system. Scheduled to be when I get into coding items specifically (I may have to do a custom solution if this happens), as my project's top priority is the rollback for projectiles.

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

3 participants