Fix #108096: UI: copy-paste buttons can change source properties #108270

Philipp Oeser · 2023-05-25T13:55:03+02:00

Philipp Oeser commented

2023-05-25 13:55:03 +02:00

Pasting numerical array buttons happens with Ctrl + Alt + V.
Holding Alt also triggers uiSelectContext, so having other nodes/
objects etc. selected while doing this would try to copy the pasted
values back to other objects (possibly to the ones you pasted from) and
that happens relative to the original value, so the value actually
changes.

NOTE: the Ctrl + Alt + V shortcut can also be used on non-array buttons, so was an issue for them as well.

To prevent the "copy-to-selected" behavior, refine the IS_ALLSELECT_EVENT macro to be more specific.

Pasting numerical array buttons happens with `Ctrl + Alt + V`. Holding `Alt` also triggers uiSelectContext, so having other nodes/ objects etc. selected while doing this would try to copy the pasted values back to other objects (possibly to the ones you pasted from) and that happens relative to the original value, so the value actually changes. NOTE: the `Ctrl + Alt + V` shortcut can also be used on non-array buttons, so was an issue for them as well. To prevent the "copy-to-selected" behavior, refine the `IS_ALLSELECT_EVENT` macro to be more specific.

👍 1

Philipp Oeser added 1 commit 2023-05-25 13:55:14 +02:00

916d9018d4 Fix #108096 : UI: copy-paste array buttons can change source properties

Pasting numerical array buttons happens with `Ctrl + Alt + V`.
Holding `Alt` also triggers uiSelectContext, so having other nodes/
objects etc. selected while doing this would try to copy the pasted
values back to other objects (possibly to the ones you pasted from) and
that happens relative to the original value, so the value actually
changes.

To prevent the "copy-to-selected" behavior, check for the specific button
combination in `ui_apply_but` and skip it in that case.

Philipp Oeser added this to the User Interface project 2023-05-25 13:55:22 +02:00

Philipp Oeser requested review from Harley Acheson 2023-05-25 13:55:33 +02:00

Philipp Oeser requested review from Julian Eisel 2023-05-25 13:55:43 +02:00

Philipp Oeser requested review from Campbell Barton 2023-05-25 13:55:51 +02:00

Harley Acheson commented

2023-05-26 01:19:59 +02:00

This isn't just for arrays:

This isn't just for arrays: ![Paste1.gif](/attachments/734aa1cb-21ed-4b7d-9388-6ce6ebe32a5b)

Paste1.gif

32 KiB

Philipp Oeser changed title from ~~Fix #108096: UI: copy-paste array buttons can change source properties~~ to Fix #108096: UI: copy-paste buttons can change source properties

2023-05-26 08:48:00 +02:00

Philipp Oeser commented

2023-05-26 08:49:45 +02:00

This isn't just for arrays:

With the same Ctrl + Alt + V shortcut you mean? Clarified the commit message in that regard.

> This isn't just for arrays: > > ![Paste1.gif](/attachments/734aa1cb-21ed-4b7d-9388-6ce6ebe32a5b) > With the same `Ctrl + Alt + V` shortcut you mean? Clarified the commit message in that regard.

Harley Acheson commented

2023-05-26 17:03:37 +02:00

With the same Ctrl + Alt + V shortcut you mean?

Yes, the above was with the same ctl-alt-c and ctrl-alt-v process. In case is_array_paste should be reconsidered for variable name.

> With the same `Ctrl + Alt + V` shortcut you mean? Yes, the above was with the same ctl-alt-c and ctrl-alt-v process. In case `is_array_paste` should be reconsidered for variable name.

Philipp Oeser commented

2023-05-26 17:19:41 +02:00

With the same Ctrl + Alt + V shortcut you mean?

Yes, the above was with the same ctl-alt-c and ctrl-alt-v process. In case is_array_paste should be reconsidered for variable name.

well the shortcut is meant for array pasting (see argument for ui_but_paste), so think it still makes sense

> > With the same `Ctrl + Alt + V` shortcut you mean? > > Yes, the above was with the same ctl-alt-c and ctrl-alt-v process. In case `is_array_paste` should be reconsidered for variable name. > > well the shortcut is meant for array pasting (see argument for `ui_but_paste`), so think it still makes sense

Campbell Barton commented

2023-05-30 15:53:32 +02:00

This fix feels too spesific, we could instead expand IS_ALLSELECT_EVENT so it only responds to events which are expected to be used for multi button editing.

e.g.

#  define IS_ALLSELECT_EVENT(event) \
    (((event)->modifier & KM_ALT) != 0 && \
     (ISMOUSE((event)->type) || ELEM((event)->type, EVT_RETKEY, EVT_PADENTER)))

(this can be made into a function too).

This way we don't need to pick out individual keys whenever this kind of conflict occurs.

This fix feels too spesific, we could instead expand `IS_ALLSELECT_EVENT` so it only responds to events which are expected to be used for multi button editing. e.g. ``` # define IS_ALLSELECT_EVENT(event) \ (((event)->modifier & KM_ALT) != 0 && \ (ISMOUSE((event)->type) || ELEM((event)->type, EVT_RETKEY, EVT_PADENTER))) ``` (this can be made into a function too). --- This way we don't need to pick out individual keys whenever this kind of conflict occurs.

Philipp Oeser commented

2023-06-01 14:18:23 +02:00

This fix feels too spesific, we could instead expand IS_ALLSELECT_EVENT so it only responds to events which are expected to be used for multi button editing.

Sounds better indeed, thx for the heads up!
Will update the PR shortly

> This fix feels too spesific, we could instead expand `IS_ALLSELECT_EVENT` so it only responds to events which are expected to be used for multi button editing. > Sounds better indeed, thx for the heads up! Will update the PR shortly

Philipp Oeser added 1 commit 2023-06-01 15:17:32 +02:00

b27578b758 tweak event handling

make IS_ALLSELECT_EVENT more specific in regards to events which are
expected to be used for multi button editing (instead of checking the
events for array pasting specifically)

Campbell Barton reviewed 2023-06-06 15:39:14 +02:00

source/blender/editors/interface/interface_handlers.cc Outdated

						
				@ -277,3 +277,3 @@

				                                   const double value_orig);

				#  define IS_ALLSELECT_EVENT(event) (((event)->modifier & KM_ALT) != 0)

				#  define IS_ALLSELECT_EVENT(event) \