CATROID-1640 Reset embroidery, plot, and laser drawing state on fresh starts

[wslany]

Summary

This PR fixes stale drawing/stitch runtime state being reused across fresh executions.

https://catrobat.atlassian.net/browse/CATROID-1640

If a project ended while a running stitch, plot, cut, or engrave mode was still active, the next fresh run could inherit that old state and create an unintended initial segment from the previous endpoint to the new start position.

The fix resets that runtime state when execution is supposed to start from a blank slate, while preserving the existing behavior for resume-style flows.

Why this approach

The underlying issue was not in the stitch/plot geometry itself, but in sprite runtime state surviving longer than intended.

Sprite keeps drawing-related runtime objects such as:

• runningStitch
• plot
• penConfiguration

Those objects should be preserved for resume behavior like pause/continue and Continue scene, but they must be reset for fresh starts such as project restart and Start scene.

To keep that distinction explicit, this PR introduces a dedicated drawing-state reset helper on Sprite and uses it only on fresh-start lifecycle paths.

Changes

• Added a regression test for sprite drawing-state reset
• Added Sprite.resetDrawingState()
• Reused that helper from resetSprite()

Explicitly reset drawing state on:

• project restart
• Start scene
• Left these flows unchanged:
• stage pause / continue
• Continue scene via scene backup/restore

Result

Fresh runs now start with clean embroidery/plot/laser state, even if no explicit stop brick was used in the previous execution.

This covers:

• embroidery running stitches
• plotting
• laser cutting
• engraving

Testing

Verified with:

./gradlew :catroid:testCatroidDebugUnitTest --tests org.catrobat.catroid.test.content.SpriteDrawingStat

Test on phone: screen recording before and after fix: https://photos.app.goo.gl/yGBKrRXvmYrQhcev6

Your checklist for this pull request

Please review the contributing guidelines and wiki pages of this repository.

• Include the name of the Jira ticket in the PR’s title
• Include a summary of the changes plus the relevant context
• Choose the proper base branch (develop)
• Confirm that the changes follow the project’s coding guidelines
• Verify that the changes generate no compiler or linter warnings
• Perform a self-review of the changes
• Verify to commit no other files than the intentionally changed ones
• Include reasonable and readable tests verifying the added or changed behavior
• Confirm that new and existing unit tests pass locally
• Check that the commits’ message style matches the project’s guideline
• Stick to the project’s gitflow workflow
• Verify that your changes do not have any conflicts with the base branch
• After the PR, verify that all CI checks have passed
• Post a message in the catroid-stage or catroid-ide Slack channel and ask for a code reviewer

[Copilot]

Pull request overview

This PR addresses stale embroidery/plot/laser runtime state being unintentionally reused across fresh executions by introducing an explicit drawing-state reset on Sprite and invoking it on fresh-start lifecycle paths.

Changes:

• Added Sprite.resetDrawingState() to reinitialize pen/plot/running-stitch runtime objects and reused it from resetSprite().
• Reset drawing state when starting a scene fresh (startScene) and when restarting/reloading a project (reloadProject).
• Added a regression unit test covering the drawing-state reset behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
catroid/src/test/java/org/catrobat/catroid/test/content/SpriteDrawingStateResetTest.kt	New unit test verifying that resetDrawingState() reinitializes pen configuration, plot state, and running stitch state.
catroid/src/main/java/org/catrobat/catroid/stage/StageListener.java	Invokes resetDrawingState() on sprites for fresh-start flows (start scene, project reload).
catroid/src/main/java/org/catrobat/catroid/content/Sprite.java	Introduces resetDrawingState() and delegates drawing-related reinitialization from resetSprite() to this helper.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[wslany]

@dorianpercic Should I rewrite the Java production code files to Kotlin here as well? They seem a bit broad for the narrow topic of this PR, and would require more extensive testing. Please let me know what you think.

[dorianpercic]

@dorianpercic Should I rewrite the Java production code files to Kotlin here as well? They seem a bit broad for the narrow topic of this PR, and would require more extensive testing. Please let me know what you think.

No I think our convention was that only new files should be implemented in Kotlin. Maybe in future, to accelerate refactoring to Kotlin, especially with today's LLM capabilities, we might change the policy to let rewrite each changed file immediately to Kotlin, but this might need some more testing on how good this works. Maybe there exists some tools already which alleviate this?

[wslany]

No I think our convention was that only new files should be implemented in Kotlin. Maybe in future, to accelerate refactoring to Kotlin, especially with today's LLM capabilities, we might change the policy to let rewrite each changed file immediately to Kotlin, but this might need some more testing on how good this works. Maybe there exists some toold already which alleviates this?

OK. I've included in my AGENTS.md file the following lines:

