search

IslandzVW / halcyon

InWorldz Halcyon 3d virtual reality world simulator
BSD 3-Clause "New" or "Revised" License
21 stars 26 forks source link

Expose Grid Name through llGetEnv #6

Closed zauberparacelsus closed 8 years ago

zauberparacelsus commented 9 years ago

This is a suggestion that I thought may be useful, which is that by passing the "grid_name" parameter to llGetEnv, it will return the name of the grid that the script is running on. For example, run it on InWorldz, it'll return "InWorldz". Run it on InWorldz Beta, it'll return "InWorldz Beta". Run it on HalcyonGrid2000, get "Halcyon Grid 2000" in return.

I figure that this function will be useful to scripts written for Halcyon. If OpenSim or WhiteCoreSim include a similar extension for llGetEnv, Halcyon can probably follow that extensions convention to reduce the work needed to get opensim scripts to work with Halcyon.

appurist commented 9 years ago

There is already these two: llGetEnv("sim_channel") -> "InWorldz Server" and llGetEnv("InWorldz") -> "1" (true)

These need to be updated for non-InWorldz environments (they are currently hard-coded to allow testing by cross-platform LSL scripts. But they should be extended, e.g. to have a hard-coded: llGetEnv("Halcyon") -> "1" (true) and another such as the suggested "grid_name" that returns something from the Grid Info settings. There are two in the INI file, "gridname" (descriptive) and "gridnick" (shorter version, good for script tests). Might be good to add llGetEnv strings for other such as the login URI.

appurist commented 9 years ago

I didn't specifically mention what to do about "sim_channel" above, on grids other than the main InWorldz grid (e.g. "InWorldz Beta") but I was thinking it would be good to return the "gridname" setting from the [GridInfo] INI data. However, that would be an incompatible change for the main grid because it currently returns "InWorldz Server" for "sim_channel" while the INI file specifies a gridname of "InWorldz". What we could do though is map llGetEnv("sim_channel") where gridname=="InWorldz" to continue to return "InWorldz Server" there instead, otherwise return the gridname field from the INI settings. We could do this in addition to adding two more for "gridname" and "gridnick" settings.

Vinhold commented 9 years ago

While you are at it, all hard coded references to InWorldz should take the information from the INI "gridname" or "gridnick" as appropriate. That way there is only one place to put the grid name and nickname to change all places if that is possible. In my Gospel World the client shows InWorldz for the Library inventory items and shows up in a number of places in the client because of the hard coded names. :)

kf6kjg commented 9 years ago

For the interested and not yet aware, the relevant section of code is in OpenSim.Region.Framework/Scenes/Scene.cs : GetEnv(string).

I just tripped across it while working on something else.

zauberparacelsus commented 9 years ago

Yeah, I was poking around in there a few days ago to add support for some of the unsupported parameters from Second Life.

appurist commented 9 years ago

There is a lot more involved to implement this request, or any from the GridInfo settings. The problem is that the script runs on a region server, and the grid info is provided by the login user login service (a completely separate process).

On the InWorldz grid, that user login service is running on the login.inworldz.com machine, and provides that info for viewer requests. The region servers have not had any need for this information to date, so not only is it not local to the machine, it's an XMLRPC or REST call to the remote service to fetch the info.

Now, in order to prevent a script from impacting inter-service comms, we'd need to fetch the grid info on demand and cache those values for some TTL-like value, like 10 minutes, in case they were updated and the user login service was restarted.

So at this point, I'm not sure the cost/benefit trade-off is favorable. It would probably depend on how important the real usage case is here. If it is used to just display the information to the user, well, it already is, on their Login screen (they chose which grid to log in).

appurist commented 9 years ago

However, I do think it is probably a good thing to provide something to detect a Halcyon server, just as we did for the "InWorldz" test. But that is no longer an appropriate choice for all Halcyon use, so adding an llGetEnv("Halcyon") == "1" is probably a good thing.

I'm not sure what the best way to detect the beta grid, or any other grid might be. If the host names were known, as they are on the InWorldz Beta grid (all one machine), there is a simple test there using the recently-added llGetEnv("simulator_hostname"). That would not work be as useful on a multi-machine grid though, and currently that returns "COMPUTERNAME" on the beta grid (if you call llGetSimulatorHostname())...

kf6kjg commented 9 years ago

Interesting. I figured, based on our setup, that every region got the same INI file - however after thinking about your answer I realize that not every subsystem needs every entry in the file. That said there is quite a small pile of places in the code that currently return a hardcoded value for the grid name, as Vinhold was alluding to. LoginResponse.cs, LoginService.cs, and LibraryRootFolder.cs being primary culprits.

