Skip to content

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
Stephan-S:mainfrom
KeilerHirsch:fix/marker-sync-bit-offset
Open

fix: marker error branch reads one bit too few, shifting the rest of the graph stream#553
KeilerHirsch wants to merge 1 commit into
Stephan-S:mainfrom
KeilerHirsch:fix/marker-sync-bit-offset

Conversation

@KeilerHirsch

Copy link
Copy Markdown

The problem

AutoDriveSync.streamWriteGraph writes four values per map marker (scripts/Sync.lua:126-131):

for _, marker in pairs(mapMarkers) do
    streamWriteUIntN(streamId, marker.id, self.MWPC_SEND_NUM_BITS)
    AutoDrive.streamWriteStringOrEmpty(streamId, marker.name)
    AutoDrive.streamWriteStringOrEmpty(streamId, marker.group)
    streamWriteBool(streamId, (marker.isADDebug == true))
end

AutoDriveSync.streamReadGraph reads four in the happy path (:201, :206, :207, :208) — but only three in the error branch (scripts/Sync.lua:210-214):

else
    Logging.error("[AutoDriveSync] Error receiving marker %s (%s)", AutoDrive.streamReadStringOrEmpty(streamId), markerId)
    -- we have to read everything to keep the right reading order
    _ = AutoDrive.streamReadStringOrEmpty(streamId)
    -- isADDebug (1 bit) is never consumed
end

The comment already states the intent — the streamReadBool for isADDebug is 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-267 detects orphaned markers on config load, logs them, and keeps them in the graph — so the server will happily send them:

for markerIndex, marker in pairs(ADGraphManager:getMapMarkers()) do
    if ADGraphManager:getWayPointById(marker.id) == nil then
        Logging.info("[AutoDrive] mapMarker[" .. markerIndex .. "] : " .. marker.name .. " points to a non existing waypoint! Please repair your config file!")
    end
end

streamWriteGraph/streamReadGraph are also used by scripts/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 marker line.

The fix

One line: consume the bool in the error branch, matching the happy path and the writer.

Honesty about verification

  • Verified statically: writer 4 values vs. error branch 3, on current main (48702cd). luac -p passes.
  • Not reproduced in-game. I run a dedicated server with AutoDrive 3.0.1.2 and checked its logs: the 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. :122 sizes the send with #mapMarkers while :126 iterates with pairs() — 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.

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).
@Axel32019

Copy link
Copy Markdown
Collaborator

This fix prevent a data mix in case of sync of a not OK network, which seems to be a rare case.
Nevertheless we can take it as it is.
@KeilerHirsch The topic "Side note..." you may follow if you want in an separate PR to keep it easy.
Appreciate your support!

@Iwan1803 Could be integrated in the release.

@Iwan1803

Copy link
Copy Markdown
Collaborator

@Axel32019 to avoid merge problems, please merge it to your local sources and then push it into Dev_RC.

Axel32019 added a commit that referenced this pull request Jul 19, 2026
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.

3 participants