Skip to content

The stack tool can delete stacks now.#4

Open
Teh-Mad-Cow wants to merge 3 commits into
krawthekrow:mainfrom
Teh-Mad-Cow:stack-delete
Open

The stack tool can delete stacks now.#4
Teh-Mad-Cow wants to merge 3 commits into
krawthekrow:mainfrom
Teh-Mad-Cow:stack-delete

Conversation

@Teh-Mad-Cow
Copy link
Copy Markdown

The stack tool now switches to a toolset like the config tool, so that the secondary button deletes whole stacks. It supports brushes, lines, and boxes.
While it performs better than the stack tool when dragging, it could still use some improvement.

Signed-off-by: Teh-Mad-Cow <5084133+Teh-Mad-Cow@users.noreply.github.com>
Signed-off-by: Teh-Mad-Cow <5084133+Teh-Mad-Cow@users.noreply.github.com>
Signed-off-by: Teh-Mad-Cow <5084133+Teh-Mad-Cow@users.noreply.github.com>
@krawthekrow
Copy link
Copy Markdown
Owner

  1. Stack tool already has a framework to gather all the particles affected by a draw. It would be better to reuse those instead of reimplementing draw functions.
  2. If floodfill isn't ready, it would make sense to omit DrawFill altogether for now.
  3. If stack tool is toggleable, "[Stk Tool]" or something like that should show at the top left of the debug HUD (see close to the end of OnDraw()).
  4. Having the delete-stack tool be connected to stack tool feels suboptimal from a user interface standpoint, since they perform fundamentally different functions. It might be more natural to make it part of stack mode. Here are two possible proposals:

(A) Make the default right-click action delete stacks, and make the stack mode right-click action delete single particles from stacks. In this proposal, unmodified-right-click in stack mode could also be made to delete only one particle per mouse-down. This would be most consistent with stack mode's existing intuition, but would make default right click laggy and mess up existing users' workflow.
(B) Keep the default right-click action the same, but have stack mode's right click delete entire stacks when ctrl or shift is held down, and delete only one particle per mouse-down otherwise. This would cast stack mode as a more general stack utility mode.

I'm leaning more towards (B), but let me know what you think.

@Teh-Mad-Cow
Copy link
Copy Markdown
Author

Teh-Mad-Cow commented Mar 5, 2022

  1. Stack tool already has a framework to gather all the particles affected by a draw. It would be better to reuse those instead of reimplementing draw functions.

I wasn't sure how to do that originally, but I think I got it. The exact implementation might vary, but is this reasonable? I could change how StackTool::ProcessParts uses parts based on whether a new flag is set in the sim: StackDeleteMode. Then when we're in [Stack] mode and detect the active tool is DEFAULT_PT_NONE (or something), we set the StackDeleteMode flag to true and override the active tool to StackTool. That way we can use that code to do the erasure. I'm open to any suggestions/corrections though.

  1. If floodfill isn't ready, it would make sense to omit DrawFill altogether for now.

Oops yeah I wasn't planning to continue work on it, till you mentioned how [Stack] mode treats clicks. I'm going to save that for last though, as it's still incredibly niche. I only wanna include it for completeness

As for points 3 and 4, yeah I'll go ahead and do that. Option B is the least disruptive to regular workflow. Took me a bit to come around, but yeah it really does fit there better in the [Stack] mode.

@krawthekrow
Copy link
Copy Markdown
Owner

Your changes had inspired me to rework the toolset framework a bit, so hold on the changes related to making the stack tool toggleable for now. I think I might end up just going ahead and merging that manually.

For (1), I would recommend abstracting those functions so that they return a vector of particles, and putting them in a separate file. The reason why I mentioned them is because its implementation of DrawLine is faster, and it's a good starting point if you're planning to port DrawFill. If this sounds complicated, it's fine if you just copy-paste your existing routines; I can do the refactor myself.

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