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

Hollow outline #56

Merged
merged 7 commits into from
Mar 22, 2021
Merged

Hollow outline #56

merged 7 commits into from
Mar 22, 2021

Conversation

Firigion
Copy link
Collaborator

Added hollow and outline commands and brushes, to keep chipping away at tasks in #28.

The commands affect all blocks within a selection, hollowing out or outlining (adding at the border of) blobs of blocks of the same type. There's an optional block argument in both commands to specify what types of blocks to affect. The brushes work a bit differently, since instead of affecting all blocks within the selection, they search for a blob of blocks connected to the block clicked with the brush (blob of blocks meaning all connected blocks of the same type as the starting one) and apply hollow or outline only to those blocks. Outline replaces only air (or greenery, if used with -g).

The name outline collides with the one added in #45 (and as of now waiting to be readded in #52). I suggest renaming that one to outline_selection and leaving this new command as plane outline. If that is the case, the renaming should be done when merging that PR.

Implementing these two features brings to light an issue that is to be solved in the future by someone that wants to, not in this PR: flood, hollow, outline and drain all take into account only neighburs in cartesian directions (that is, the 6 direct neighbors of a block, not counting the diagonal ones). This is fine for most cases, but it produces counter intuitive results in some cases. Particularly in cases where an outline brush is meant to be applied multiple times to give a thicker outline: the second application doesn't work properly, since the outline placed by the app is not considered as a "block of blocks" (they connect only diagonally in some cases). I think the best solution for this will be adding a new flag that affects all functions that check neighbors. I'll open an issue later.

@Ghoulboy78 Ghoulboy78 added this to the v1.3.0 milestone Mar 21, 2021
Copy link
Owner

@Ghoulboy78 Ghoulboy78 left a comment

Choose a reason for hiding this comment

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

ok, sorry it took this long to review, but it's pretty good now.

@Ghoulboy78 Ghoulboy78 merged commit 8e9eb9b into master Mar 22, 2021
@Ghoulboy78 Ghoulboy78 deleted the hollow-outline branch March 22, 2021 21:10
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