New addon: asset-ctx-menu - #3418

You know I very recently made a PR for a delete others option button. Which hasn't been merged yet. You created a PR which adds the same functionality without building upon it.

I think each addon should only do one thing if possible - moving costumes and deleting them isn't the same thing. Using separate addons makes it easier for users to find them because of how the settings page was designed: searching for "Wikipedia" doesn't show the forum toolbar addon.

I think each addon should only do one thing if possible - moving costumes and deleting them isn't the same thing. Using separate addons makes it easier for users to find them because of how the settings page was designed: searching for "Wikipedia" doesn't show the forum toolbar addon.

@mxmou

Yes, but I think there should be a clear statement that defines what is an addon and what is not, while making this change. I suggest this one:
"an addon is any feature added, removed or modified directly about Scratch. Configurations or changes to an addon should be a part of the addon as a setting."

e.g Findbox is a feature that adds something to Scratch itself, so it can be an independent new addon, yet there shouldn't be an addon for each extension, as there is Automatically Add Extensions.

Split up addons can probably remain in a container-like thing that resembles the original addon as well.

I don't think these addons should be merged.
I would like to move "send to top/bottom" outside of devtools at some point, tho. The standalone devtools extension should continue to have that feature, but not the addon. And we should automatically enable the addon if the user has devtools enabled when we ship the update.
@apple502j Don't you think we could have an addon.tab.createBlockContextMenu equivalent for sprites, costumes and sounds? That seems a better idea than merging the addons.

@WorldLanguages What I'm thinking is:

• Deprecate standard DevTools and migrate people to SA. Maintaining two different extensions are difficult, especially when compatibility matters.
• Add settings to control which ones to show.
• Migration code can enable the addon without delete buttons (with settings).

IMO, this can be one addon - it adds some actions to the context menu of assets for asset management. What we don't want is addons like "add buttons next to stop button addon" which doesn't make sense.

Deprecate standard DevTools and migrate people to SA. Maintaining two different extensions are difficult, especially when compatibility matters.

This is already happening naturally. We don't need to artificially make it happen faster.
Maintaining the extension hasn't been very complex at least in my experience.

IMO, this can be one addon - it adds some actions to the context menu of assets for asset management. What we don't want is addons like "add buttons next to stop button addon" which doesn't make sense.

Eh, good point. I'm not that convinced we cannot merge both addons into a single one by now.

@WorldLanguages I implore you to not merge a PR which duplicates the functionality that I created in my PR. As a software developer you should always extend parts of the code and only replace code if it is necessary. Here apple decided that he just didn't like the style of my code and rather than request changes or wait to merge and makes those changes he rewrote it completely (in a less maintainable and less efficient way). Now I agree that an add on which combines both context menu options should exist but this is the complete wrong to do it. The correct way would be to have waited for my PR to merge. Then discuss with the team how the two add ons should be best merged. I would much prefer my code being extended to include the other addon rather than made redundant by a diverging PR. This kind of behaviour would have got you into serious trouble where I work and if you repeatedly did it you probably would get fired. On a separate note event listeners is objectively the best way to do things. This wait for an element function should not be used when there exists an easy alternative that does not put unnecessary load on the browser. There's a reason Griffpatch also did it this way. And if your reason is to make it match the rest of your codebase, then I would actually recommend changing your code base to use event listeners where possible instead.

IMO, this can be one addon - it adds some actions to the context menu of assets for asset management. What we don't want is addons like "add buttons next to stop button addon" which doesn't make sense.

Eh, good point. I'm not that convinced we cannot merge both addons into a single one by now.

If we merge these two, why shouldn't they be merged with sprite folders, which also adds actions for asset management to the asset context menu?

@mxmou This addon does not affect sprites.

@Joeclinton1 Three things.

1. We are not your company. We do not fire people for that.
2. Complete refactoring of code is something we often do.
3. There is no evidence that event handler is faster. Griffpatch did it that way because the alternative method didn't have good browser support back then.

@mxmou This addon does not affect sprites.

If that's important, sprite-folders should be split into three addons (for sprites, costumes, and sounds) and the costume one could then be merged with this one.

Each addon should do a single thing, but it can do it in multiple places.

@apple502j I checked how exactly the wait for element works and it is doing a query every single time the page changes. You have to believe me when I say that's super inefficient. Clearly you aren't going to listen to me so I'll stop discussing this. Also I won't be contributing any more in the future as clearly this repository has low standards if this is considered the norm.

@Joeclinton1 Well, not every single time. There are optimization paths and in many computers the difference is unnoticeable. Computers that lack performance generally run poorly on vanilla Scratch.