[Home] [Downloads] [Search] [Help/forum]

Register forum user name Search FAQ

Gammon Forum

[Folder]  Entire forum
-> [Folder]  MUSHclient
. -> [Folder]  Development
. . -> [Subject]  Improvements to plugin callbacks
Improvements to plugin callbacks

It is now over 60 days since the last post. This thread is closed.     [Refresh] Refresh page

Pages: 1 2  3  

Posted by Nick Gammon   Australia  (22,975 posts)  [Biography] bio   Forum Administrator
Date Fri 17 Sep 2010 06:10 AM (UTC)
Message
After all this talk of refactoring I have done a bit myself.

I have made some fairly extensive changes to the way plugin callbacks are called.

See: http://github.com/nickgammon/mushclient/commit/54797dc5f74

The main changes are:


  • Removed the list of DISPIDs from the CPlugin class, and replaced with a simple map of callback names to DISPID, like this:

    
map<const char *, DISPID> m_PluginCallbacks;


    Now rather than having dozens of variables, you can simply get a DISPID (dispatch ID) by something like this:

    
foo = m_PluginCallbacks ["OnPluginChatAccept"];


  • This has allowed the simplification of quite a bit of code, and in many places rather ugly loops through each plugin are now replaced with simpler stuff like:

    
SendToAllPluginCallbacks (ON_PLUGIN_TICK);




Those of you who are doing private builds might like to pull this stuff in and test it. It seems to be OK, but the changes are so extensive, well, who knows?

There is scope for a bit more improvement yet, but I think that this alone makes the code look tidier and somewhat less obscure.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by Twisol   USA  (2,257 posts)  [Biography] bio
Date Reply #1 on Fri 17 Sep 2010 06:38 AM (UTC)
Message
...

I love you.

(Platonically of course.)
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
[Go to top] top
Posted by Worstje   Netherlands  (899 posts)  [Biography] bio
Date Reply #2 on Fri 17 Sep 2010 06:39 AM (UTC)
Message
I'll check it out later. It definitely sounds promising. :)
[Go to top] top
Posted by Twisol   USA  (2,257 posts)  [Biography] bio
Date Reply #3 on Fri 17 Sep 2010 06:46 AM (UTC)

Amended on Fri 17 Sep 2010 06:54 AM (UTC) by Twisol

Message
Nick, could mushclient.clw be added to the .gitignore? From a cursory glance, it seems to be used simply to keep track of the last class edited with the class wizard? Like VS2005's *.vcproj.<user> files (which I have in .gitignore), it seems like something specific to one developer, not the whole project.

[EDIT]: Just finished looking at the diff, it looks a lot better! The m_CurrentPlugin saving has been clustered around only a handful of methods, which is very nice. I'm not able to compile it at the moment because I'm working in Linux currently, but I will definitely pull it down.

