kbuffington / Georgia

Dynamic foobar2000 theme
286 stars 16 forks source link

Library panel crashing #93

Closed dwmartin0906 closed 3 years ago

dwmartin0906 commented 3 years ago

Sorry to bug you again. Now I'm getting a different error. I've reverted back to the previous version so I'm good to go for now. I download your changes as soon as they come out because it makes retrofitting my changes easier. And, frankly, I really don't have anything else to do. Playing with Foobar and Georgia is my main hobby. But I don't want to keep bugging you. If you would prefer I wait to report any issues until you release an official version just let me know. I keep backups of my backups so I can always easily revert to a working version. Thank you for your help.

Error: Spider Monkey Panel v1.4.1 (Georgia: Georgia v2.0.2 by Mordred) a is undefined

File: Panel_Library.js Line: 930, Column: 10 Stack trace: arraysIdentical@Panel_Library.js:930:10 treeState@Panel_Library.js:1069:15 on_metadb_changed@Panel_Library.js:3221:16 on_metadb_changed@georgia-main.js:1684:12

kbuffington commented 3 years ago

Do you know what you were doing when that error occurred? Not sure how to reproduce it, but it looks like it could be serious (as in I seriously screwed something up).

kbuffington commented 3 years ago

Nevermind, I can reproduce. Should have a fix by this evening.

kbuffington commented 3 years ago

Figured out what I did wrong, but instead decided to muddy the waters by updating parts of this file to use updated logic from a more recent version of Library_panel. Lemme know if that works out your issues like it did mine.

dwmartin0906 commented 3 years ago

As soon as I click on the library button, Georgia crashes. I searched for DrawText in the Georgia-master folder and in your changes for Panel_Library.js and couldn't find any statements defining it. Maybe you uploaded the wrong version?

Error: Spider Monkey Panel v1.4.1 (Georgia: Georgia v2.0.2 by Mordred) DrawText is not defined

File: Panel_Library.js Line: 660, Column: 2 Stack trace: panel_operations@Panel_Library.js:660:2 initLibraryPanel@Panel_Library.js:3424:7 btnActionHandler@Control_Button.js:297:5 onClick@Control_Button.js:160:3 buttonEventHandler@Control_Button.js:83:16 on_mouse_lbtn_up@georgia-main.js:1768:3

kbuffington commented 3 years ago

Sorry, I forgot to include a file. Should be good now.

dwmartin0906 commented 3 years ago

When I opened the panel I noticed that the top node which usually displays the type of view had undefined instead of Artist as I expected. I went to change the view to see if that fixed it and it crashed. Now it crashes every time I try to open the panel. Actually, I just discovered it is now crashing just a few seconds after starting Foobar without doing anything else. I guess it's a good thing I have a separate install to use as a test bed.

Error: Spider Monkey Panel v1.4.1 (Georgia: Georgia v2.0.2 by Mordred) br[i].name is undefined

File: Panel_Library.js Line: 1812, Column: 9 Stack trace: LibraryTree/this.buildTree@Panel_Library.js:1812:9 LibraryTree/this.branch@Panel_Library.js:1787:8 rootNodes@Panel_Library.js:1256:43 lib/<@Panel_Library.js:3297:16

dwmartin0906 commented 3 years ago

Crashes on open of library panel and then on all subsequent opens of Foobar.

Error: Spider Monkey Panel v1.4.1 (Georgia: Georgia v2.0.2 by Mordred) v.name is undefined

File: Panel_Library.js Line: 1811, Column: 9 Stack trace: LibraryTree/this.buildTree/<@Panel_Library.js:1811:9 LibraryTree/this.buildTree@Panel_Library.js:1810:7 LibraryTree/this.branch@Panel_Library.js:1785:8 rootNodes@Panel_Library.js:1254:43 lib/<@Panel_Library.js:3316:16

dwmartin0906 commented 3 years ago

The crashes I were getting were caused by setting the root node property to 2. Changing the root node property to anything other than 0 - hide causes a crash. Also, the filter button doesn't seem to work. And when performing a search the x button to clear the search doesn't appear. But manually clearing the search term does work. I may be doing something wrong, but I wanted to let you know what I am seeing.

apr292000 commented 3 years ago

