How to handle changes that specifically break InWorldz backwards compatilibilty?

[kf6kjg]

I'm seeing now multiple instances of "hey, we might need to be able to keep X feature running the way it always has been for InWorldz". I've got no problem directly with that: the business has clients to keep happy. So the question is how to do so in the cleanest way? Can or should said process also be used for fixing backwards compatibility issues between various Halcyon releases?

While this discussion is based on issues brought up in #51 and #56 it is more about an overarching business decision than a specific decision on a specific case.

I can think of a few ways of doing this, most likely not exhaustive and not in any particular order:

1. Compile time flags, such as an INWORLDZ_COMPAT flag, surrounding blocks of code. Said flag could be specified at build time for testing and release editions.
2. Refactoring relevant code to call specialized private methods that are chosen based on a setting in a configuration file.
3. Refactoring relevant code to call specialized classes via interface definitions (polymorphism) that are chosen based on a setting in a configuration file.
4. The InWorldz team keeping a private set of patches to, or a private fork of, the Halcyon source that is for keeping such things and others from proving to be a hindrance to Halcyon itself.

There are likely other ways as well.

I think that information ownership could cause a problem for 2 and 3: potentially many sections of the code could be wanting to know this information. 4 could provide some benefit in that it would allow you to develop private extensions that might have licensing issues, but could mean MORE work for a small team. 1 at least shares the load, but means messier code.

Thoughts, opinions, decisions?

[appurist]

I am not a fan of conditional compiles as they tend to end up eventually polluting the code and/or making it less readable, as well as other issues (like matching { and } and intellisense issues). Most of the other suggestions seem a bit manual in nature, or a bit heavy-handed for the problem (polymorphism).

I think I'd rather just see something that could read a setting from the Halcyon.ini file that would cause a runtime difference of behavior. The differences would then also be self-documenting and searchable using the conditional.

But we haven't really had a problem with maintaining compatibility so far without such a scheme. Which features would you want to change in an incompatible way?

[kf6kjg]

The idea here wasn't a specific instance, but a guideline for the future: this will keep coming up and having a decision made as to a most acceptable practice will likely help prevent problems.

Thankfully the system has been designed to allow for expandability already, so if I wanted to develop the completely different scripting system that's been bouncing around my skull for the past couple of years, all I'd have to do is create a new DLL and activate it - I think; I'm not at the moment seeing a way to choose which script engine to use but I think there is a mechanism already in place for doing so.

However something like removing "InWorldz" parameter, or changing to "0", would be a lot more difficult. In this case the code's no longer explicitly on the "InWorldz" grid so this parameter no longer makes direct sense either way. The core question with this example would be in what way would this be acceptable? Removing or changing breaks InWorldz, but it is irrelevant and incorrect on other grids using Halcyon. Exact discussion for this example belongs in #6 I think, but it is where I first starting seeing this problem show up.

Since then we've had #51 with changes to sim_channel, inconsequential possible changes to simulator_hostname, and I predict that more of these kinds of breakages will show up - GetEnv is just getting hit the most at the moment. It is my opinion that having a guideline in place, with the discussion of why it was put in place, will help prevent headaches and community problems as the community grows.

[appurist]

I think it depends on the specific cases. In some cases, in particular the environment-related ones, we could for example use the gridnick to differentiate, but for most other cases, we should be doing our best to avoid a script being able to tell if it's on InWorldz or another grid. Scripts that work in InWorldz should really just work on any Halcyon server, unless there's a significant reason not to (such as license restrictions on the script). And in those cases, we have the environment-related functions to differentiate the grids.

So I think llGetEnv is naturally going to attract a lot of these cases, while others are not going to need to know. For data elements other than something that is, by definition, environment-sensitive (such as gridnick), I don't particularly think it's a good idea for other grids running Halcyon servers to be incompatible with the same scripts on InWorldz.

For the cases where we do want to differentiate in the server code, we should probably pick something, and wrap/factor it inside some kind of Scene.IsInWorldz() function. That something could be the gridnick, but I think it would be better to simply add an InWorldz=1 or 0 to the GridInfo section in the INI file. Internally that could be used to differentiate, but I don't think that's a good idea to pass that to scripts directly. So...

