-
Notifications
You must be signed in to change notification settings - Fork 362
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
New addon: sort assets by alphabetical order #7247
base: master
Are you sure you want to change the base?
Conversation
I have implemented a better sorting system, which uses natural sort. this is just...better, but it is slower, and when a lot of things are sorting, it can take a while. I am not sure if this can be improved. besides that, sprites still need to be done, and for consistency i would like to find a way to make the menu close when the item is clicked on while it is greyed out (which you will see when you click on the grayed out "restore" button) i am not sure how to do either of those, and believe me i have tried. if you have ideas, please tell me, and if you think those things are fine as is, i guess no changes to them need to be made |
2024-03-03.19-44-27.mp4oh, and anyone know why it doesnt come back when the addon is reenabled while the menu is open and it was disabled before the menu was opened, but it does come back when it was enabled before the menu was opened, disabled when the menu was open, and reenabled when the menu was still open? |
For sprite sorting a function exists similar to reorderSound or reorderCostume, it's called reorderTarget. |
oh my god thank you how did i not see that |
(latest commit does not work) |
ok im pretty sure it works, sprites are done, the performance is actually really good, now just need to figure out how to close that menu |
Chatgpt thinks your natural sort can be done by using localCompare, which would make it alot more concise. Could you test? Array.prototype.naturalSort = function() {
return this.sort((a, b) =>
String(a).localeCompare(String(b), undefined, {numeric: true, sensitivity: 'base'})
);
}; |
well ill be damned it seems to sort the same |
See here You need to get the menu-bar and call Edit: if you look at what onRequestCloseEdit calls, you can probably close the menu with a redux dispatch |
This comment was marked as spam.
This comment was marked as spam.
How do you find this shit, I looked so hard...I can't do this tonight so remind me to do it tmr |
I am happy to have figured out that redux event on my own, redux is one of my weaknesses so that felt good, anyway i think this is all done now! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actual reordering works perfectly for sprites, costumes and sounds in my tests. Edit menu works fine.
- Dynamic/enable works properly in all cases except the edge case that the menu is already open. Can you make it check if the menu is open when enabled and insert the option?
- The originally selected asset is no longer selected after sorting, can you fix this? Take a look at the code in New Addon: Asset Conflict Resolution Dialog #7232 for how to select an asset. You can probably copy and paste it. You'll need to do the sprites a little bit differently though, but it's really just a case of adding another selector.
- I would've preferred a context menu option over an edit menu option, but i guess since this is applied to all costumes instead of one it kind of makes sense for it to not need context. Having to go to the code tab to sort sprites is a little awkward but sprites are kind of an after thought anyway.
- I don't like the While True loop. Addons should not use while loops unless absolutely neccessary for performance reasons. There is an event triggered when that menu is opened why not just trigger your code on that? Otherwise the code is good.
|
should be fixed now, i cant repo the edit menu not closing as it always does for me, but this should be working now. EDIT: nope, the edit menu closing was something else, fixing that too in the next commit (it currently doesnt close after it succesfully orders them, it should.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
I think this just needs reviews and approvals |
Joeclinton1 has already approved this pull request, so you don't have to request another review until you make changes. |
export default async function ({ addon, msg, console }) { | ||
const vm = addon.tab.traps.vm; | ||
|
||
Array.prototype.naturalSort = function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to add this to the prototype? Doesn't sound like the best idea. Why not just make it a normal function that receives the array as an argument?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not necessary but I find it much more convenient in this scenario and it has no negative effects as far as I can tell.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the expected behavior of Ctrl+Z/undo after sorting alphabetically? |
Well one would assume it will undo the reordering.. I'm not sure if this actually happens, I think all I need to do is call some function in the VM and that will save it as a snapshot that can be undone. |
Are you talking about the undo buttons in the costume editor? Ctrl+Z can't usually undo costume deletions, additions, or reorders, AFAIK. |
That is correct, but perhaps in this case it should? Or maybe change the sort button to be restore previous order? |
Well, I meant |
Related addon idea: notification with undo button when deleting things or doing other destructive actions, including alphabetizing a sprite's costumes or sounds. |
Resolves suggestion from the community server
Changes
Adds an option to the edit menu to sort the assets by alphabetical order.
Reason for changes
In a future version of this when I make it, it could be used to fix the order of broken gifs? will add settings for different sorting methods, current is basic js array sort.
Tests
works on brave for both costumes and sounds, sprites are not done yet, and i need pointers on how to go about that. if its not feasible, sprites will be ommited (they arent that useful to order anyway)