Hi James. I took a look at the code and added your repo but didn't get to install the plugin - it looks like IntegrationService.HandleConsoleInstallPlugin is always existing if cmd.Length != 2, but the first two strings are always going to be "install" and "plugin" so nothing will ever happen. I'll leave you to fix that :). But I liked the bits I saw up until that point.

Here's a grab-bag of comments from a first pass.  Sorry, there are a lot of 
them.

* How mature is the Mono.Addins package, is it going to hold up for us? I see it's getting some love (just hit v1.0 [1]) but I find the need to insert a Thread.Sleep(300) in LoadRegionsPlugin() very fragile - I think we would need to find out what is really going wrong here.

* On IntegrationService, I would far rather see the "web" methods return the mono addin classes directly that the IntegrationServerHandler would then be response for serializing, rather than returning byte[] directly. Then these methods could lose their "web" monikers and potentially be much more easily be used programatically in other places. It would also be more in keeping with the structure of the other handlers and services, I believe.

* The same goes for handling console arguments. At the moment, these are fed directly into PluginManager which would make it harder to call that programatically. Perhaps another class is warranted explicitly for registering and handling console input.

* Can the console commands give more feedback in both success and failure cases? There was no feedback for me on a successful repo add or repo enable.

* How would this structure play with the existing interfaces? How hard would it be to ship 'core' plugins? Would this need an existing local repo?

* What stability benefit does this approach confer? The existing OpenSimulator service interfaces change extremely rarely, if at all. Also, how much can this approach protect against internal database or code changes?

* Could you add some method/class doc to IIntegrationService?

* Could the defaults paths for PluginRegistryLocation and IntegrationConfig be inside the OpenSimulator tree rather than outside (but not just bin/ for god's sake! Maybe we could even avoid bin/ altogether and put it in opensim/etc)? Having them be full paths is going to require everybody to reconfigure to use them.

* Could you rename IUtils to IntegrationUtils or similar?  IUtils makes it look 
like an interface.

* Some methods have a space between the name and the args (e.g. 
HandleWebDisableRepository (OSDMap request)).

[1] https://github.com/mono/mono-addins
On 11/07/12 02:41, James Hughes wrote:
Thanks Justin, sure, I'll comb through it and change those. Also, when you look 
at it, please look at the command names
and comment about those. They are a little scattered and maybe need to use "show" as 
opposed to "list" etc.

Thanks for the feedback!

BlueWall


On 07/10/2012 08:02 PM, Justin Clark-Casey wrote:
Hi BlueWall. Interesting stuff! I will certainly take a look though I
might not get sufficient spare time until Friday.

One small point, I see various methods are taking a UUID s_sessionID
argument. Could we change this to secureSessionID? The code standard in
OpenSimulator is not to have underscores in variable names (as per the
MSDN C# coding conventions).

On 10/07/12 21:16, James Hughes wrote:
I have been working on some things, for a while now, revolving around
the idea of making external applications
integration easier and helping provide a place to buffer between the
changes that can happen in core and external
applications that depend on stability. The proposal can be found at:
http://opensimulator.org/wiki/IntegrationService
with additional material at http://bluewallvirtual.com/node/46 . Code
is in the OpenSim repository under the integration
branch or may be seen at
https://github.com/BlueWall/opensim/tree/integration.

The bulk of the code is in
https://github.com/BlueWall/opensim/tree/integration/OpenSim/Services/IntegrationService
and
some at
https://github.com/BlueWall/opensim/tree/integration/OpenSim/Server/Handlers/Integration
. Some example code is
here...

landtool using a plugin to retrieve data needed for land sales:
https://github.com/BlueWall/IntegrationHelper with the
php here: http://pastebin.com/yUGph1aD

It is working for test purposes and has some supporting facilities
setup for testing with more details at:
http://bluewallvirtual.com/node/46 . I will be getting some additional
documentation together about developing the
plugins and setting up repositories. I have set some time aside to be
responsive to working on this and would appreciate
input. I am interested in getting more eyes on it and getting it ready
to include in core under an experimental status.
After that, I would be interested in helping develop a stable/basic
API for integration with the plugins and helping get
the API into popular language libraries.

Thanks!
BlueWall
_______________________________________________
Opensim-dev mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/opensim-dev




_______________________________________________
Opensim-dev mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/opensim-dev



--
Justin Clark-Casey (justincc)
http://justincc.org/blog
http://twitter.com/justincc


_______________________________________________
Opensim-dev mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/opensim-dev

Reply via email to