Prefer Kotlin over Java for Catroid source code.

When a PR needs changes in an existing Catroid Java source file, prefer rewriting that touched source file to Kotlin as part of the same PR, so a follow-up refactoring ticket is not needed again.

Apply that preference with judgment:

• new source code files should normally be written in Kotlin, not Java
• if a PR touches an existing Java production file, plan for a Kotlin rewrite of that file unless there is a strong reason not to
• keep tests aligned with the same red/green workflow when a Java-to-Kotlin rewrite is part of the change
• do not quietly skip the Kotlin rewrite expectation for touched Java files; call it out explicitly if a rewrite would be too large or risky

[dorianpercic]

No I think our convention was that only new files should be implemented in Kotlin. Maybe in future, to accelerate refactoring to Kotlin, especially with today's LLM capabilities, we might change the policy to let rewrite each changed file immediately to Kotlin, but this might need some more testing on how good this works. Maybe there exists some toold already which alleviates this?

OK. I've included in my AGENTS.md file the following lines:

Prefer Kotlin over Java for Catroid source code.
When a PR needs changes in an existing Catroid Java source file, prefer rewriting that touched source file to Kotlin as part of the same PR, so a follow-up refactoring ticket is not needed again.
Apply that preference with judgment:

• new source code files should normally be written in Kotlin, not Java
• if a PR touches an existing Java production file, plan for a Kotlin rewrite of that file unless there is a strong reason not to
• keep tests aligned with the same red/green workflow when a Java-to-Kotlin rewrite is part of the change
• do not quietly skip the Kotlin rewrite expectation for touched Java files; call it out explicitly if a rewrite would be too large or risky

In refactoring tickets (example: https://catrobat.atlassian.net/browse/CATROID-1548) we have these guidelines:

1. Get rid of "!!" and make sure there can't be any NPE's. Since every Exception in KT is treated like a Runtime Exception be sure you catch them.
2. Take advantage of Null-Safety features in Kotlin (https://kotlinlang.org/docs/null-safety.html).
3. If you find any other smells in the code or unclean code parts feel free to improve them as well. But make sure that the behavior of the code is not changed.

maybe we can add this, and especially Null Safety handling, as well.

[urkat]

Lgtm

[dorianpercic]

Can I ask, what this PR actually really fixes? Since all there is done now is instead of doing:

penConfiguration = new PenConfiguration();
plot = new Plot();
runningStitch = new RunningStitch();

a new function is called, which does exactly the same. I might be confused, but I don't see what this really changes now. Or maybe only the PR description does not match the content; if it's only a small refactoring ticket though, then I get it.

[wslany]

Can I ask, what this PR actually really fixes? Since all there is done now is instead of doing:

penConfiguration = new PenConfiguration();
plot = new Plot();
runningStitch = new RunningStitch();

a new function is called, which does exactly the same. I might be confused, but I don't see what this really changes now. Or maybe only the PR description does not match the content; if it's only a small refactoring ticket though, then I get it.

You are right, thanks a lot for that save! The apk Katja and I tested was created before a "cleanup" in response to the “redundant reset” review by Copilot. Shouldn't have done that, the order was wrong...

The supporting evidence from reflog/history is:

• #763398e5 = last commit with the StageListener loops
• APK build happened before #fcb4dfd7
• #fcb4dfd7 is exactly where those loops disappeared

This also revealed the insufficiency of the current test. I will by fixing that in my next commit.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[wslany]

@dorianpercic Thanks again. The PR has been updated, and it now contains the actual fix again.

What changed:

• the branch was rebuilt into a proper red/green history
• the old helper-only test was replaced with a lifecycle regression test
• the production fix in StageListener was restored

Why the old test was not good enough:

• the previous test only called the new helper directly on Sprite
• that proved the helper worked, but not that the real bug path was fixed
• so it could stay green even if project restart / Start scene still reused stale drawing state in practice

Why the new test is more meaningful:

• the new test exercises StageListener.reloadProject(), i.e. an actual fresh-start lifecycle path
• it first puts a sprite into a “dirty” state with active pen / plot / running-stitch data
• then it triggers the fresh-start flow
• before the fix, the same pen/plot/stitch objects were still there, so the test fails
• with the fix, the drawing state is replaced/reset during the fresh-start path, so the test turns green

So the two commits now tell the intended story:

• 98c4bc27 test: add regression for fresh-start drawing reset
  this is the red test against old behavior
• bc710825 fix: reset drawing state on fresh starts
  this restores the StageListener resets and turns the test green

And in addition to the automated regression test, I also manually verified the rebuilt APK on-device just now: the embroidery start behavior is correct again, so eyeballs 1.0 also confirm the fix. 👀

[dorianpercic]

Thanks, now it's clear! :)