Error: Spider Monkey Panel v1.4.1 ({7641A6FA-005C-4850-A250-78849046A0A1}: Georgia v2.0.2 by Mordred) logo is null

File: georgia-main.js Line: 1528, Column: 4 Stack trace: on_playback_new_track@georgia-main.js:1528:4

kbuffington commented 3 years ago

@apr292000 and @SpiffyJUNIOR the issues you guys are experiencing have nothing to do with the library panel (and the issues that dwmartin has been helping me find/debug are in an unreleased version). You'd do better opening a new ticket and saying whether you're using the official 2.0.2 or if you pulled from master.

kbuffington commented 3 years ago

Actually with a little investigating I've submitted changes which will prevent both crashes from happening regardless of what version you're on.

kbuffington commented 3 years ago

I think all the library crashes might be sorted out now in master. If you want to take a look @dwmartin0906 and see if you can't break it, I'd appreciate it :)

dwmartin0906 commented 3 years ago

I haven't been able to make it crash so far, but will keep testing. I did find one issue, though. When I switch views, for example from artist to album, 'Search' disappears from the search bar and I can't perform any searches until I close the panel and reopen it. But you are definitely making progress. I'll let you know if I find anymore issues. On a more positive note, some issues I had with smooth scrolling in the playlist panel seem to have disappeared.

kbuffington commented 3 years ago

Damn. The disappearing "Search" thing was driving me crazy earlier and then it stopped happening so I thought I'd inadvertently fixed it. I think somehow the positioning of the Search label and the clickable area is getting moved outside the Library panel, but I can't figure out how. Thanks for the 2nd set of eyes!

dwmartin0906 commented 3 years ago

I've been playing with the library panel since your last changes and so far everything is great. Changing the background and text color field names threw me for a loop after I retrofitted my changes, but I figured it out. I also wanted to mention that scrolling is still messed up in the playlist panel when showing now playing and having smooth scrolling turned off, just in case you thought you had it fixed. Thanks again for all your hard work.

dwmartin0906 commented 3 years ago

I really like your newest version, particularly the new options menu and the ability to assign different actions for single clicking. I did notice that the genre view works a little differently. In the right click menu 'expand' is greyed out at the genre level. Also the genre view now lists each individual album for the selected genre. It use to group them by artist, which I personally prefer. These changes may have been intentional, but I wanted to let you know just in case.

dwmartin0906 commented 3 years ago

Sorry, my bad. I'd forgotten that I had changed the genre view properties in my version to group by artist and display some additional fields. Everything is looking great so far.

kbuffington commented 3 years ago

Finally squashed the scrolling issue. Thanks for reminding me, and thanks for the testing on the Library Panel. So many moving parts that it's hard to get working perfectly. Seems pretty solid on my end now at least.

dwmartin0906 commented 3 years ago

Yep, scrolling is fine now, with or without smooth scrolling turned on. And I haven't had any issues with the Library Panel. I do have a question, though. The Library Panel has single click options for sending tracks to the playlist. One of these is add if playing and another is add if content. I'm not quite sure what the difference is. And I'm not sure I understand their function. I assumed they would add to existing tracks in the playlist. But instead, they replace the existing tracks just like the send and play option. Am I missing something?

kbuffington commented 3 years ago

No clue what "Send to Playlist && Play [Add if Content]" was supposed to do, so I removed. "Send to Playlist && Play [Add if Playing]" seems to have never been implemented so I added the code to do what it says. It's real dumb IMO, but it does it now :)

dwmartin0906 commented 3 years ago

Sorry, I didn't mean to make more work for you. I was just curious. You might want to consider changing the wording, though, to avoid any confusion. As coded now, it adds to the Georgia Playlist regardless of whether that playlist is playing or another playlist is playing, or no playlist is playing. But it doesn't actually play the playlist if another playlist is playing. If you want it to play, you would need to comment out the following lines of code. I suggest just "Add to Playlist". But you don't need to change anything on my account. I'm just letting you know what I found. Thanks again for all your hard work.

if (autoPlay === 3) { autoPlay = false; // add to playlist if already playing }

kbuffington commented 3 years ago

Think everything should be fixed in here now that 2.0.3 has officially released.