View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0019023 | MMW 5 | Extensions framework | public | 2022-04-28 07:06 | 2022-08-31 20:20 |
Reporter | zvezdan | Assigned To | |||
Priority | urgent | Severity | feature | Reproducibility | N/A |
Status | closed | Resolution | reopened | ||
Target Version | 5.0.4 | Fixed in Version | 5.0.4 | ||
Summary | 0019023: playlistRenamed and playlistMoved events needed | ||||
Description | I 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. | ||||
Tags | No tags attached. | ||||
Fixed in build | 2650 | ||||
|
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); }); |
|
Added in 5.0.4.2650 and merged to 5.0.3.2626 |
|
Verified 2626 Tested using @Ludek example from 0019023:0068181 |
|
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. |
|
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? |
|
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) |
|
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? |
|
Verified implementation in 2627 @ludek, can you please answer and close if it will be pushed to 5.0.4? |
|
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) => { ... }) |
|
Verified 2661 Updated fix to reflect Changes for 5.0.4. eg. 0019023:0068228 and 0019023:0068204 |