If we assume the grid name is something that's not going to be easily changed, then it should be ok to specify that it can and should be in every server process' and region's Halcyon.ini. Once that's done we can then use that specification that allows us to feed the above request for a script-accessible grid name, as well as feeding these other services: we simply make sure the values are read from the file (or made accessible some easy way) by every server process that needs it, and go from there.

kf6kjg commented 9 years ago

Do you think it'd be usable/acceptable just to specify the grid name in the region's xml? That'd solve the problem of getting that data to the relevant section of code, but would mean that every region could technically have a different grid name. Not the cleanest nicest design, but solves the problem easily.

Vinhold commented 9 years ago

The grid name problem here is really not fully a part of the initial comment posted by Zauber. But it will be a problem with anyone who creates a grid using Halcyon as the server code. Additional discussion moved to a new issue entry.

kf6kjg commented 9 years ago

Well, I figured out a "cleanish" way:

        public string GetEnv(string name)
//...
                case "grid_name":
                    ret = m_config.Configs["GridInfo"].GetString("gridname", "GridInfo gridname missing from region server's Halcyon.ini!");
                    break;
                case "grid_nick":
                    ret = m_config.Configs["GridInfo"].GetString("gridnick", "GridInfo gridnick missing from region server's Halcyon.ini!");
                    break;
//...

This does mean, as I stated in my region xml idea, that every region could technically have a different grid name. An administration problem, not really a showstopper. And this is far faster than trying to use XMLRPC calls or whathaveyou! :D

I suggest that sim_channel and sim_version stay almost like they are, with some minor exceptions. I'll explain my thought process first: first, these two are copying from LL's system and should therefore be as meaning compatible as possible; second, these have existing meaning and shouldn't be upset unless there's reason. sim_channel to me, based on LL's and Inworldz' usage, is the software name - in this case "Halcyon Server". Yes that's an incompatible change from InWorldz: 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. sim_version has a problem. Following the LL format makes the most sense for scripts coming from that grid, but InWorlds implemented a different format. My suggestion is to revert it to the LL-style version numbering so that scripters 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.

appurist commented 9 years ago

Don't forget about existing installations. We have 1600 region XML files. We could update our templates and reconfigure them all, but I have to ask what the usage case is here. What would a script use this for (other than for informational display purposes)? All of the issues mentioned above seem to be related to hard-coded info in the viewer, which already knows the grid name; it just needs to replace some of the hard-coded strings with the grid name it already knows. None of that is related to llGetEnv as far as I can see, since the viewer has no relationship with LSL script functions.

If a script wants to know which kind of grid it is running on, it can check for InWorldz, and we should add Halcyon so it could check for that on other grids to know it's running on Halcyon software. But the root problem here is that there has not been any need so far for regions to know what the grid is. That information is exchanged at login time between the login user grid service and the viewer. If we added an LSL function to fetch the grid name, it should be an XML RPC call to the login service for the same data viewers fetch. As this would be an expensive call, it would probably need to be a separate function that invokes the dataserver event as a response, e.g. an iwRequestGridName or something of that nature.

appurist commented 9 years ago

I really don't like the idea of duplicating the grid info across every region config file. There's nothing wrong with fetching it from the login service from the source, except that there's no code to do that currently, since regions have not cared what grid they were on. It wouldn't have to be invoked every time, it could be fetched for example if it hasn't been fetched in the last 10 minutes. It's just that this would be too heavy for llGetEnv to return directly.

kf6kjg commented 9 years ago

While I agree the use cases should be studied deeper, the information about the client getting the grid name mixed up shouldn't be part of the discussion: I've asked Vin to make that a separate issue report. This is about a request for scripts to be able to detect the grid: not about hardcoded inventory names &c.

Interesting about the duplication counterargument: while I understand that not every section is needed in every copy of the ini, all the (scarce at this time) installation instructions we've got have resulted in full copies of everything in for every region. Hence why this wasn't seen as a problem: our grid already has massive duplication.

I've approached this from a coder standpoint up to this point. However Jim, you're right: there needs to be a viable usecase presented for why scripts need to know what grid they are on that can't be fullfilled by knowing what server they are on.

To answer a possible on that last: cross grid scripts may want to know because not every grid may be vanilla Halcyon. However I don't know the viability of cross grid scripts: I think the very idea is ugly.

appurist commented 9 years ago

I was just thinking the same; the wrong strings coming out is for the most part a viewer/server issue. Most of the correct names are provided in the economy info packet at login time. I'm not sure why it wouldn't work now, except some viewers seem to ignore it.

The sim_channel / sim_version discussions should probably be a separate discussion too.

