[Opensim-dev] Remove check for IsGod in some OSSL functions

[Justin Clark-Casey]

I don't think this can always be done but in this case I would accept renaming the method and not have an 
EstateSettings.IsEstateManager() at this point.

On 14/04/12 00:13, Melanie wrote:
> Well, current assumption in the code is that an estate owner ism
> able to do anything a manager can do. Except in OSSL. It's one thing
> to rename a method to make it's function more clear, but I believe
> the separate method you proposed is not needed.
>
> If you rename a method, any external code that used it will fail and
> will have to be adapted.
>
> If you, however, rename a method and then introduce another one with
> the name of the old one, but different functionality, external code
> will still compile but behave wrong.
>
> So I would not oppose renaming the function to
> IsEstateOwnerOrManager, but I would suggest not to implement a
> specific IsEstateManager function at this time. That functionality
> is only needed in one place and there is already code there that
> emulates it.
>
> Melanie
>
> On 14/04/2012 01:08, Justin Clark-Casey wrote:
>> You are conflating two separate points, a category system for OSSL functions and the fact that the code is internally
>> inconsistent in its view of whether an estate manager includes an estate owner or not.
>>
>> Any philosophy of "Don't touch a running system" is the way to long term irrelevance and code that is increasingly
>> complicated for new contributors to work out.  It has no place in an alpha-level open-source project.
>>
>> On 13/04/12 23:59, Melanie wrote:
>>> That would overomplicate things, IMHO. There is no VIEWER BASED
>>> permission that an estate manager has that an owner doesn't have.
>>> Therefore creating a whole new method just for a single use (OSSL)
>>> seems unnecessary to me.
>>>
>>> Looks to me like a case of "Don't touch a running system".
>>>
>>> Melanie
>>>
>>> On 14/04/2012 00:21, Justin Clark-Casey wrote:
>>>> I can see an argument for ESTATE_OWNER and ESTATE_MANAGER being separate categories.  I'm assuming that in the viewer
>>>> estate owners are can also be listed/not listed as estate managers.  As Melanie says, we have to be aware that
>>>> OpenSimulator is trying to accomodate many different use cases.
>>>>
>>>> However, that does not mean that permission categories can be allowed to become a confusing and/or overlapping mess.
>>>> Whether that is or isn't the case I don't know - this needs to be written out properly on the wiki for all permission
>>>> types (parcel, estate, etc.) rather than only being embedded within the code.
>>>>
>>>> I see that EstateSettings.IsEstateManager() regards an owner as a manager, whilst OSSL_Api.CheckThreatLevel explicitly
>>>> does not regards an owner as an ESTATE_MANAGER.  This is a contradiction.  The code MUST be internally consistent both
>>>> for sanity's sake and for ANY hope that the code can document itself.
>>>>
>>>> In the first instance, I would want to create EstateSettings.IsEstateManagerOrOwner() and call this where appropriate,
>>>> with IsEstateManager only returning true for estate managers, as is proper.
>>>>
>>>> On 13/04/12 15:07, Oren Hurvitz wrote:
>>>>> Regarding estate owner vs. manager: here's the existing implementation of EstateSettings.IsEstateManager():
>>>>>
>>>>>            public bool IsEstateManager(UUID avatarID)
>>>>>            {
>>>>>                if (IsEstateOwner(avatarID))
>>>>>                    return true;
>>>>>
>>>>>                return l_EstateManagers.Contains(avatarID);
>>>>>            }
>>>>>
>>>>> It explicitly adds the estate owner to the list of managers. The permission ESTATE_MANAGER should match the method
>>>>> IsEstateManager(). Therefore, it should include the estate owner.
>>>>>
>>>>> Oren
>>>>>
>>>>>
>>>>> On Fri, Apr 13, 2012 at 4:49 PM, Melanie-2 [via opensim-dev]<[hidden email]
>>>>> </user/SendEmail.jtp?type=node&node=7462795&i=0>>   wrote:
>>>>>
>>>>>       Not so. ESTATE_MANAGER MEANS estate MANAGER. This is intentional. I
>>>>>       reread the code and found that it already allows a comma separated
>>>>>       list notation, so you can use ESTATE_OWNER,ESTATE_MANAGER.
>>>>>
>>>>>       This allows for finer grained control. Please understand that estate
>>>>>       owners may be significantly different from estate managers, for
>>>>>       instance in the case of a mainland. This division is useful and,
>>>>>       again, making changes could cause privilege escalation in some
>>>>>       grids. So, -1. What you want to achieve can already be achieved.
>>>>>
>>>>>
>>>>> ------------------------------------------------------------------------------------------------------------------------
>>>>> View this message in context: Re: Remove check for IsGod in some OSSL functions
>>>>> <http://opensim-dev.2196679.n2.nabble.com/Remove-check-for-IsGod-in-some-OSSL-functions-tp7462127p7462795.html>
>>>>> Sent from the opensim-dev mailing list archive<http://opensim-dev.2196679.n2.nabble.com/>   at Nabble.com.
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> 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