Skip to content

Conversation

@pedrolamas
Copy link
Member

Previously, the toolhead coordinates were checked and reset individually, but changes from 4e6f7aa caused this issue as the move object was copied completely, overriding previous values - this reverts back to the original approach, fixing the problem.

Fixes #1774

@pedrolamas pedrolamas added this to the 1.36.2 milestone Jan 2, 2026
@pedrolamas pedrolamas requested a review from Copilot January 2, 2026 16:12
@pedrolamas pedrolamas added the GH - Bug Something isn't working label Jan 2, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a regression in the GcodePreview toolhead position calculation. The previous Object.assign approach would override previously found non-null coordinates with null values from subsequent moves. This fix reverts to individually checking and assigning only null coordinates.

  • Replaces Object.assign with individual coordinate null checks
  • Adds a count variable to track how many coordinates have been found
  • Only assigns coordinates when output is NaN and move coordinate is non-null

}

for (let i = moveIndex; i >= 0 && (Number.isNaN(output.x) || Number.isNaN(output.y) || Number.isNaN(output.z)); i--) {
for (let i = moveIndex, count = 0; i >= 0 && count < 3; i--) {
Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loop termination condition using count < 3 is incorrect. The loop should continue while any coordinate is still NaN, not just until 3 coordinates are found. If all three coordinates (x, y, z) are found from a single move, the count will be 3 but the loop will still iterate through all remaining moves unnecessarily. The condition should be count < 3 && (Number.isNaN(output.x) || Number.isNaN(output.y) || Number.isNaN(output.z)) to match the original logic and stop early when all coordinates are found.

Suggested change
for (let i = moveIndex, count = 0; i >= 0 && count < 3; i--) {
for (
let i = moveIndex, count = 0;
i >= 0 && count < 3 && (Number.isNaN(output.x) || Number.isNaN(output.y) || Number.isNaN(output.z));
i--
) {

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot is wrong: count increases for every coordinate that is currently NaN and that is set to a non-null value, which should happen 3 times, hence the check.

Keep code as is.

@pedrolamas pedrolamas merged commit 8a6b082 into fluidd-core:develop Jan 2, 2026
10 checks passed
@pedrolamas pedrolamas deleted the pedrolamas/fix-1774 branch January 2, 2026 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

GH - Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GCode-Preview updating layers, but no head movement shown

1 participant