Add clipboard support to //deform and //brush deform #2276
Conversation
TomyLobo
force-pushed
the
deform-clipboard
branch
2 times, most recently
from
d7499d1
to
7077c74
Compare
Codecov Report
@@ Coverage Diff @@
## master #2276 +/- ##
===========================================
+ Coverage 8.68% 8.71% +0.02%
- Complexity 1041 1044 +3
===========================================
Files 881 883 +2
Lines 46291 46370 +79
Branches 5160 5158 -2
===========================================
+ Hits 4022 4042 +20
- Misses 42087 42144 +57
- Partials 182 184 +2
... and 4 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| this(destination, region, expression, mode, false); | ||
| } | ||
|
|
||
| public Deform(Extent destination, Region region, String expression, Mode mode, boolean useClipboard) { |
There was a problem hiding this comment.
I sort of feel a sourceExtent & sourceRegion would be more useful here as something to pass in rather than useClipboard. Would also allow deform brushes to "bake in" the clipboard so you can have multiple different ones
There was a problem hiding this comment.
Sorry, I only just saw your comment.
We'd have to offer both, because some people would want the current clipboard, some wouldn't.
The -o flag also needs to offer both static and dynamic modes.
That'd probably a separate PR.
There was a problem hiding this comment.
baking in is the default for most things and should probably be consistent for ux. (see clipboard brush, #sel vs #dsel masks, etc)
There was a problem hiding this comment.
ok I wasn't aware //brush clipboard already baked in the clipboard.
I use brushes so rarely :)
guess I'll have to bake that in before merging it then
There was a problem hiding this comment.
-o, on the other hand, is not so simple.
- Do we read placement type at
//brushtime or at click time? - Do we read
pos1at//brushtime or at click time? - Do we read the clipboard at
//brushtime or at click time? - Do we read
playerposition at//brushtime or at click time?
- Probably at
//brushtime - That'd be the difference between
#seland#dsel, so we'd probably want to offer both - We said we want to bake it in at
//brushtime, but if we offer both in 2., offering both here as well actually saves code - EDIT: If the player wants this fixed to certain position, they can use
pos1. I also added ahereplacement mode in my other PR to have a static variant ofplayer. Given that, we'd probably want to always use the dynamic position here, right?
TomyLobo
force-pushed
the
deform-clipboard
branch
from
7077c74
to
e7f8480
Compare
|
rebased |
TomyLobo
force-pushed
the
deform-clipboard
branch
2 times, most recently
from
39257c9
to
bb880d3
Compare
|
rebased this on top of most of the //placement PR in order to be able to use the new Placement system for the Deform brush. |
TomyLobo
force-pushed
the
deform-clipboard
branch
from
bb880d3
to
6116518
Compare
TomyLobo
force-pushed
the
deform-clipboard
branch
from
6116518
to
83eb6aa
Compare
Test coverage so far: - setPlaceAtPos1 - isPlaceAtPos1 - togglePlacementPosition - getPlacementPosition for both pos1 and player modes
TomyLobo
force-pushed
the
deform-clipboard
branch
from
83eb6aa
to
7f88a3e
Compare
This adds a -l flag to
//deformand//brush deformwhich, when used, will fetch blocks from the clipboard instead of the world.Additionally, coordinate transformations that would normally be based on the selection position+size are based on the clipboard's position+size when using that mode.
Example usage:
The shape on the right is the result of copying the square shape on the left and deforming the selection with the command on screen.
This is the following brush using the same clipboard contents as before:
//brush deform -l sphere 6 x=-x;y=0;z=-zAddresses parts of #1554