Select a color in the palette (so that button is set)
Hover over the canvas (so that pointer is set)
Optionally draw on the canvas. This changes which error is thrown (because pointer_previous gets set)
Select the Eraser tool
Press numpad +/-
Looking through the usages of button and ctrl in the codebase, I don't see any reason why these globals should be set when clicking on a color well. Perhaps it was out of an ill-conceived notion of logical consistency, or blind copy pasting, or, heavens forbid, the slight brevity of dropping e. used to access the state from the event object. Perhaps the Edit Colors dialog used these before refactoring it to determine the color_selection_slot externally?
Digging up the code with git blame, this code's been in the project for ten years. It was introduced here.
This was before the Edit Colors dialog, when it used the browser's native color picker. It looks like it was using it to persist the state while selecting a color with the native color picker. Since it's before I implemented the Edit Colors dialog, there's no function call to add a parameter to which would hold the state. It uses the change event of an <input type="color">, so it needed a variable to store which mouse button was used, and thus which color selection to change, and I may have used the existing globals button and ctrl out of convenience. So it's similar to my last theory, but on a longer time frame.
And yes, I have a global called button, it's pretty bad.
Note: I already have a fix for this. (I just wanted a space to gather my thoughts without framing it in the past tense, mainly.)
When pressing numpad plus/minus (or regular +/-) to increase or decrease the size of the eraser, it can throw an error:
or:
To reproduce the errors:
buttonis set)pointeris set)pointer_previousgets set)Looking through the usages of
buttonandctrlin the codebase, I don't see any reason why these globals should be set when clicking on a color well. Perhaps it was out of an ill-conceived notion of logical consistency, or blind copy pasting, or, heavens forbid, the slight brevity of droppinge.used to access the state from the event object. Perhaps the Edit Colors dialog used these before refactoring it to determine thecolor_selection_slotexternally?Digging up the code with git blame, this code's been in the project for ten years. It was introduced here. This was before the Edit Colors dialog, when it used the browser's native color picker. It looks like it was using it to persist the state while selecting a color with the native color picker. Since it's before I implemented the Edit Colors dialog, there's no function call to add a parameter to which would hold the state. It uses the
changeevent of an<input type="color">, so it needed a variable to store which mouse button was used, and thus which color selection to change, and I may have used the existing globalsbuttonandctrlout of convenience. So it's similar to my last theory, but on a longer time frame.And yes, I have a global called
button, it's pretty bad.