Skip to content

feat: implement "mutable" transform holder for random rotations #3173

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

Merged
merged 4 commits into from
Jun 8, 2025

Conversation

dordsor21
Copy link
Member

@dordsor21 dordsor21 requested a review from a team as a code owner April 13, 2025 16:50
@github-actions github-actions bot added the Feature This PR adds a new feature label Apr 13, 2025
Copy link
Member

@PierreSchwang PierreSchwang left a comment

Choose a reason for hiding this comment

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

Looks reasonable

*
* @since TODO
*/
default void mutate() {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if I like this design, it's somewhat invasive. Would it make sense to have a static method in MutatingOperationTransform instead that handles that part instead?

@SirYwell
Copy link
Member

Hmm how does that deal with multithreading?

@dordsor21
Copy link
Member Author

Hmm how does that deal with multithreading?

It doesn't, but currently there should be no reason for there to be any kind of contention, and regardless multi-threading should probably be handled implementation-specifically

@SirYwell
Copy link
Member

What about RandomFullClipboardPattern? Doesn't that cause multiple threads to update the same transform instance?

dordsor21 added 3 commits May 30, 2025 17:24
 - also implement random rotation to schematic loadall
 - "mutable" transform allows random rotation of schematics outside of clipboard brush
 - fixes #3129
@dordsor21 dordsor21 force-pushed the feat/mutable-transform-holder branch from 6ae1449 to 3d4ea3c Compare June 1, 2025 15:23
@dordsor21 dordsor21 requested a review from SirYwell June 1, 2025 15:23
@dordsor21 dordsor21 requested a review from SirYwell June 6, 2025 13:23
@NotMyFault NotMyFault merged commit 8bf59a5 into main Jun 8, 2025
11 checks passed
@NotMyFault NotMyFault deleted the feat/mutable-transform-holder branch June 8, 2025 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature This PR adds a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Function not add, while it is announced in the wiki.
4 participants