Whether that InWorldz INI variable should be directly returned in llGetEnv("inworldz") is questionable. I see that as something typically differentiating between InWorldz and completely different systems (namely SL), the same way that you probably want to select OpenSim when adding InWorldz in a viewer grid list. It pretty much means "not SL". If on other Halcyon servers you started returning "0" for llGetEnv("InWorldz"), it might cause those scripts to assume it's an SL server. But maybe we could leave that configurable to other Halcyon grids, by having a second INI option (like InWorldzGetEnv=0 or something like that), defaulting to matching the InWorldz one, but allowing other Halcyon grids to override it and set it to 0 if they wanted, and weren't worried about possibly confusing scripts from InWorldz.

But this last part also reminds me we probably also need a llGetEnv("script_engine") == "Phlox" on Halcyon servers. I was going to suggest a "Phlox" environment variable but we made that mistake already with the "InWorldz" one and I don't want to repeat it. Ideally that should have been something like llGetEnv("gridtech") to match "gridnick" and "gridname", and could have returned "InWorldz", i.e. return a possible enumeration rather than a boolean. There are often third choices. For example, if we adopted the enumeration approach, OpenSim could add something compatible for "script_engine" that does not return "Phlox".

[kf6kjg]

I agree, in regards to scripting llGetEnv IS going to attract the majority of these cases! 😀

I'm liking the Scene.IsInWorldz() function idea. That's a good alternative, though in possible non-Scene.cs cases it might get unwieldy vs directly reading from the proposed INI parameter, thought it'll likely be rare that non-Scene.cs code won't have access to a Scene instance.

Ah, I was seeing llGetEnv("InWorldz") == "1" as a shortcut and workaround, due to InWorrldz not returning a useful "simulator_hostname", to asking llSubStringIndex(llGetEnv("simulator_hostname"), "aditi") > -1 which is the safest way I can think of to test if the script is on the SL main grid or llSubStringIndex(llGetEnv("simulator_hostname"), ".lindenlab.com") > -1, and probably should be similar for testing for the InWorldz grid once "simulator_hostname" is fixed.

I agree that careful thought should be the practice when adding user-facing values like this.

[appurist]

Exactly. And I'm not sure about putting such a function in Scene either; that was mostly just an example. One of the OpenSim base modules available everywhere may be a better location. I'll take a closer look at this in a day or so.

[appurist]

ok so with the Halcyon.ini support added to llGetEnv, I think we can pretty much close this one. If we need to differentiate, we can use scene.GetEnv("grid_management") or scene.GetEnv("InWorldz") or substring checks on scene.GetEnv("simulator_hostname") or scene.RegionInfo.ExternalHostName, all of which are directly or indirectly available to LSL scripts as well.

[kf6kjg]

While those will definitely help, the concept here was to grind out a recommended procedure for dealing with changes that break compatibility in general, not just the scripting and llGetEnv.

I think the closest we got was your statement "I think it depends on the specific cases" - which is true, but doesn't really answer the question: how do we, as a team of Halcyon devs deal with useful changes that come in that could cause breakage or backwards compatibility issues in InWorldz, the biggest consumer of Halcyon? Writing a well thought out answer to this will help prevent community fragmentation in the future: people will understand and not wind up "taking sides", and potentially causing a development fork, when hard calls have to be made.

No I don't have a specific situation or goal to accomplish that could cause this: I just foresee a potential problem that could be mitigated by a little early decision making.

[appurist]

I said we'd have to take a look at each situation because we haven't really encountered such a situation in years of doing this, because we don't actually make changes that break existing content. Our network protocols and storage definitions add new fields in backwards-compatible ways, and we have been migrating to ProtoBuf formats for both that can easily be extended. For example object crossings now send a full list of seated avatars rather than just the count, but still send the count so there's no issue with crossing to an older region. The only difficult backwards-compatibility is with existing content, and for that we have two approaches: either maintain compatiblity, or introduce versioning so that we can distinguish between new content or old. We've come close to doing that a few times, but to date have managed to avoid it. The avatar-as-a-prim changes in OpenSim broke existing content, and for that reason we would reject such an approach. Our implementation will preserve the existing behavior but will recognize when a script attempts to treat a seated avatar as a prim, and switch into the SL-compatible semantics then. If we couldn't do that, we'd add versioning to the script engine and resolve it that way.

[kf6kjg]

With the close of InWorldz this is less relevant now.

That said, I've used some of what we've discussed above to build https://github.com/InWorldz/halcyon/wiki/Grid-support