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

Remove unnecessary override #19723

Merged
merged 3 commits into from
Oct 9, 2024
Merged

Remove unnecessary override #19723

merged 3 commits into from
Oct 9, 2024

Conversation

HellAholic
Copy link
Contributor

@HellAholic HellAholic commented Oct 7, 2024

Bug fix, value should not be overridden since it cancels out the formula calculating the distance based on the sparse
Update, the override is setting the value to the value that the parent definition would calculate.
Resolves #19716

image
image

HellAholic and others added 2 commits October 7, 2024 22:27
value overrides formula and default value sets the initial value.
if value is overridden, the formula cannot update it based on the infill sparse density changes
@wawanbreton
Copy link
Contributor

What is the point of setting this hard-coded 8 value then ? If I'm correct, it will never be used in normal conditions within Cura. I'm fine with it, but at some point, somebody probably put this value for a good reason ?

@HellAholic
Copy link
Contributor Author

Yeah the entire formula needs to be adjusted so the output value is correct for the printer.
The override of the value in default profile means that you can't rely on the value for different infill percentages, and you can only print at 10%

@HellAholic HellAholic marked this pull request as draft October 8, 2024 08:04
@wawanbreton
Copy link
Contributor

So could you adjust the formula for the printer, so that it gives an 8 at 10%, and something appropriate for other values ?

the fdmprinter definition formula calculates as 8 for 10% infill
@HellAholic
Copy link
Contributor Author

Just removed the override and at 10% the fdmprinter formula calculates as 8, so I don't even know why you would override that.

@HellAholic HellAholic marked this pull request as ready for review October 8, 2024 16:06
@HellAholic HellAholic changed the title change value to default_value Remove unnecessary override Oct 8, 2024
@wawanbreton
Copy link
Contributor

Just removed the override and at 10% the fdmprinter formula calculates as 8, so I don't even know why you would override that.

Ok, well cool then 😄

@wawanbreton
Copy link
Contributor

Do you want to redirect to 5.9 ?

@HellAholic
Copy link
Contributor Author

HellAholic commented Oct 8, 2024

I'll do a test on a build to confirm and probably cherrypick :P

Redirect is also an option. Depending on what is the preferred way. But my assumption was it would be more than 1 line removal change.

@HellAholic HellAholic merged commit abfdeca into main Oct 9, 2024
7 checks passed
@HellAholic HellAholic deleted the bug-fix-ankermake_m5c branch October 9, 2024 12:04
@HellAholic
Copy link
Contributor Author

Also cherry picked commit to 5.9

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.

[5.8.1] Infill Line Distance dosen't change
2 participants