fix: marker error branch reads one bit too few, shifting the rest of the graph stream#553
Open
KeilerHirsch wants to merge 1 commit into
Open
fix: marker error branch reads one bit too few, shifting the rest of the graph stream#553KeilerHirsch wants to merge 1 commit into
KeilerHirsch wants to merge 1 commit into
Conversation
The writer sends four values per map marker (id, name, group, isADDebug), but the reader's error branch consumed only three, leaving the read head one bit behind for every remaining marker and the whole group block. Sync.lua:126-131 (write) vs Sync.lua:200-215 (read).
Collaborator
|
This fix prevent a data mix in case of sync of a not OK network, which seems to be a rare case. @Iwan1803 Could be integrated in the release. |
Collaborator
|
@Axel32019 to avoid merge problems, please merge it to your local sources and then push it into Dev_RC. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
AutoDriveSync.streamWriteGraphwrites four values per map marker (scripts/Sync.lua:126-131):AutoDriveSync.streamReadGraphreads four in the happy path (:201,:206,:207,:208) — but only three in the error branch (scripts/Sync.lua:210-214):The comment already states the intent — the
streamReadBoolforisADDebugis just missing. Once that branch runs, the read head is one bit behind for every remaining marker and for the entire group block that follows at:218.When it triggers
The error branch runs when a marker points at a waypoint the reader doesn't have (
:202,wayPoints[markerId] == nil). That state is reachable, and the codebase knows it:scripts/XML.lua:263-267detects orphaned markers on config load, logs them, and keeps them in the graph — so the server will happily send them:streamWriteGraph/streamReadGraphare also used byscripts/Events/Graph/RoutesUploadEvent.lua:20,24, so a route upload hits the same path.The likely symptom is a joining client that decodes garbage markers/groups from that point on, or stalls without a useful error — the only hint in the log would be the generic
[AutoDriveSync] Error receiving markerline.The fix
One line: consume the bool in the error branch, matching the happy path and the writer.
Honesty about verification
main(48702cd).luac -ppasses.Please repair your config file!line does not appear there, and all 511 markers on my graph resolve — so my own setup never enters this branch. I found this by auditing the stream layer, not by hitting it.So please treat the "silent join hang" as the plausible consequence of a proven bit offset rather than as something I measured. The bit offset itself is not in question.
Side note (not fixed here, to keep the diff minimal)
The same error branch skips
mapMarkers[i] = marker, leaving a hole in the table.:122sizes the send with#mapMarkerswhile:126iterates withpairs()— those two disagree on a sparse table. Filling densely with a separate counter would be the tidier fix, but it changes more behaviour than a bit-alignment fix should, so I left it out. Happy to follow up separately if you want it.