Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Revert "Revert "Draw pen lines via fragment shader""" #582

Merged
merged 1 commit into from
Apr 9, 2020

Conversation

fsih
Copy link
Contributor

@fsih fsih commented Apr 9, 2020

Reverts #580

This will cause some compatibility issues with projects drawn with the pre-existing Scratch 3.0 jagged, oval pen, and Scratch team members want to spend some more time discussing the implications

@fsih fsih merged commit 58bd9cb into develop Apr 9, 2020
@fsih fsih deleted the revert-580-revert-559-revert-438-pen-shader branch April 9, 2020 16:30
@adroitwhiz
Copy link
Contributor

adroitwhiz commented Apr 9, 2020

It's frustrating to wait a year for my PR to be merged, and then reverted because in the year it took for it to be merged, some people somewhere may have made projects relying on the behavior that existed because it wasn't merged 😕

This also raises a lot of questions about other behavior-effecting PRs like #479 and #542 -- those certainly change the way "touching" blocks behave. Will all future versions of Scratch, in whatever language they're written in, have to implement two different code paths for "touching" blocks depending on the number of pixels in the thing being touched, because of compatibility concerns with projects that rely on the buggy behavior of "touching" blocks?

It seems like the longer these bugs go without being fixed, or the more often that fixes which solve many problems are reverted because of a single regression that may not have even been reproduced (a common occurrence when modifying hacky code while trying to keep PRs small enough to review), the greater the compatibility concern becomes and the greater the burden on future maintainers to keep "bug-for-bug compatibility" with the existing codebase.

This means that adding hacky user-facing changes is not the only way to increase tech debt-- simply leaving those changes in the codebase increases tech debt with each passing day as Scratchers come to rely on broken behavior, and every future bugfix becomes riskier to accept despite nothing changing in the codebase.

@fsih
Copy link
Contributor Author

fsih commented Apr 9, 2020

I totally agree! "Because the pen had a bug and spent some time not being a circle" definitely isn't a good reason to leave it as not a circle forever!

The way our release process is set up, most of our engineering team doesn't get to see the bug fixes they didn't work on until soon before we're supposed to release. If they have any questions or concerns about any of the bug fixes, we have to revert them to remove them from the release, because there aren't more than ~15 minutes to discuss each bug before release time. Reverting gives us a week to settle discussions before putting changes back in. It definitely doesn't mean we've decided not to put the change in anymore. I know it must be really frustrating for you!

Overall, the team is excited for this change, and everyone thinks the pen looks a lot better now! We haven't found any projects that would be broken by the pen changes. But there are a lot of pen projects, and it really sucks when your project breaks one day for no apparent reason, so we wanted extra time to really thoroughly test this change.

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.

2 participants