[Opensim-dev] remote admoin wiki page

Justin Clark-Casey jjustincc at googlemail.com
Tue Dec 13 21:57:29 UTC 2011


On 13/12/11 00:02, Argus wrote:
> 1) I agree on renaming the SceneContainsRegion() and already was thinking of CheckRegionParametersInScene() to mach with
> CheckStringParameters(). I also think adding it after CheckStringParameters() would group them nicely.

Hi Michelle.  Sounds good to me.

>
> 2) SceneContainsRegion() is just as clumsy as it was before. Once the variations of region_id is removed it will look
> slighty leaner, hehe.
>
> 3) about 3/4 is using some variation of region_id. In the end its less work for us having all variation allowed for this
> short timeperiod instead of changing 3/4 twice. Neewbies will start off using region_id while everyone else can change
> their code whenever they have time for this. Immidiatly changing the very small variations meens one has a deadline with
> no transitional phase... and I would be one of those affected by this change ;) If someone wants to use a variation for
> the time beeing, well, its not our time being wasted.

Yeah, on reflection it doesn't matter if there are variations as long as we don't publicise them :)  Then in 6 months 
time the ones that are already known about can be removed.

>
> 4) Yes, lots of existing identical text is beeing replaced by identical text. The command functions are however much
> leaner. On avarage we are replacing 20 lines of identical code with 9 short identical code lines. All together the patch
> has more code in the end, because not all commands did accept region_id AND region_name, and neither did they return the
> error parameter. One could add the error parameter and the exeption to SceneContainsRegion() which would shorten it even
> more, but one can also use SceneContainsRegion() to check if a region does not exist when the exeption is not thrown
> within SceneContainsRegion(). Another improvement is, that we have equal error messages in all commands when a region is
> not found.

Yeah, I was just wondering why the patch had sections where no changes were actually made, since it was replacing the 
original line with an identical copy, as far as I could see.  Or perhaps there's a line ending thing going on.

Not a big issue, but this makes "git blame" less useful when you want to find out why a certain change was made.

>
>
> --
> Justin, little off topic... I am working on implementing a bigger number of new commands to RemoteAdmin. Would you
> prefer 1 big Patch or each command as a seperate Patch? Maybe single Patches for each command which have a predifined
> step-by-step order each considering the changes from the previous patch? (A small number of commands can be seen on the
> new wiki Proposal page ;) )

I would vastly prefer one patch per command, so that any issues can be isolated to a specific commit.  There's no 
problem with having one patch rely upon another, as long as the ordering is clear (git will number them 0001, 0002, etc.).

>
>
>
> Am 12.12.2011 22:41, schrieb Justin Clark-Casey:
>> Thanks Michelle. Commented on the Mantis. To be honest, I think it there are variations on region_id that are only
>> used in one spot then they should just be removed, rather than cluttering up the code and suddenly accepting multiple
>> variations everywhere. I think we should only cater for the major region id parameter variations.
>>
>> On 10/12/11 16:11, Argus wrote:
>>> Justin, could you have a glance at the patched RemoteAdminPlugin.cs I posted in Mantis (
>>> http://www.opensimulator.org/mantis/view.php?id=5814 )? I didnt test the code yet and there is still some cleaning up to
>>> do, also the summaries need updates...etc...
>>>
>>> For all RA function that check for regions I added a new general function SceneContainsRegion(). For now all 5 found
>>> versions of region_id would pass through there and also the region_name is then available to all RA functions. I also
>>> added a log warning for those region_id parameter that will be deprecated.
>>>
>>> Advantage:
>>> - less code
>>> - common error warnings when region not found
>>> - easier to remove deprecated region_id versions
>>> - all RemoteAdmin functions checking scenes use parameters region_id _and_ region_name.
>>>
>>> Disatvantage:
>>> - a view RA function get a diffrent error message with the same meaning when the region is not found
>>>
>>> Before I create a patch, I would like to get some feedback if adding the new function is the right direction...
>>>
>>> NB: I finaly managed to update the RemoteAdmin wiki pages.
>>>
>>> Am 10.12.2011 01:46, schrieb Justin Clark-Casey:
>>>> Deprecating the parameters in the wiki is a good idea. 6 months is a reasonable timeframe in my opinion.
>>>>
>>>> On 09/12/11 18:11, Argus wrote:
>>>>> I also prefer the region_id. I will post a patch in mantis this weekend adding region_id were it was not used...
>>>>>
>>>>> Just wondering if one might deprecate the non region_id parameters in 6 months or so, giving everyone time to
>>>>> adapt. In
>>>>> that case I would also add a notice to the code? As i am not a dev, i dont know what agreements there are on this
>>>>> subject...
>>>>>
>>>>>
>>>>>
>>>>> Am 09.12.2011 01:17, schrieb Justin Clark-Casey:
>>>>>> In this case, it wouldn't be so hard to add the facility to take region_id as well as region_uuid (or vice versa).
>>>>>> Personally, I feel that region_id is better then region_uuid.
>>>>>>
>>>>>> Patches are welcome.
>>>>>>
>>>>>> On 08/12/11 21:53, Argus wrote:
>>>>>>> You will find these kind of diffrences everywere in opensim, not only with region uuid. The problem is, that once
>>>>>>> its
>>>>>>> implemented its not easy to change because someone will already have developed something that uses it exactly as it
>>>>>>> was
>>>>>>> implemented.
>>>>>>>
>>>>>>> Am 08.12.2011 21:56, schrieb R. Gunther:
>>>>>>>> Hmm, i see its going deep. its also named regionid in the xml files.
>>>>>>>> Still regionid keeps confusing compared to region_uuid
>>>>>>>>
>>>>>>>> On 2011-12-08 21:18, R. Gunther wrote:
>>>>>>>>> There are somethings not the same on the wiki remopte admin page.
>>>>>>>>> On the remote admin page with many commands you see regionid others use region_uuid
>>>>>>>>> It would be nice and more clear to use with all command the same line "region_uuid"
>>>>>>>>> Or there must be a difference between region_uuid and regionid i dotn see.
>>>>>>>>> _______________________________________________
>>>>>>>>> Opensim-dev mailing list
>>>>>>>>> Opensim-dev at lists.berlios.de
>>>>>>>>> https://lists.berlios.de/mailman/listinfo/opensim-dev
>>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> Opensim-dev mailing list
>>>>>>>> Opensim-dev at lists.berlios.de
>>>>>>>> https://lists.berlios.de/mailman/listinfo/opensim-dev
>>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Opensim-dev mailing list
>>>>>>> Opensim-dev at lists.berlios.de
>>>>>>> https://lists.berlios.de/mailman/listinfo/opensim-dev
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Opensim-dev mailing list
>>>>> Opensim-dev at lists.berlios.de
>>>>> https://lists.berlios.de/mailman/listinfo/opensim-dev
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> Opensim-dev mailing list
>>> Opensim-dev at lists.berlios.de
>>> https://lists.berlios.de/mailman/listinfo/opensim-dev
>>
>>
>
> _______________________________________________
> Opensim-dev mailing list
> Opensim-dev at lists.berlios.de
> https://lists.berlios.de/mailman/listinfo/opensim-dev
>


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



More information about the Opensim-dev mailing list