Skip to content
This repository has been archived by the owner on Aug 19, 2018. It is now read-only.

llGetEnv() sim_channel and sim_version compatibility #51

Closed
kf6kjg opened this issue Oct 14, 2015 · 5 comments
Closed

llGetEnv() sim_channel and sim_version compatibility #51

kf6kjg opened this issue Oct 14, 2015 · 5 comments

Comments

@kf6kjg
Copy link
Contributor

kf6kjg commented Oct 14, 2015

sim_channel and sim_version are two requests to llGetEnv that are part of the SL "spec" - they have examples of structure that result in scripter expectations and consequent unforseen uses - but that are not currently matching that defacto spec terribly well.

Firstly: sim_channel to me, based on LL's and Inworldz' usage, is the software name - in this case "Halcyon Server" and should be changed to such. Yes that's an incompatible change from InWorldz where the string was "InWorldz Server": the product's name has changed and it cannot be called by the old name anymore, at least not without causing confusion on other grids. It is not, as some have thought, ever changed across grids: in SL it is "Second Life Server" no matter whether the main grid or the beta grid. The only place that I can think it would change is if the region was running one of the testing server editions, such as "Magnum". In this case it still is the product name, just a special and highly volatile product name.

Following the above logic, sim_version has a problem: it doesn't follow the defacto "spec" in the format of the version string. Following the LL format makes the most sense to support script compatibility for scripts coming from that grid, but InWorlds historically implemented a different format. My suggestion is to revert it to the LL-style version numbering so that scripters fresh from SL get back what they expect: "11.22.33.9876" as documented on the LSL wiki instead of "InWorldz 11.22.33 R9876" or "Halcyon 11.22.33 R9876" as the code currently returns. Again, the reasoning is that some scripts may be, for whatever reason, parsing the contents and attempting to use the results in unpredictable ways. As a side note, why is this being so redundant and putting in the server product name?!

Now this means that existing scripts that are attempting to parse these values may have issues: people may have already coded relying on the existing format. I can't say one way or the other. What I can see is that when I look at the SL wiki page for llGetEnv() I see a string format that will be returned, and when I test in InWorldz or on Halcyon I don't get what I expect based on what I read there. Another side note: a simple parse of the sim_channel would quickly let any script know what simulator type it's on.

I have an incoming pull request waiting on the results of this discussion, however I figured this kind of change needed more input up front before locking in the changes and submitting them for review.

kf6kjg added a commit to kf6kjg/halcyon that referenced this issue Oct 16, 2015
…pt away.

This is the most complex change so far in this process.
For these changes the VersionInfo file was just the ticket, and that was easy, just needed some tuning.

llGetEnv did get some potentially backwards compatibility breaking changes however - see Issue IslandzVW#51 for the place to discuss those.

I also cleaned out a lot of local duplication and unneeded function-call passing of the version string.  In one location I may have broken ownership rules by giving Phlox direct access to VersionInfo, I'm not 100% certain.

Overall I attempted to try and keep the goals of each of these version and product name calls the same, just centralizing where they got that information from.  The main idea being that the more direct the access, the easier it is to determine what the effects are for any given change.  For example, previously it was a bear to find all the places changing the version string format would effect, now it's as simple as looking at where VersionInfo is used.
@kf6kjg kf6kjg mentioned this issue Oct 16, 2015
@ddaeschler
Copy link
Contributor

See the comments made to #56 as well

@appurist
Copy link
Member

I took a look at llGetEnv results on both InWorldz, Halcyon and SL servers (see output below).

First, I think we should continue to return 'InWorldz Server' on InWorldz regions because scripted objects like delivery boxes or vendors may be using that to distinguish the source grids. However, if we do load gridname and gridnick from the INI grid info, we could do a case-insensitive string comparison against "inworldz" (or also include "inworldz_beta"), otherwise return 'Halcyon Server'.

Second, for "sim_version", I don't really think that would be used by scripts to differentiate actions. There's a slim possibility that changing it to move the revision number to the fourth field of the version might cause code that chopped it after the space to fail, but there isn't much chance of that and scripts should not be too dependent on the resulting string format, and should be written to adapt to small format changes. If there wasn't "sim_channel", I'd be more concerned about this, but given that is the better choice for most of these uses, I think we should go ahead and change it to return the x.y.z.r version string. This wouldn't require any code changes in the GetEnv code but rather would be a more global change to VersionInfo.Version which I will need to review. We'd need to figure out if we want to continue to describe the revision publicly as the Rnnnn version. (I'd need to give that a bit of thought first.)

Also, not part of this Issue, but I think the "simulator_hostname" (and llGetSimulatorHostname) should change to return the external host name or IP address rather than the NETBIOS name. This is mostly meaningless to everyone (including IW staff) and the SL implementation returns the an external domain address. We can implement this by returning RegionInfo.ExternalHostName (which comes from the region.xml file) in that field instead of the system name.

InWorldz 0.9.17:

llGetEnv("sim_channel") = 'InWorldz Server'
llGetEnv("sim_version") = 'InWorldz 0.9.17 R5720'
llGetEnv("region_product_name") = ''
llGetEnv("region_product_sku") = ''
llGetEnv("agent_limit") = ''
llGetEnv("simulator_hostname") = ''

Halcyon 0.9.18:

llGetEnv("sim_channel") = 'InWorldz Server'
llGetEnv("sim_version") = 'Halcyon 0.9.18 R5767'
llGetEnv("region_product_name") = 'Estate / Scenic'
llGetEnv("region_product_sku") = '3'
llGetEnv("agent_limit") = '40'
llGetEnv("simulator_hostname") = 'WIN-RJQIQFPAM3B'

SL:

llGetEnv("sim_channel") = 'Second Life Server'
llGetEnv("sim_version") = '15.09.25.305449'
llGetEnv("region_product_name") = 'Estate / Full Region'
llGetEnv("region_product_sku") = '024'
llGetEnv("agent_limit") = '100'
llGetEnv("simulator_hostname") = 'sim10452.agni.lindenlab.com'

SL beta grid:

llGetEnv("sim_channel") = 'Second Life Server'
llGetEnv("sim_version") = '15.09.25.305449'
llGetEnv("region_product_name") = 'Mainland / Full Region'
llGetEnv("region_product_sku") = '023'
llGetEnv("agent_limit") = '40'
llGetEnv("simulator_hostname") = 'sim9004.aditi.lindenlab.com'

@kf6kjg
Copy link
Contributor Author

kf6kjg commented Oct 18, 2015

You mean like

                 case "simulator_hostname":
-                    ret = System.Environment.MachineName;
+                    ret = RegionInfo.ExternalHostName;
                     break;

I'd been saving that one, and a couple more, until I got the descision on these! :P

@appurist
Copy link
Member

I think we can safely change the hostname, since the NETBIOS name is not going to be useful to any script or external site in communication with the script, while the external hostname would be.

@appurist
Copy link
Member

I believe this one is resolved in #74 where the suggestions in the OP above are all implement and a few more.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants