SRL Archive - MSI Suggestions

procedure MSI_CloseHeader(closer: string);
begin
  MSI_Debug('Result: ' + closer);
  Dec(CurrDebugLevel);
end;

What change would this make?
Before

[ Ora ] -- Created Debug file
[ Ora ] -- Gathering report info, please wait...
[ Ora ] -- MSI_SetupPlayer
[ Ora ] ------ Scanning for randoms...
[ Ora ] ---- MSI_BestTool
[ Ora ] ------ CurrentSkill level: 39
[ Ora ] ---- MSI_BestTool: Adamant pickaxe
[ Ora ] ---- MSI_FindTool - GameTab: 26
[ Ora ] ------ Failed to find tool: Adamant pickaxe
[ Ora ] ---- MSI_FindTool: False
[ Ora ] ---- MSI_FindTool - GameTab: 26
[ Ora ] ------ Failed to find tool: Mithril pickaxe
[ Ora ] ---- MSI_FindTool: False
[ Ora ] ---- MSI_FindTool - GameTab: 26
[ Ora ] ------ Failed to find tool: Steel pickaxe
[ Ora ] ---- MSI_FindTool: False
[ Ora ] ---- MSI_FindTool - GameTab: 26
[ Ora ] ------ Found DTM: Iron pickaxe
[ Ora ] ---- MSI_FindTool: True
[ Ora ] -- MSI_SetupPlayer: True
[ Ora ] -- MSI_AutoObjects

After

[ Ora ] -- Created Debug file
[ Ora ] -- Gathering report info, please wait...
[ Ora ] -- MSI_SetupPlayer
[ Ora ] ------ Scanning for randoms...
[ Ora ] ---- MSI_BestTool
[ Ora ] ------ CurrentSkill level: 39
[ Ora ] ------ Result: Adamant pickaxe
[ Ora ] ---- MSI_FindTool - GameTab: 26
[ Ora ] ------ Failed to find tool: Adamant pickaxe
[ Ora ] ------ Result: False
[ Ora ] ---- MSI_FindTool - GameTab: 26
[ Ora ] ------ Failed to find tool: Mithril pickaxe
[ Ora ] ------ Result: False
[ Ora ] ---- MSI_FindTool - GameTab: 26
[ Ora ] ------ Failed to find tool: Steel pickaxe
[ Ora ] ------ Result: False
[ Ora ] ---- MSI_FindTool - GameTab: 26
[ Ora ] ------ Found DTM: Iron pickaxe
[ Ora ] ------ Result: True
[ Ora ] ---- Result: MSI_SetupPlayer: True
[ Ora ] -- MSI_AutoObjects

From looking at the debug atm it is visible there's still some places to add it to.

I think this way things look cleaner.
For low-level functions (functions that have no MSI subfunctions) 'Result: true' is usually enough debug info.

For high-level functions such as MSI_SetupPlayer, printing out 'Result: MSI_SetupPlayer: true' makes it easy to remember what was going on before.

I think this is an improvement. Opinions?

~RM

I liked how MSI_SetupPlayer lines up at the header and footer (for the longer functions anyway), but the change does look much cleaner for the smaller ones like MSI_FindTool.

Honestly, I like it both ways :redface:

From: NCDS
Honestly, I like it both ways :redface:

Ha.

Anyway, it wouldn't be hard to have both. And it wouldn't be hard to have it the way you were talking about, RM. I like the example you gave. :)

I think we should do this. We would have to remove the procedure name in a few CloseHeaders though, but that's shouldn't be a huge deal. What happens when there's a CloseHeader that doesn't have a result (i.e. it's a procedure, not a function)?

Make everything return a result? I rather like how libMML works with that, actually.. Everything is a function which returns RESULT_OK (0), RESULT_FALSE (1), or RESULT_ERROR (-1). And if you really need anything from the function, just out whatever. I think I'm gonna shape SPyRL like that, as well. :)

The consistency really makes things extremely easier to understand.. Rather than things possibly not returning anything, some things returning bools, floats, ints, strings.. It gets a bit confusing.

You can check out libMML if you want to see that it really does work (now that I think of it, a couple things don't return anything, like set_last_error, but that could be changed. Might talk to Wizzup? about it, but it's not a big deal), or I can write some SPyRL stuff up tonight and show you an example script or something. :)

Really I think it'd be good if I did write some SPyRL up and make an example script or two, as this would be quite the change to MSI and everything, so I'll do it anyway because it may be a good structure for SRL5/6/whatever's coming up ;p

From: i luffs yeww
Make everything return a result? I rather like how libMML works with that, actually.. Everything is a function which returns RESULT_OK (0), RESULT_FALSE (1), or RESULT_ERROR (-1). And if you really need anything from the function, just out whatever. I think I'm gonna shape SPyRL like that, as well. :)

This is the only thing about that post I followed. >.<

And I suppose everything could be made a function, but there are just some where it's completely unnecessary like a function that sets values. It's just setting some variables, it's pointless to have it a function. :unsure:

Sorry that didn't make much sense.. As Wizzup? knows, I need to sleep more. ;3

Very simply, I'll write up some examples of SPyRL (SRL Python Resource Library, as it seems you're not keeping up with some of this stuff :p) to show how it works with everything returning results.

And I felt the same way about setting values, but in libMML, you can see that it returns results for set_tolerance_speed.

function set_tolerance_speed(C: TClient; nCTS: Integer): Integer; cdecl;
begin
  if not validate_client(C) then
  begin
    exit(RESULT_ERROR);
  end;
  try
    begin
      C.MFinder.SetToleranceSpeed(nCTS);
      result := RESULT_OK;
    end;
  except on e : Exception do
    begin
      set_last_error(e.message);
      result := RESULT_ERROR;
    end;
  end;
end;

But still, I'll give some examples tomorrow hopefully.

I knew what you meant, just the last parts didn't make sense. :p

tl;dr, it's consistent, thus less confusing, and I'm gonna work on SPyRL tonight.

I think this can be done pretty easily. I will certainly make things neater. Someone just needs to update MSI_CloseHeader, and go through the MSI files to update the occurrences of MSI_CloseHeader. Most would be changed to

MSI_CloseHeader(BoolToStr(Result));

If you don't have commit rights and you feel up to this, just create a patch and we can apply it easily. :)

There's no need to change anything Coh3n ;)

procedure MSI_CloseHeader(closer: string);
begin
  MSI_Debug('Result: ' + closer);
  Dec(CurrDebugLevel);
end;

All this needs is committing. It's compatible backwards :p

~RM

From: Sir R. Magician
There's no need to change anything Coh3n ;)

procedure MSI_CloseHeader(closer: string);
begin
  MSI_Debug('Result: ' + closer);
  Dec(CurrDebugLevel);
end;

All this needs is committing. It's compatible backwards :p

~RM

Yeah I know it will work, but every function will say:

[] ---- Result: MSI_FindTool: True

Which doesn't really matter I guess, but the function name was taken out in your example.

Committed.

E: I may revert back to the older system. Although this looks neater for shorter functions, I'm finding it A LOT harder to read long debugs, especially when there's a lot of functions called in one function. It's hard to tell which result is for which. It was much easier when it was laid out like begin..end nests. If no one has any objections, I'll do this later.

E2: Reverted back, and it's 100% easier to follow. Yes, it sometimes doesn't look as nice, but that can always be changed in the actual function if needed.