Skip to content

Conversation

@LightHardt
Copy link
Contributor

These changes aim to handle #5366. Updated documentation to alert users of new clear option that will allow them to unset a previously set order. This probably could have been a little cleaner if I added the clear at the end however I think putting it as the first parameter makes it read quite well. Compiled and tested on windows. Please let me know if there is anything I need ti change/do!

Copy link
Member

@ab9rf ab9rf left a comment

Choose a reason for hiding this comment

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

I have a lot of grouses with this code, but most of them are from code already there, and this PR doesn't make it worse and actually addresses the open issue

I also note that brace placement is inconsistent with our typical house style, but the original code was inconsistent so I can't blame the proposer for doing the best they can under suboptimal circumstances

Gonna have to put this entire tool on the "needs refactor" list

In any case, changes approved

@LightHardt
Copy link
Contributor Author

Yeah looking at it definitely not the cleanest code thank you for the insight. I looked over it again and missed adding the validateMaterialCategory function to the parameter check for the clear case after work or maybe around lunch will be able to make that hopefully final change.

@ab9rf
Copy link
Member

ab9rf commented Jul 24, 2025

Needs changelog, otherwise lgtm

@ab9rf
Copy link
Member

ab9rf commented Jul 24, 2025

Last CI failure was probably because I was in the middle of a release when CI ran.

@ab9rf ab9rf merged commit 02a2cc9 into DFHack:develop Jul 24, 2025
14 checks passed
@LightHardt LightHardt deleted the autoclothing-clear branch July 24, 2025 21:44
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