View Issue Details

IDProjectCategoryView StatusLast Update
0019023MMW 5Extensions frameworkpublic2022-08-31 20:20
Reporterzvezdan Assigned To 
PriorityurgentSeverityfeatureReproducibilityN/A
Status closedResolutionreopened 
Target Version5.0.4Fixed in Version5.0.4 
Summary0019023: playlistRenamed and playlistMoved events needed
DescriptionI know I am talking to the wall here, since you didn't even assigned anyone to my request for the same thing back in 2013 for MM3 (https://www.ventismedia.com/mantis/view.php?id=11094), but here it is anyway.

I have several add-ons that require these events. I used some dirty tricks to overcome this in MM4, but they are not applicable in MM5 anymore.

Edit: If you ever decide to add these events, it would be nice if their handlers have two arguments. The first one is the moved/renamed playlist itself. The second argument would be:
- for playlistMoved - the old (previous) parent playlist from which it was moved out;
- for playlistRenamed - the old title.
TagsNo tags attached.
Fixed in build2650

Relationships

related to 0019230 closedLudek app.listen(app.playlists.root, 'playlistChanged' and 'playlistAdded' should have a playlist object as argument 
related to 0019229 closedLudek app.listen(app.playlists.root, 'playlistChanged' ... doesn't fire when track is removed from playlist 

Activities

Ludek

2022-05-19 22:44

developer   ~0068181

Last edited: 2022-05-19 23:15

Implemented by expanding the existing 'playlistchange' event, can be used like this:

  app.listen(app, 'playlistchange', (playlist, changeType, newValue, oldValue) => {
          if (changeType == 'title')
              uitools.toastMessage.show(playlist.title + ' renamed from ' + oldValue + ' to ' + newValue);
          else
          if (changeType == 'new_child')
              uitools.toastMessage.show(playlist.title + ' has new child ' + newValue.title);
          else
          if (changeType == 'child_removed')
              uitools.toastMessage.show(playlist.title + ' -- child was removed ' + newValue.title);
        });

Ludek

2022-05-19 23:15

developer   ~0068182

Added in 5.0.4.2650 and merged to 5.0.3.2626

peke

2022-05-20 19:30

developer   ~0068196

Verified 2626

Tested using @Ludek example from 0019023:0068181

zvezdan

2022-05-21 00:29

updater   ~0068199

Thanks! I cannot test it until you release that build, but I have some observations about your implementation.

You have interesting approach from the point of the parent playlists, but I think that it would be more practical from the point of the child playlists. If my scripts want to know about a child playlist being moved, they need to wait for both 'new_child and 'child_removed' changeTypes to find out which are the old and new parents of moved playlist. It would be more simpler for scripts if you implemented just one changeType with the first argument being a child playlist (whose parent property is a new parent) and just one additional argument being the old parent.

Your implementation is also confusing in regards to the terminology. When is the 'playlistchange' event exactly fired with 'new_child' and 'child_removed' changeTypes? Is it only on drag&drop and cut/paste or is it also on adding/removing playlists? It will be very complicated for scripts if it is fired even when user or another script adds/removes playlists. However, technically speaking, it _should_ fire even in such cases since the names of these changeTypes suggest such thing.

I think instead of 'new_child' and 'child_removed' changeTypes it would be much better if you implemented just one e.g. 'parent' changeType as I suggested. In that case you would also avoid confusion when such event should fire, since 'parent' playlist cannot be 'changed' when playlists are added or removed, but only when they are moved.

Also, changeType == 'title' doesn't need the newValue argument since it is contained as a property of the playlist argument.

zvezdan

2022-05-22 12:55

updater   ~0068200

I just tested it and it behaves as I suspected. The event is fired for a parent playlist both when a child playlist is added/removed and when it is moved, which means that it will be difficult to differentiate what really caused that change.

The question of terminology also remains. You see, a parent node could have 'added' or 'removed' a child playlist, but it cannot have 'changed' a child playlist. However, a child playlist could have 'changed' parent playlist, as this event name implies.

Could you please add one new changeType called 'parent' with the first argument being a child playlist that is moved and the third argument (after changeType) being the old parent playlist, as I initially requested?

Ludek

2022-05-23 14:47

developer   ~0068204

OK, added the 'parent' changeType, to be used like this:

app.listen(app, 'playlistchange', (playlist, changeType, newValue, oldValue) => {
     if (changeType == 'parent')
           uitools.toastMessage.show(playlist.title + ' parent changed from ' + oldValue.title + ' to ' + newValue.title);
});

=> added in 5.0.4.2650 and merged to 5.0.3 SVN branch (in case there is going to be another 5.0.3 build)

zvezdan

2022-05-23 15:08

updater   ~0068207

Thank you very much! I am waiting for that build to test it.

I saw that changing order of tracks fires with 'tracklist' chageType, which is nice. But I also noticed that the event is fired on removing a playlist, with the first argument having the removed playlist and the changeType being undefined. Is that supposed to happen?

peke

2022-05-24 22:30

developer   ~0068226

Verified implementation in 2627

@ludek, can you please answer and close if it will be pushed to 5.0.4?

Ludek

2022-05-25 11:30

developer   ~0068228

Re the changeType for deleted playlist.
Currently for any SharedObservable descendant there is deleted property, see: https://www.mediamonkey.com/docs/api/classes/SharedObservable.html#property_deleted
So if 'change' is fired on any object then the deleted property gives the indication whether it was deleted.
But you are true that it would be better to have it also as the param in the 'change' event handler.

So for 5.0.4.2650 added the 'deleted' changeType param as suggested above + added it also to any SharedObservable descendant. To be used like listen(object, 'change', (changeType) => { ... })

peke

2022-08-31 20:20

developer   ~0069132

Verified 2661

Updated fix to reflect Changes for 5.0.4. eg. 0019023:0068228 and 0019023:0068204