Skip to content

fix: No duplicate call to process_settings()#612

Merged
elementbound merged 7 commits into
foxssake:mainfrom
TuysuzTavsan:fix-duplicate-register
Jul 2, 2026
Merged

fix: No duplicate call to process_settings()#612
elementbound merged 7 commits into
foxssake:mainfrom
TuysuzTavsan:fix-duplicate-register

Conversation

@TuysuzTavsan

Copy link
Copy Markdown
Contributor

Prevent duplicate call to process_settings on rollback-syncronizer.

DANGER!
I also removed multiplayer.connected_to_server.connect(process_settings) from _ready.
As i believe NetworkTime.is_initial_sync_done() wont be happening if we are not connected anyway?
So there should be no need for us to process settings when we are connected to server after initial_sync_done.
If that line was serving some other purpose it should be added back?

@elementbound elementbound force-pushed the fix-duplicate-register branch 2 times, most recently from 319653a to 829e712 Compare June 28, 2026 14:57
@elementbound

Copy link
Copy Markdown
Contributor

@TuysuzTavsan I've prepped this PR for merge, but found one concern. I had to add this fix earlier to StateSynchronizer, not sure if it applies here too:

func _ready():
	# Reprocess authority
	# Important if nodes are pre-placed in the scene - node starts as owned by
	# us ( offline peer is 1 ), but once we connect, we no longer own the node
	multiplayer.connected_to_server.connect(process_settings)

@TuysuzTavsan

Copy link
Copy Markdown
Contributor Author

I think that ownership issue happened because state-synchronizer doesnt really check network condition before processing_settings.
I recommend:
1- we also get rid of process_settings call on _enter_tree for state-synchronizer.
2- Instead we should process_settings on _ready just like rbs and predictive synchronizer.
3- All 3 nodes should check for network condition before processing_settings.

Now the question:

if not NetworkTime.is_initial_sync_done():
	# Wait for time sync to complete
	await NetworkTime.after_sync

Is this inital sync has some important task on rollback-synchronizer?
We can add this to state-synchronizer too?
Waiting for initial sync is kinda weird because its not neccessary, what we need is a common connected signal but i cant think of any :(

@elementbound elementbound force-pushed the fix-duplicate-register branch from 829e712 to a945649 Compare July 2, 2026 13:39
@elementbound elementbound changed the title fix duplicate call to process_settings on rbs fix: No duplicate call to process_settings() Jul 2, 2026
@elementbound elementbound merged commit 755bdaf into foxssake:main Jul 2, 2026
4 checks passed
elementbound added a commit that referenced this pull request Jul 4, 2026
Prevent duplicate call to process_settings on rollback-syncronizer.

---------

Co-authored-by: Tamás Gálffy <ezittgtx@gmail.com>
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

Successfully merging this pull request may close these issues.

2 participants