monome / norns

norns is many sound instruments.
http://monome.org
GNU General Public License v3.0
630 stars 145 forks source link

favorites: removed/renamed script remains starred at top #1212

Closed dndrks closed 2 years ago

dndrks commented 3 years ago

some small weirdness with 'favorites' -- if a starred script is removed or renamed, its entry in the favorites table persists, which creates a ghost entry in the favorites section.

unsure what would be the best way to address, but perhaps a quick read on the favorites table to check whether the script path is still valid (and if invalid, remove the entry) could be enough? what do you think @zzsnzmn ?

tyleretters commented 2 years ago

i could hit this

tyleretters commented 2 years ago

looks like this is the cause:

https://github.com/monome/norns/blob/740cbeed0fd4890f420d9fbc8691ccefbe73b5e4/lua/core/menu/select.lua#L55-L63

favorites are cached in menu init(). they're removed from the list in remove_favorite() but since scripts can be deleted external to the menu system there's no way for norns to know that the script was removed out from underneath it.

suggested fix:

  1. pull out the favorite list builder to it's own function build_favorites() or something
  2. call build_favorites() from init()
  3. whenever the script select page is viewed, call build_favorites()

this won't solve for viewing the script page and deleting a script, which i think is a reasonable compromise. the only way to solve that would be some type of polling system.

let me know if i'm on the right track.

tehn commented 2 years ago

this sounds good--- but i also think calling build_favorites() on every SELECT menu might be overkill. i think calling it once at startup would be sufficient, in case someone deleted some stuff in maiden then shut down. it seems improbable that someone would intentionally delete something in maiden and then be confused by its presence in favorites.

my main point here is to not add even more processing time to SELECT, which already has a substantial load time if you have a lot of scripts.

On Sat, Oct 9, 2021 at 12:55 PM Tyler Etters @.***> wrote:

looks like this is the cause:

https://github.com/monome/norns/blob/740cbeed0fd4890f420d9fbc8691ccefbe73b5e4/lua/core/menu/select.lua#L55-L63

favorites are cached in menu init(). they're removed from the list in remove_favorite() but since scripts can be deleted external to the menu system there's no way for norns to know that the script was removed out from underneath it.

suggested fix:

  1. pull out the favorite list builder to it's own function build_favorites() or something
  2. call build_favorites() from init()
  3. whenever the script select page is viewed, call build_favorites()

this won't solve for viewing the script page and deleting a script, which i think is a reasonable compromise. the only way to solve that would be some type of polling system.

let me know if i'm on the right track.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/monome/norns/issues/1212#issuecomment-939327156, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB4I4BIZBSINJ7RU7PSNBTUGBXWXANCNFSM4S3I2F4A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

tehn commented 2 years ago

i'm sorry, just re-read my answer and it's unclear. i'm suggesting something like validate_favorites()... which just makes sure all the stuff in the favorites list actually exists. called once, at startup

On Sat, Oct 9, 2021 at 12:55 PM Tyler Etters @.***> wrote:

looks like this is the cause:

https://github.com/monome/norns/blob/740cbeed0fd4890f420d9fbc8691ccefbe73b5e4/lua/core/menu/select.lua#L55-L63

favorites are cached in menu init(). they're removed from the list in remove_favorite() but since scripts can be deleted external to the menu system there's no way for norns to know that the script was removed out from underneath it.

suggested fix:

  1. pull out the favorite list builder to it's own function build_favorites() or something
  2. call build_favorites() from init()
  3. whenever the script select page is viewed, call build_favorites()

this won't solve for viewing the script page and deleting a script, which i think is a reasonable compromise. the only way to solve that would be some type of polling system.

let me know if i'm on the right track.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/monome/norns/issues/1212#issuecomment-939327156, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB4I4BIZBSINJ7RU7PSNBTUGBXWXANCNFSM4S3I2F4A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

tyleretters commented 2 years ago

i don't think a validate_favorites() at startup would solve the bug. my understanding is the code already works that way. it builds the table on startup and saves it to memory. there's no cache or persistence.

i believe the scenario is specifically for when, "someone would intentionally delete something in maiden and then be confused by its presence in favorites." @dndrks can you confirm?

tehn commented 2 years ago

ok just confirmed the behavior.

the list is re-generated each time we enter SELECT mode (in m.init)

my suggestion:

this means we're not existence-checking all the faves every single time we enter the SELECT menu (major waste of cpu)

tehn commented 2 years ago

lol, i wrote "major" and meant "minor"