Opensim 0.9.0.0 rc2

#3 Merged at 98d8bfb
Repository
Deleted repository
Branch
opensim-0.9.0.0-rc1 (2a34413ad1e4)
Repository
kwdb
Branch
default
Author
  1. makopo
Reviewers
Description

updated as of opensim-0.9.0.0-rc2. This change is to be released under the LGPLv3 license.

Comments (7)

  1. Sei Lisa repo owner

    Looks very good, thank you! Some comments:

    1. I assume these are your own contributions. I'll need a valid copyright statement, unless you place the changes in the public domain. If you don't, I can't incorporate them unless you release the changes under the same license as the file's (LGPL 3+) or a compatible one (see this list).
    2. If the descriptions for the new OS functions are yours, point 1 above applies. If they are not, I'll need to check license compatibility and the copyright status, or they should be removed. I'm still wondering if we should just get rid of all descriptions, since they don't seem to be useful by anyone.
    3. Can you please run constants-test-os.lsl and functioncheck-os.lsl in a suitable OS server, and add a new file, last-func-const-check-os.txt with the results? If they don't run, I'd appreciate a fix to functioncheck.py and constantvaluecheck.py to make them work in both SL and OS.
    4. There's a typo in osGetAvatarHomeURI: "RReturns an avatar's...". Please fix (unless removed, see point 2 above, and same applies for the rest of points below).
    5. Another typo in osForceCreateLink: "Idential" should be "Identical"
    6. osSetHealth: "avatars" should have an apostrophe: "avatar's".
    7. osSetHealthRate says that it "Gets" - shouldn't it be "Sets"? Also, shouldn't it be "disables" instead of "can disable"?
    8. (Edited to add:) Now that I notice, can you please confirm that osGetAvatarHomeURI, osForceOtherSit, osSetHealth, osGetHealthRate, osSetHealthRate all accept parameters of type string instead of key? osGetGender is marked as accepting a key, which is what I would expect.
  2. makopo author

    Thank you for comments. I updated the request commits. Some of comments and requests.

    1. OS doesn't accept "quaternion"type (in current version or from the beginning?) Although I specified as so in kwdb.xml, test script is created. I commented out "quaternion" line manually and saw successful message.

    2. The lines which has "version="sl:16.05.24.315768"" like properties (e.g. ATTACH_LHAND_RING1) didn't have grid="sl", which resulted in creating script for OS as well. I added grid properties to them to avoid creation.

    3. The value for RCERR_CAST_TIME_EXCEEEDED is changed from 3 to -3 in 0.9.0 due to Mantis#7591. To pass the test, I changed the value.

    4. The value for PIs is not as the same as that of Second Life. From what I have searched, even 0.7.4 had different value.

    5. The test code for deprecated constant (OS_NPC) was generated. To pass the test, I commented out OS_NPC. Shouldn't I put deprecated methods or constants if OS server no longer accept them on compilation process?

    6. llSetPhysicsMaterial is actually has method but due to interface bug, it generates compilation error. To pass the test, I removed for now. I reported it as Mantis#8117. It seems to be trivial issue so I believe it will be fixed soon, but at least not 0.9.0.0-rc1.

    7. I changed double type to float type in some methods I added this time. OS doesn't have double type at all.

    8. (To your 8.) I tested both string and key in those method, the test was passed. Do you mean if the method actually accept uuid, the type should be stated as "key"? I looked in some wiki pages (e.g. osForceOtherSit) and saw it is publicly stated as "string" even if it is actually accepted as key in implementation. If you really think they should be treated as keys, I'll fix all os* functions to follow that standard.

  3. makopo author

    Since OpenSimulator 0.9.0.0-rc2 was published just ago, I updated kwdb.xml again. Surprisingly, llSetPhysicsMaterial issue which I reported yesterday was fixed in rc2.

    Only leaving test-generator issue was "quaternion" error, I commented out that line in generated lsl, then run the test and created last-func-const-check-os.txt with the results.

  4. Sei Lisa repo owner

    Great job, thank you very much! Final (hopefully) notes:

    1. The version for ATTACH_FACE_TONGUE has been inadvertently changed from 16.06.20.316774 to 16.05.24.315768 (I presume that's a copy/paste error).
    2. I don't think the law distinguishes between copyright and partial copyright. When a work is a joint effort, all authors have copyright over it. As for the (C) symbol, it doesn't have any legal meaning, only the © symbol does, but since it's hard to guarantee that the encoding will be correct when the file is read, I didn't use it. I preferred to put the (C) to the left so that the rest of the sentence wasn't interrupted by the (C).

    On your point 1: Thanks for the note. The test generator program lsl2dfg/lsloutputs/functioncheck.py will need adjustment, then, to generate the types programmatically. I can take care of that if you prefer.

    On your point 2: Thanks for the heads-up. It seems easy to forget to add the grid on new keywords.

    On your point 5: Yes, status="deprecated" is for keywords that are still present but are not recommended to be used because they may disappear or, more likely in SL, because there's a better replacement. When a constant is removed, it should not be in the database, but for historic purposes, it's best to at least leave it in commented form. It has happened in SL twice that I know of: with KFM_CMD_SET_MODE and with ATTACH_FACE_TOUNGE. Look them up to use them as examples, if you wish.

    On your point 7: I don't know where I got 'double' from when I first went through the OS list of keywords. I used the source code as a reference, so it's likely that the type in the code didn't match the type used in the script. Thanks for fixing it.

    On your/my point 8: If they are internally defined as strings, then they are strings, and if they are internally defined as keys, they are keys. If the documentation says strings, chances are that they are defined as strings, thanks for checking that. LSL automatically inserts type casts from key to string and vice versa in almost all cases, making it hard to know whether it was a string or a key in the first place, but it makes a difference when concatenating strings and for memory usage. I don't know if it makes any difference whatsoever in OpenSim, but being rigorous is the best bet, that's why I asked.

    Lastly, you didn't specify the license you want for your changes. If it's LGPLv3 then all is good already, but I need to know. Sorry, I know legal issues are the most tedious and boring part of dealing with free software, but I'm attempting to do it right.

  5. makopo author

    I fixed version issues and copyright. Thank you for pointing out. I tested the generated code again and confirmed.

    About OS_NPC keyword, I confirmed that it was removed from master just after introducing it, but it had been there in the release builds until 0.8.2.1.

    About test scripts, I have manually commented out this time. I hope you'll fix the generator in the future.

    About key or string thing, OS internally treats both LSL types as C# String (wiki). Some developers happened to use string types for uuid in old days, and related functions treated the same parameters as such, despite of request e.g. Mantis#7973. Looking into existing kwdb.xml, I guess the arguments defined as string or LSL_String in IOSSL_Api.cs are described as string type, and those defined as key or LSL_Key are described as key type, so I described as such. osGetGender is actually defined as key, and others you mentioned were string. (FYI osGetGender was implemented by one of the top contributors, and Health* was first implemented by an non-core contributor...)

    About license, I stated at the top of this pull request, since I have no idea where to put. Is that ok?