SeExpr: structured testing fixes.
This MR fixes various issues detected with SeExpr in Krita Artists.
Bug list from structured testing
-
If the name is the same as the existing resource, the resource folder will not generate or overwrite the old file 🟥 -
In addition, there is no uniform specification for file names in resource folders. Sometimes it is “123.KSE”. Sometimes it is “seexpr_scripts123.kse” 🟥 -
When you click “overwrite preset”, this button does not turn gray, and the “reload the preset” button always exists. If you click “reload the preset” at this time, krita crashes 🟥 -
You cannot change the name, a new file will appear in the resource folder, it will still have the same name as before 🚗
The following bugs are strictly resource system purview (depends on !1072 (merged)):
-
Cannot overwrite existing resources when importing 🟥. -
After deleting a resource, creating another resource with the same name is invalid 🟥 -
After deleting a file, importing a file with the same name will have the same problem as above, and the resource will not appear - Tiar says these two are one and the same. Halla says we should implement a "preset reuse" dialog, which is outside the scope of this resource type
-
Sometimes the resource cannot be rendered. :red_square: The resources in the resources folder have not changed 🟥
This MR completes changes made in 126565fc.
Bug list from feedback and improvements thread
-
Rename feature - included above
-
When saving preset with empty thumbnail krita crashes. - fixed in 0636d6f1
-
There is no clear indication that the script is stored to a file- This relies on the "dirty" indicator, which is fixed above. Overwrite Preset is !1072 (merged) scope.
-
Unable to change icon for the script postfactum -
Stability. - This is triggered by a runtime assertion on usage of
curve
andccurve
with invalid interpolation parameters.
- This is triggered by a runtime assertion on usage of
-
Unclear checkbox - This is used upstream, but I can try hiding them.
- Script edit
-
The tab symbol is too large. I would set it to default of value 4 (and replace with spaces). - (making it configurable would require discussion and would be for 5.1)
-
Line numbers. For better error messages interpretation and debugging adding line numbers to editor would be nice to have. - Subsumes "There is no way for a user to understand what 76 and 77 mean." (Qt cursors think as indexes on a string instead of line/columns.)
-
Differentiate between different assignments of the same variable- For a future release, this needs parser support.
-
Signal to the user that the script is being rendered- (Halla:) That would be good, but an hourglass during the editor freeze would be good to have – if we don’t have that already.
- EDIT (Oct 5): we already do, the preview generator gets an instant snapshot of the script code and the layer docker shows the progress.
-
-
Lock editing for scripts shipped with Krita. It is easy to mess things up and loose scripts shipped with krita. Maybe making such that the user have to make a copy first, and then edit it.- Out of scope; although the resource system now has overrides for bundled presets.
-
This is one is more like an idea. But… Node aditor for SeExpr. Something similar how Blenders’ Node based material editor with Node Wrangler works. Would be a killer feature.- Out of scope
Bug list from b.k.o
Test Plan
Build Krita and structure test SeExpr scripts.
Formalities Checklist
-
I confirmed this builds. -
I confirmed Krita ran and the relevant functions work. -
I tested the relevant unit tests and can confirm they are not broken. (If not possible, don't hesitate to ask for help!) -
I made sure my commits build individually and have good descriptions as per KDE guidelines. -
I made sure my code conforms to the standards set in the HACKING file. -
I can confirm the code is licensed and attributed appropriately, and that unattributed code is mine, as per KDE Licensing Policy.
Edited by Amy spark