Skip to content

[Lua] Add eyedropper action to useTool#5692

Merged
dacap merged 1 commit into
aseprite:betafrom
Gasparoken:add-eyedropper-usetool
Mar 18, 2026
Merged

[Lua] Add eyedropper action to useTool#5692
dacap merged 1 commit into
aseprite:betafrom
Gasparoken:add-eyedropper-usetool

Conversation

@Gasparoken

Copy link
Copy Markdown
Member

No description provided.

@Gasparoken Gasparoken self-assigned this Mar 6, 2026
@Gasparoken Gasparoken requested a review from dacap as a code owner March 6, 2026 19:25

@aseprite-bot aseprite-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread src/app/script/app_object.cpp Outdated
// Get the first point from the points table
lua_pushnil(L);
if (lua_next(L, -2) != 0) {
gfx::Point pt = convert_args_into_point(L, -1);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: variable 'pt' of type 'gfx::Point' (aka 'PointT') can be declared 'const' [misc-const-correctness]

Suggested change
gfx::Point pt = convert_args_into_point(L, -1);
gfx::Point const pt = convert_args_into_point(L, -1);

@Gasparoken Gasparoken assigned dacap and unassigned Gasparoken Mar 6, 2026
Comment thread src/app/script/app_object.cpp Outdated
Comment on lines +454 to +455
type = lua_getfield(L, 1, "points");
if (type == LUA_TTABLE) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks like these lines are duplicated, probably checking for the "points" tablet is the first thing we can do, and after that decide the eyedropper path or the ToolLoop one.

@dacap dacap assigned Gasparoken and unassigned dacap Mar 9, 2026
@Gasparoken Gasparoken force-pushed the add-eyedropper-usetool branch from 31b9eb2 to 9b2976d Compare March 10, 2026 12:53

@aseprite-bot aseprite-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

}
// Do the tool loop for other tools
else {
InlineCommandExecution inlineCmd(ctx);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: variable 'inlineCmd' of type 'InlineCommandExecution' can be declared 'const' [misc-const-correctness]

Suggested change
InlineCommandExecution inlineCmd(ctx);
InlineCommandExecution const inlineCmd(ctx);

const bool isSelectionInk = (params.ink ==
toolbox->getInkById(tools::WellKnownInks::Selection));
const tools::Pointer::Button button = (!isSelectionInk ?
(buttonIdx == 0 ? tools::Pointer::Button::Left :

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: conditional operator is used as sub-expression of parent conditional operator, refrain from using nested conditional operators [readability-avoid-nested-conditional-operator]

                                               (buttonIdx == 0 ? tools::Pointer::Button::Left :
                                                ^
Additional context

src/app/script/app_object.cpp:489: parent conditional operator here

      const tools::Pointer::Button button = (!isSelectionInk ?
                                             ^

Comment thread src/app/script/app_object.cpp Outdated
tools::Pointer::Button::Right) :
tools::Pointer::Button::Left);
while (lua_next(L, -2) != 0) {
gfx::Point pt = convert_args_into_point(L, -1);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

warning: variable 'pt' of type 'gfx::Point' (aka 'PointT') can be declared 'const' [misc-const-correctness]

Suggested change
gfx::Point pt = convert_args_into_point(L, -1);
gfx::Point const pt = convert_args_into_point(L, -1);

@Gasparoken Gasparoken force-pushed the add-eyedropper-usetool branch from dd19630 to ad7965b Compare March 11, 2026 15:51
@dacap dacap merged commit 21b07c3 into aseprite:beta Mar 18, 2026
12 checks passed
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.

3 participants