The duplication concern isn't specifically duplication; it is a question of data ownership. Duplication does not matter if the various pieces of software ignore the data that they don't own. The GridInfo is ignored currently except by one component. Only one owns the data and is the authoritative source for the info. We can fan that out to regions, but they must fetch it from the source. At some point we will finish cleaning up these files and I'd hate to not be able to do that because a piece of the software that isn't responsible for that data happens to be peeking at a local copy of some of the settings.

kf6kjg commented 9 years ago

The concept of ownership is a very valid one, and I see your point. Therefore I suggest this particular discussion be tabled until a nice valid usecase shows up. I've above suggested one usecase, however I'm not convinced of the validity as it's predicated on "cross grid scripts" and non-vanilla Halcyon instances - a combo that currently doesn't exist and therefore the usecase is invalid and feature creep.

I'll also fork the other discussion.

appurist commented 9 years ago

The one use case I can think of would be for something like marketplace orders placed to off-world locations, where a script must communicate with an external host such as a website. Even there, I'm not sure it needs to know which grid it's coming from except for statistical reports, etc. Both it and the script have the hostname connecting from, and the scripts have other info such as region name. I'm sure there are scenarios where having the grid name would be useful, but I also suspect there are existing methods (like registering the host name in a table, or subnet identification, or communicating with a central LSL server on that grid which knows (e.g. it's in the server's Description or config notecard), and other possible solutions depending on the scenario).

I think contacting the login user service for the full collection of Grid Info settings is the way to resolve this, but it's a bit of a cost/benefit trade-off at this point with so much other work which is of higher priority. I like the idea and could probably use it for my chatworldz translators, but also like the idea of shelving it until either we have a clear use/need, or priorities allow adding the code to contact the login user service.

appurist commented 9 years ago

My position on this has weakened quite a bit; it's a bit moot for us if we use automation templates that put the gridname and gridnick in the Halcyon.ini file and replicate to the the templates to every region anyway. My concerns were twofold: that a single central data authority remain for the gridnick and gridname (and it does in InWorldz in the template expansion in InWorldz), and the practical nature of this data being in every Halcyon.ini file (also not a concern if they are created from a single central template file, which they are).

So I don't think I have an issue with grabbing the grid info from the Halcyon.ini file, and in hindsight I suspect that this is where the User login server is getting the data too. On any standalone machine, this would also be true, and on other grids, it could simply be the responsibility of the admin to ensure they Halcyon.ini files are in sync when it comes to grid info, or scripts may get inconsistent results traveling from region to region.

So bottom line: I think I'm okay with reading this from the Halcyon.ini file unless Dave or someone else raises another objection.

appurist commented 8 years ago

I've got lots of goodness in PR #74. I'll document it all tomorrow but both "grid_nick" and "grid_name" are in, plus several version options as well as an updated "simulator_hostname". Also "InWorldz" is now variable, "Halcyon" is added, "short_version", "long_version" and ways to differentiate environments for #57 and this issue, in "platform", "grid_management" and "shard". I also made the keyword name strings passed to llGetEnv case-insensitive, so llGetEnv("InWorldz") and llGetEnv("inworldz") are the same.

appurist commented 8 years ago

Here's the test script I used:

ShowEnv(string option)
{
        llSay(0, option+": ["+llGetEnv(option)+"]");
}

default
{
    touch_start(integer num)
    {
        ShowEnv("sim_channel");
        ShowEnv("sim_version");
        ShowEnv("short_version");
        ShowEnv("long_version");
        ShowEnv("frame_number");
        ShowEnv("region_idle");
        ShowEnv("dynamic_pathfinding");
        ShowEnv("InWorldz");
        ShowEnv("halcyon");
        ShowEnv("script_engine");
        ShowEnv("simulator_netname");
        ShowEnv("simulator_hostname");
        ShowEnv("agent_limit");
        ShowEnv("region_product_name");
        ShowEnv("region_product_sku");
        ShowEnv("estate_id");
        ShowEnv("estate_name");
        ShowEnv("platform");
        ShowEnv("grid_management");
        ShowEnv("grid_nick");
        ShowEnv("grid_name");
        ShowEnv("shard");
    }
}
appurist commented 8 years ago

A little more on rationale and the changes I committed. I looked at this stuff again in terms of seeing if there is anything we need to do in order to release the next InWorldz main grid update. After more review, I don't think there is a problem with either the change to llRequestSimulatorData(DATA_SIM_RELEASE) or llGetEnv("sim_channel") as those should really be arbitrary server texts that could change at any time. The problem though is that we haven't had alternatives that don't change over time, but now we do with "platform", "grid_management", and the requested grid tags.

So here are the new ones and changes:

appurist commented 8 years ago

I believe this one is resolved now, with the server providing considerably more than was requested now.