What's wrong with this script?

Hi,
I think there have been some changes since I last upgraded mAirList and am having an UNKNOWN IDENTIFIER: SAVE MMD error when running this script:

[code]var
i: integer;

begin
for i := 0 to CurrentPlaylist.GetCount - 1 do
CurrentPlaylist.GetItem(i).SaveMMD;
end.[/code]

I’m trying to save all files in the current playlist to MMD. Any ideas where I’m going wrong?

Ant

Yes, glad to help! :slight_smile:

Firstly, I’d recommend using a copy of the CurrentPlaylist, like so:

var plCurrent: IPlaylist; … begin … plCurrent := Factory.CreatePlaylist; plCurrent.Assign(CurrentPlaylist); … end.
(this is Torben-recommended-and-approved ‘best practice.’ ;))

So from here on, you’re working with plCurrent where you previously used CurrentPlaylist:

var i: integer; plCurrent: IPlaylist; currentItem: IPlaylistItem; … begin … plCurrent := Factory.CreatePlaylist; plCurrent.Assign(CurrentPlaylist); for i := 0 to plCurrent.GetCount - 1 do begin currentItem := plCurrent.GetItem(i); … end; … end.
And to finally get around to answering your original question :D, alphadevil: yes, things have changed. SaveMMD has been ‘promoted’ to become a method of IFilePlaylistItem, so the (again, Dr.W-approved) way—wrapped in a try/except/end block—is this:

const SCRIPTNAME = 'WritePlaylistMMDs script: '; … var i: integer; plCurrent: IPlaylist; currentItem: IPlaylistItem; … begin … plCurrent := Factory.CreatePlaylist; plCurrent.Assign(CurrentPlaylist); for i := 0 to plCurrent.GetCount - 1 do begin currentItem := plCurrent.GetItem(i); try IFilePlaylistItem(currentItem).SaveMMD; except SystemLog(SCRIPTNAME + 'ERROR trying to write item ' + IntToStr(i + 1) + ' to MMD file.'); end; end; end.
Note that I added a bit of error logging there. Sorry about that, but it’s helpful to know. It’s also the only way to give any kind of feedback whatever to a user from your script. >shrug< Live with it. :smiley: Oh, and a const for the script name: handy for error logging, so you only change it once if you rename the .mls file. :wink:

‘BUT we don’t want to give you that!’ as Chris Tarrant, host of the UK version of ‘Millionaire’ would say. It’s always a good idea when writing a script to ‘do something’ to every item in a playlist to first make sure that a) the playlist isn’t empty, and b) the playlist is NOT in AUTO mode (trust me: it prevents accidents! :D). So I would usually do that first, make what we’ve written so far into a procedure, and only run the procedure if a) and b) above check out.

Also, using GetCount in the for statement means it gets checked and one subtracted from it every time around the loop. Much better to calculate this ONCE, before and outside the loop, and set an int variable to that number to use in the for statement.

And it would also help the user to know whether everything worked properly, so let’s tally any errors and also give the user that count in a useful info. message when we shut up shop.

So the finished item would be something like this:

[code]// Write Playlist MMDs
// This script saves the Properties of all Playlist items as MMD filesPlaylist

// Written by alphdevil
// with some trivial assistance from Cad Delworth, Leith FM, Edinburgh, Scotland

const
// Name of script, used as a prefix to all SystemLog messages (default: 'WritePlaylistMMDs script: ')
// If you change this, keep a colon and space as the last two characters.
SCRIPTNAME = 'WritePlaylistMMDs script: ';
var
i, iErrorCount, iItemCount, iMax: integer;
sPlaylist: string;
plCurrent: IPlaylist;
currentItem: IPlaylistItem;
procedure ProcessCurrentPlaylist;
begin
plCurrent := Factory.CreatePlaylist;
plCurrent.Assign(CurrentPlaylist);
iErrorCount := 0;
// Remember iItemCount in the startup code down below?
// We re-use it here to set the loop limit
iMax := iItemCount - 1;
for i := 0 to iMax do
begin
currentItem := plCurrent.GetItem(i);
try
IFilePlaylistItem(currentItem).SaveMMD;
except
iErrorCount := iErrorCount + 1;
SystemLog(SCRIPTNAME
+ ‘ERROR trying to write item ’
+ IntToStr(i + 1)
+ ’ to MMD file.’);
end;
end;
end;
begin
// This is the playlist number, in case we need it for a message
sPlaylist := IntToStr(CurrentPlaybackControl.GetIndex + 1);
iItemCount := CurrentPlaylist.GetCount;
// Check that the playlist contains items
if iItemCount < 1 then
SystemLog(SCRIPTNAME
+ ‘Please put one or more items in Playlist ’
+ sPlaylist
+ ‘, then try again.’)
// Check that we are in ASSIST mode
else if CurrentPlaybackControl.GetAutomation = true then
SystemLog(SCRIPTNAME
+ ‘Please put Playlist ’
+ sPlaylist
+ ’ into ASSIST mode, then try again.’)
else
begin
ProcessCurrentPlaylist;
SystemLog(SCRIPTNAME
+ ‘Processing complete. ’
+ IntToStr(iItemCount - iErrorCount)
+ ’ items processed successfully (’
+ IntToStr(iErrorCount)
+ ’ items had errors).’);
end;
end.[/code]

Hope that helps. 8)

BFN
CAD

When you work on a copy of the playlist, you can safely ignore these conditions.

It is true that the playlist might change while the script is runnning. This is because mAirList is “multi-threaded”, just like most modern applications. For the example, the automation might start and stop players and remove items from the top of the playlist as it does so. Or the user can click the “Delete” button while your script is executing in the background. Your “for” loop will get into trouble then.

If you’re sure that your operation is quick, the easiest way is to lock the playlist using BeginRead…EndRead while you access it, and use try…finally to make sure that EndRead is executed even if your code causes an error (exception):

CurrentPlaylist.BeginRead;
try
  (your code goes here)
finally
  CurrentPlaylist.EndRead;
end;

The BeginRead statement makes sure that no other thread is accessing the playlist as you enter your code block, and will keep any other thread from doing so until EndRead is called. Note: If you want to make changes to the playlist, e.g. add or remove items, use BeginUpdate…EndUpdate instead.

One thing to keep in mind is that locking the playlist may affect other parts of the application. For example, the GUI may freeze (while it’s waiting to redraw the playlist), and the automation cannot start or stop any players. So if your operation is somewhat lengthy (e.g. writing 500 file tags), or you’re uncomfortable with the BeginRead…EndRead stuff, it might be favorable to work on a copy of the playlist instead, as Cad pointed out above.

This copy is local to your script, so it cannot be accessed by any other threads, and it is not necessary to lock it. It is basically a snapshot of your playlist made at the time the “Assign” procedure is called. If the playlist changes while your script runs, these changes won’t be reflected in your copy - but, so what.

Also, using GetCount in the for statement means it gets checked and one subtracted from it every time around the loop. Much better to calculate this ONCE, before and outside the loop, and set an int variable to that number to use in the for statement.

That’s not true for Delphi - even when used in the for statement, GetCount is called exactly once, before the loop. And if the playlist shrinks by one item during the loop, you will get a “List index out of bounds” error for the last item, because its index (GetCount-1) doesn’t exist anymore.

That’s different from other languages, e.g. C++ or Java, where the end condition is checked on each iteration. But for loops in C++ and Java are much different from those in Delphi. In Delphi, you can only specify a start and stop value. In C++, you can enter arbitrary expressions as the break condition.

Oh, and considering my comments above, I would say that the first script you proposed is perfectly ok.

Cad/Torben
Great! Many thanks - having read the details and digested them it makes sense now. Thanks for that.

Onwards & upwards! :slight_smile:

A

I’m having a problem with the last one, Cad - 'Error executing action: [Error] (17:3): ‘BEGIN’ expected.

I think I know what this means but can’t for the life of me work out where the problem lies!

There’s a “begin” missing at the beginning of the ProcessCurrentPlaylist procedure. I had noticed that before, but I didn’t want to be fussy :wink:

:-[ Now fixed!

BFN
CAD

I did think that might be the case so I tried it and got ‘Error executing action: [Error] (23:8): Assignment expected’ - but I thought it was just me!

Same error on the latest script Cad.

Though its useful as a learning curve for me!! ;D

[quote=“alphadevil, post:9, topic:6959”]I did think that might be the case so I tried it and got ‘Error executing action: [Error] (23:8): Assignment expected’ - but I thought it was just me!

Same error on the latest script Cad.

Though its useful as a learning curve for me!! ;D[/quote]

You need to change
iMax = iItemCount - 1;
to
iMax := iItemCount - 1;

I always forget that Pascal syntax for assignments is NOT just a = sign! ::slight_smile:

PS: This is now fixed in the code in my post above.

BFN
CAD