I do have a small nit (and it's rather unrelated to the changeset at large). 'PluginCallbacksTable' is defined in xml_load_world.cpp, but to my mind it would make more sense if it were defined in plugins.cpp.
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
[Go to top] top
Posted by Nick Gammon   Australia  (22,975 posts)  [Biography] bio   Forum Administrator
Date Reply #4 on Fri 17 Sep 2010 10:13 PM (UTC)
Message
Another feature I added today was callback counts. Since the callbacks are more centralised, this is a bit easier to do.

In the Debug "summary" list you can now click on a [Cb] hyperlink to see counts, like this:


------ Callback List (alphabetic order) ------

OnPluginClose                  -> Valid: yes.  Called 0 times.
OnPluginDisable                -> Valid: yes.  Called 1 time.
OnPluginEnable                 -> Valid: yes.  Called 0 times.
OnPluginTrace                  -> Valid: yes.  Called 41 times.
4 callbacks.


This could be useful for checking if your callbacks are being called as often (or perhaps more often) as you expect.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by Twisol   USA  (2,257 posts)  [Biography] bio
Date Reply #5 on Fri 17 Sep 2010 10:18 PM (UTC)
Message
Nifty. Since you have a separate dispid class now, and it seems to be keeping track of its own invocation count, what do you think of using it for the trigger/alias/timers too? (Or anywhere else that some nInvocationCount thing is used.)
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
[Go to top] top
Posted by Nick Gammon   Australia  (22,975 posts)  [Biography] bio   Forum Administrator
Date Reply #6 on Fri 17 Sep 2010 11:33 PM (UTC)
Message
Yes, conceivably. However that is more major changes, so for the moment I'll just test plugins still work. One thing that slipped through, for example, was that hotspots stopped working.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by Nick Gammon   Australia  (22,975 posts)  [Biography] bio   Forum Administrator
Date Reply #7 on Sat 18 Sep 2010 03:08 AM (UTC)

Amended on Sat 18 Sep 2010 03:11 AM (UTC) by Nick Gammon

Message
Here is my test bed plugin.


<?xml version="1.0" encoding="iso-8859-1"?>
<!DOCTYPE muclient>

<muclient>
<plugin
   name="plugin1"
   author="Nick Gammon"
   id="91b90c3974e1da6daf1e3b9e"
   language="Lua"
   purpose="Tests script callbacks"
   date_written="2010-09-18 10:49:15"
   requires="4.61"
   version="1.0"
   >

</plugin>


<!--  Script  -->


<script>
<![CDATA[

-- Tests all plugin callbacks (for version 4.62 testing)
-- Author: Nick Gammon
-- Date:   18 Sep 2010

-- what to return
retval = true

function showinfo (name, args)
  CallPlugin ("126d9061f9758498c878a204", "MsgNote", "Plugin1 got: " .. name)
  for k, v in ipairs (args) do
    CallPlugin ("126d9061f9758498c878a204", "MsgNote", " arg " .. k .. " = " .. v)
  end -- for
  CallPlugin ("126d9061f9758498c878a204", "MsgNote", "")
end -- showinfo

function OnPluginBroadcast (...)
  showinfo ("OnPluginBroadcast", {...})
  return retval
end  -- OnPluginBroadcast



function OnPluginChatAccept (...)
  showinfo ("OnPluginChatAccept", {...})
  return retval
end  -- OnPluginChatAccept



function OnPluginChatDisplay (...)
  showinfo ("OnPluginChatDisplay", {...})
  return retval
end  -- OnPluginChatDisplay



function OnPluginChatMessage (...)
  showinfo ("OnPluginChatMessage", {...})
  return retval
end  -- OnPluginChatMessage



function OnPluginChatMessageOut (...)
  showinfo ("OnPluginChatMessageOut", {...})
  return retval
end  -- OnPluginChatMessageOut



function OnPluginChatNewUser (...)
  showinfo ("OnPluginChatNewUser", {...})
  return retval
end  -- OnPluginChatNewUser



function OnPluginChatUserDisconnect (...)
  showinfo ("OnPluginChatUserDisconnect", {...})
  return retval
end  -- OnPluginChatUserDisconnect



function OnPluginClose (...)
  showinfo ("OnPluginClose", {...})
  return retval
end  -- OnPluginClose



function OnPluginCommand (...)
  showinfo ("OnPluginCommand", {...})
  return true
end  -- OnPluginCommand



function OnPluginCommandChanged (...)
  showinfo ("OnPluginCommandChanged", {...})
  return retval
end  -- OnPluginCommandChanged



function OnPluginCommandEntered (...)
  showinfo ("OnPluginCommandEntered", {...})
  return retval
end  -- OnPluginCommandEntered



function OnPluginConnect (...)
  showinfo ("OnPluginConnect", {...})
  return retval
end  -- OnPluginConnect



function OnPluginDisable (...)
  showinfo ("OnPluginDisable", {...})
  return retval
end  -- OnPluginDisable



function OnPluginDisconnect (...)
  showinfo ("OnPluginDisconnect", {...})
  return retval
end  -- OnPluginDisconnect



function OnPluginEnable (...)
  showinfo ("OnPluginEnable", {...})
  return retval
end  -- OnPluginEnable



function OnPluginGetFocus (...)
  showinfo ("OnPluginGetFocus", {...})
  return retval
end  -- OnPluginGetFocus



function OnPlugin_IAC_GA (...)
  showinfo ("OnPlugin_IAC_GA", {...})
  return retval
end  -- OnPlugin_IAC_GA



function OnPluginInstall (...)
  showinfo ("OnPluginInstall", {...})
  return retval
end  -- OnPluginInstall



function OnPluginLineReceived (...)
  showinfo ("OnPluginLineReceived", {...})
  return retval
end  -- OnPluginLineReceived



function OnPluginListChanged (...)
  showinfo ("OnPluginListChanged", {...})
  return retval
end  -- OnPluginListChanged



function OnPluginLoseFocus (...)
  showinfo ("OnPluginLoseFocus", {...})
  return retval
end  -- OnPluginLoseFocus



function OnPluginMouseMoved (...)
  showinfo ("OnPluginMouseMoved", {...})
  return retval
end  -- OnPluginMouseMoved



function OnPluginMXPcloseTag (...)
  showinfo ("OnPluginMXPcloseTag", {...})
  return retval
end  -- OnPluginMXPcloseTag



function OnPluginMXPerror (...)
  showinfo ("OnPluginMXPerror", {...})
  return retval
end  -- OnPluginMXPerror



function OnPluginMXPopenTag (...)
  showinfo ("OnPluginMXPopenTag", {...})
  return retval
end  -- OnPluginMXPopenTag



function OnPluginMXPsetEntity (...)
  showinfo ("OnPluginMXPsetEntity", {...})
  return retval
end  -- OnPluginMXPsetEntity



function OnPluginMXPsetVariable (...)
  showinfo ("OnPluginMXPsetVariable", {...})
  return retval
end  -- OnPluginMXPsetVariable



function OnPluginMXPstart (...)
  showinfo ("OnPluginMXPstart", {...})
  return retval
end  -- OnPluginMXPstart



function OnPluginMXPstop (...)
  showinfo ("OnPluginMXPstop", {...})
  return retval
end  -- OnPluginMXPstop



function OnPluginPacketReceived (...)
  showinfo ("OnPluginPacketReceived", {...})
  return retval
end  -- OnPluginPacketReceived



function OnPluginPartialLine (...)
  showinfo ("OnPluginPartialLine", {...})
  return retval
end  -- OnPluginPartialLine



function OnPluginPlaySound (...)
  showinfo ("OnPluginPlaySound", {...})
  return retval
end  -- OnPluginPlaySound



function OnPluginSaveState (...)
  showinfo ("OnPluginSaveState", {...})
  return retval
end  -- OnPluginSaveState



function OnPluginScreendraw (...)
  showinfo ("OnPluginScreendraw", {...})
  return retval
end  -- OnPluginScreendraw



function OnPluginSend (...)
  showinfo ("OnPluginSend", {...})
  return retval
end  -- OnPluginSend



function OnPluginSent (...)
  showinfo ("OnPluginSent", {...})
  return retval
end  -- OnPluginSent



function OnPluginTabComplete (...)
  showinfo ("OnPluginTabComplete", {...})
  return retval
end  -- OnPluginTabComplete



function OnPluginTelnetOption (...)
  showinfo ("OnPluginTelnetOption", {...})
  return retval
end  -- OnPluginTelnetOption



function OnPluginTelnetRequest (...)
  showinfo ("OnPluginTelnetRequest", {...})
  return retval
end  -- OnPluginTelnetRequest



function OnPluginTelnetSubnegotiation (...)
  showinfo ("OnPluginTelnetSubnegotiation", {...})
  return retval
end  -- OnPluginTelnetSubnegotiation



function OnPluginTick (...)
  showinfo ("OnPluginTick", {...})
  return retval
end  -- OnPluginTick



function OnPluginTrace (...)
  showinfo ("OnPluginTrace", {...})
  return retval
end  -- OnPluginTrace



function OnPluginWorldOutputResized (...)
  showinfo ("OnPluginWorldOutputResized", {...})
  return retval
end  -- OnPluginWorldOutputResized



function OnPluginWorldSave (...)
  showinfo ("OnPluginWorldSave", {...})
  return retval
end  -- OnPluginWorldSave
]]>
</script>


</muclient>


You'll see if you install it that you soon want to comment out some lines.

It requires the Messages_Window as described on this page:

http://www.gammon.com.au/forum/bbshowpost.php?id=10515

The basic idea is that it is a plugin will all possible callbacks in it. It shows when callbacks are called and what arguments get passed to them. You can also edit to control the return value (for example, if you return false or nil from some callbacks they suppress input and output).

I made a second one, almost identical copy, to see what happens with two plugins. Apart from changing the plugin name and plugin ID, the showinfo function was changed to:


function showinfo (name, args)
  CallPlugin ("126d9061f9758498c878a204", "MsgNote", "Plugin2 got: " .. name, "yellow")
  for k, v in ipairs (args) do
    CallPlugin ("126d9061f9758498c878a204", "MsgNote", " arg " .. k .. " = " .. v, "yellow")
  end -- for
  CallPlugin ("126d9061f9758498c878a204", "MsgNote", "")
end -- showinfo


Basically it has the different plugin name, and the messages in a different colour.

Example output:

- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by Nick Gammon   Australia  (22,975 posts)  [Biography] bio   Forum Administrator
Date Reply #8 on Sat 18 Sep 2010 03:15 AM (UTC)
Message
And example callbacks list with that plugin installed:


------ Callback List (alphabetic order) ------

OnPluginBroadcast              -> Valid: yes           0 calls.
OnPluginChatAccept             -> Valid: yes           0 calls.
OnPluginChatDisplay            -> Valid: yes           0 calls.
OnPluginChatMessage            -> Valid: yes           0 calls.
OnPluginChatMessageOut         -> Valid: yes           0 calls.
OnPluginChatNewUser            -> Valid: yes           0 calls.
OnPluginChatUserDisconnect     -> Valid: yes           0 calls.
OnPluginClose                  -> Valid: yes           0 calls.
OnPluginCommand                -> Valid: yes           4 calls.
OnPluginCommandChanged         -> Valid: yes          24 calls.
OnPluginCommandEntered         -> Valid: yes           4 calls.
OnPluginConnect                -> Valid: yes           0 calls.
OnPluginDisable                -> Valid: yes           1 call.
OnPluginDisconnect             -> Valid: yes           0 calls.
OnPluginEnable                 -> Valid: yes           0 calls.
OnPluginGetFocus               -> Valid: yes           2 calls.
OnPluginInstall                -> Valid: yes           1 call.
OnPluginLineReceived           -> Valid: yes          19 calls.
OnPluginListChanged            -> Valid: yes           1 call.
OnPluginLoseFocus              -> Valid: yes           0 calls.
OnPluginMXPcloseTag            -> Valid: yes           0 calls.
OnPluginMXPerror               -> Valid: yes           0 calls.
OnPluginMXPopenTag             -> Valid: yes           0 calls.
OnPluginMXPsetEntity           -> Valid: yes           0 calls.
OnPluginMXPsetVariable         -> Valid: yes           0 calls.
OnPluginMXPstart               -> Valid: yes           0 calls.
OnPluginMXPstop                -> Valid: yes           0 calls.
OnPluginMouseMoved             -> Valid: yes         864 calls.
OnPluginPacketReceived         -> Valid: yes           2 calls.
OnPluginPartialLine            -> Valid: yes         280 calls.
OnPluginPlaySound              -> Valid: yes           0 calls.
OnPluginSaveState              -> Valid: yes           0 calls.
OnPluginScreendraw             -> Valid: yes         277 calls.
OnPluginSend                   -> Valid: yes           2 calls.
OnPluginSent                   -> Valid: yes           2 calls.
OnPluginTabComplete            -> Valid: yes           0 calls.
OnPluginTelnetOption           -> Valid: yes           0 calls.
OnPluginTelnetRequest          -> Valid: yes           0 calls.
OnPluginTelnetSubnegotiation   -> Valid: yes           0 calls.
OnPluginTick                   -> Valid: yes        5017 calls.
OnPluginTrace                  -> Valid: yes           0 calls.
OnPluginWorldOutputResized     -> Valid: yes          29 calls.
OnPluginWorldSave              -> Valid: yes           1 call.
OnPlugin_IAC_GA                -> Valid: yes           0 calls.
44 callbacks.

- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by Nick Gammon   Australia  (22,975 posts)  [Biography] bio   Forum Administrator
Date Reply #9 on Sat 18 Sep 2010 05:15 AM (UTC)
Message
Worstje: Looking through some of your patches, why do this?


//----------------------------------------
// world.GetReceivedBytes
//----------------------------------------
static int L_GetReceivedBytes (lua_State *L)
  {
  lua_pushnumber (L, (lua_Number) (doc (L)->m_nBytesIn) );
  return 1; // number of result fields
  } // end of L_GetReceivedBytes


You are casting it to the type it already is:


LUA_API void  (lua_pushnumber) (lua_State *L, lua_Number n);


So that achieves nothing, apart from maybe documentation. But then you may as well cast everything in that case.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by Twisol   USA  (2,257 posts)  [Biography] bio
Date Reply #10 on Sat 18 Sep 2010 05:24 AM (UTC)
Message
I'm not sure I understand your logic, Nick. CMUSHclientDoc::m_nBytesIn is an __int64. lua_Number is (by default) a double. The cast is required, because you're casting an integer to a floating-point number.
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
[Go to top] top
Posted by Nick Gammon   Australia  (22,975 posts)  [Biography] bio   Forum Administrator
Date Reply #11 on Sat 18 Sep 2010 05:32 AM (UTC)
Message
Oh I see. This is another "get rid of warnings" thing. My point was, the destination is already a double.

eg.


double f;

f = 42;


You don't need to do:


f = (double) 42;


I just don't agree with littering the code with casts. In the case in point you are moving a __int64 to a double. There is going to be loss of precision potentially. So if you have warnings cranked up you get a warning. Now you can put in a cast to "artificially" get rid of the warning, or you can just suppress the warning. Either way you are saying "I know about this, don't warn me".
- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by Worstje   Netherlands  (899 posts)  [Biography] bio
Date Reply #12 on Sat 18 Sep 2010 05:34 AM (UTC)

Amended on Sat 18 Sep 2010 05:38 AM (UTC) by Worstje

Message
What Twisol said.

The variable is an __int64, lua_Number (currently) is typedef'd as a double, which means it is a lossy conversion since a double cannot represent every value in an int64.

It's no different than all the others casts I felt are needed, and you feel are useless for what is ironically the same reason: preventing bugs.

If some typecast can lose data, I prefer to explicitly cast it such so I know it was intentional if ever a bug DOES crop up. If you don't add it, and you lose precision somewhere along the path... good luck finding it. :)

Edit: The difference between the two approaches is that in the warning suppression case, you lose the notification you are losing precision when you are writing new code, making it easy to miss or consciously think about. With warnings enabled, and having added the typecast, you are sure to have consciously thought about the loss in precision. With warnings suppressed, it might never cross your mind. 'It wants a number, you give it a number, right?'
[Go to top] top
Posted by Twisol   USA  (2,257 posts)  [Biography] bio
Date Reply #13 on Sat 18 Sep 2010 05:35 AM (UTC)

Amended on Sat 18 Sep 2010 05:38 AM (UTC) by Twisol

Message
It's true that you can let the compiler do it for you. That's what it does whether you cast it or not, anyways. The issue here is that disabling that warning altogether is heavy-handed; you could miss other, more important typecast warning elsewhere in the code. And if you localize the #pragma to just that one place - IIRC, #pragma pop? - you might as well just use a cast.

EDIT: There was also that ugly crash I found back when I first started with the source, stemming from a change in return value in a newer MFC version. That was pretty ugly. Typecasts at least ensure that the type going in remains the same. It may not be a big deal here, but it really is good practice.
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
[Go to top] top
Posted by Worstje   Netherlands  (899 posts)  [Biography] bio
Date Reply #14 on Sat 18 Sep 2010 05:42 AM (UTC)
Message
One final thing I thought needs saying... I don't think warning suppression is per definition bad. I can well see it as having a good reason to exist in for example lua_methods.cpp, since it is all gluecode that has the warnings for one and the same reason which is that lua_Number is a one-format-fits-all kind of deal.

But the issue also surfaces at a lot of other places in the code which do not have such a global reason. The NAWS negotiation is one such place I added a cast. If that one ever ends up being buggy (with increasing screen resolutions and all), at least you look at it and immediately have your attention drawn to the cast, whereas it would otherwise be very likely you stare down the algorithm for a while trying to figure out the problem.
[Go to top] top

The dates and times for posts above are shown in Universal Co-ordinated Time (UTC).

To show them in your local time you can join the forum, and then set the 'time correction' field in your profile to the number of hours difference between your location and UTC time.

94,745 views.

This is page 1, subject is 3 pages long: 1 2  3  [Next page]

It is now over 60 days since the last post. This thread is closed.     [Refresh] Refresh page

Go to topic:           Search the forum

[Go to top] top

Quick links: MUSHclient. MUSHclient help. Forum shortcuts. Posting templates. Lua modules. Lua documentation.

Information and images on this site are licensed under the Creative Commons Attribution 3.0 Australia License unless stated otherwise.

[Home]

Written by Nick Gammon - 5K   profile for Nick Gammon on Stack Exchange, a network of free, community-driven Q&A sites   Marriage equality

Comments to: Gammon Software support
[RH click to get RSS URL] Forum RSS feed ( https://gammon.com.au/rss/forum.xml )

[Best viewed with any browser - 2K]    [Hosted at HostDash]