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
[email protected]
https://lists.berlios.de/mailman/listinfo/opensim-dev
_______________________________________________
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
_______________________________________________
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
_______________________________________________
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
