DEFINITION_BLOCK Initialization of Nested Locations

[tajmone]

• [x] Create dedicated test in tests/integrity/ to verify that complexly nested locations are properly recreated after being moved into my_game.
• [ ] Cleanup the definition_block initialization code and resolve the various problems annotated in its comments.
  • [x] Also handle DARK_ROOM and DARK_SITE instances, locating them at indoor and outdoor, respectively, just like for ROOM and SITE instances.
  • [x] Fix Rooms/Sites safeguard by swapping Steps 3 and 4 to ensure that Rooms/Sites relocations to indoor/outdoor is done as the last step, to prevent it being undone by the step that reconstructs the original nesting hierarchy.
  • [x] Document why in Step 1 nowhere is being EXCLUDEd from FROM nested OF loc.
  • [x] In Step 1, only track locations which are DIRECTLY nested into each other.
  • [ ] Step 3 fix the dubious line IF loc <> my_game AND subloc <> my_game.
• [ ] Add to the initialization some code to check that there are no other instances of definition_block except my_game? (i.e. a singleton)

---

This Issue is dedicated to the analysis and discussion of the DEFINITION_BLOCK initialization code that deals with nesting every location into the my_game instance, located in lib_definitions.i (LL 265–320).

That code seems to contain potential bugs and some redundant code. It might also need to updated due to new library classes, and in general it might benefit from some code optimization and clearer comments on what it does.

EVERY definition_block ISA LOCATION
  ...
  INITIALIZE
  ...
  -- ===========================================================================
  -- Nest Every Location into my_game
  -- ===========================================================================

  -- The following code ensures that every location in the adventure (except for
  -- `my_game` itself and the special `nowhere` location) will become nested
  -- into the `my_game` location.

  -- Thanks to this, any verb defined on `my_game` will be globally available,
  -- which is what makes it possible to override a predefined library verb by
  -- re-defining it on the `my_game` instance --- since `my_game` is always the
  -- outermost nester of any location, all verbs defined on it will always be in
  -- scope, anywhere.

    FOR EACH l ISA LOCATION
      DO
        EXCLUDE nowhere FROM nested OF l.
        IF COUNT ISA LOCATION, AT l > 0
          THEN
            FOR EACH x ISA LOCATION, AT l
              DO
                INCLUDE x IN nested OF l.
            END FOR.
        END IF.
    END FOR.

    FOR EACH l ISA LOCATION
      DO
        IF l <> my_game AND l <> nowhere
          THEN LOCATE l AT my_game.
        END IF.
    END FOR.

    FOR EACH r1 ISA ROOM
      DO
        LOCATE r1 AT indoor.
    END FOR.

    FOR EACH s1 ISA SITE
      DO
        LOCATE s1 AT outdoor.
    END FOR.

    FOR EACH l ISA LOCATION
      DO
        IF nested OF l <> {} AND l <> my_game AND l <> nowhere
        THEN
          FOR EACH x ISA LOCATION, IN nested OF l
            DO
              IF l <> my_game AND x <> my_game
                THEN LOCATE x AT l.
              END IF.
          END FOR.
        END IF.
    END FOR.

[tajmone]

Possible Initialization Bug Spotted

I was studying the code that handles the initialization of DEFINITION_BLOCK, and I've noticed a potential bug in lib_definitions.i (line 287):

    FOR EACH l ISA LOCATION
      DO
        IF nested OF l <> {} AND l <> my_game AND l <> nowhere
        THEN
          FOR EACH x ISA LOCATION, IN nested OF l
            DO
--->          IF l <> my_game AND x <> my_game
                THEN LOCATE x AT l.
              END IF.
          END FOR.
        END IF.
    END FOR.

In the innermost DO the line IF l <> my_game AND x <> my_game should probably be:

IF x <> my_game AND x <> nowhere

Because the l <> my_game check was already carried out in the previous IF:

        IF nested OF l <> {} AND l <> my_game AND l <> nowhere
                                 ^^^^^^^^^^^^

Also, it seems to make more sense that the innermost IF verifies that the iterated location x, from the nested Set, isn't my_game nor nowhere.

Does it make sense?

Code Optimization Suggestion

I also think that moving all the exceptions for nowhere and my_game into the the first loop that populates the nested Set of each location would provide a more readable and cleaner code, for we could dispose of any further checks for nowhere and my_game in the subsequent loops, since we'd know that these won't be found in the nested Sets.

It would also make it easier to maintain this initialization when new special locations are introduced in the library (e.g. the restriction levels being discussed in #113) since similar exception would only appear once in the code.

[tajmone]

Special ROOM/SITE Handling

There's also another aspect of this nested-locations initialization code that caught my attention, i.e. that there's a special loop for ROOM and SITE instances:

    FOR EACH l ISA LOCATION
      DO
        IF l <> my_game AND l <> nowhere
          THEN LOCATE l AT my_game.
        END IF.
    END FOR.

    FOR EACH r1 ISA ROOM
      DO
        LOCATE r1 AT indoor.
    END FOR.

    FOR EACH s1 ISA SITE
      DO
        LOCATE s1 AT outdoor.
    END FOR.

    FOR EACH l ISA LOCATION
      DO ...

Since both the ROOM and SITE are already declared as being AT indoor/outdoor in their class definitions, these dedicated loops might just as well be unneeded — i.e. the FOR EACH l ISA LOCATION loop should be able to handle properly all ROOM and SITE instances, unless the author has explicitly located them somewhere else than indoor/outdoor.

@AnssiR66, my questions in this regard are:

• Are these two dedicated loops for ROOM and SITE really necessary?
• Are they a precaution to enforce ROOMs and SITEs at indoor/outdoor, in case authors relocated them elsewhere?

DARK_ROOM and DARK_SITE

Now that we've added the new DARK_ROOM and DARK_SITE classes to the library, we ought to add a dedicated loop for these two, in the above code — i.e. if these loops are indeed required in the first place.

[AnssiR66]

Are these two dedicated loops for ROOM and SITE really necessary?

I am not sure, I just did it as a precaution, because the code just above,

FOR EACH l ISA LOCATION DO IF l <> my_game AND l <> nowhere THEN LOCATE l AT my_game. END IF. END FOR.

places every location at my_game. So, for example "indoor" will be placed at my_game. Similarly, any room will be also placed at my_game. Is a room still nested in "indoor", or is everything just directly nested at my_game? That was what I was not sure about.

Are they a precaution to enforce ROOMs and SITEs at indoor/outdoor, in case authors relocated them elsewhere?

I didn't think about anything authors might do, it really was just mainly a precaution against the previous bit of code, like I expressed above.

[AnssiR66]

Ok, you can make the suggested changes (delete the unnecessary code etc.), where you deem it appropriate. It was a while ago since I did the coding, and you can look at this with fresh eyes.

[tajmone]

Thanks for the clarification.

Before tweaking this rather delicate code, I want to create a small test adventure that would allow us to check that any code tweaks don't mess up the library initialization — e.g. create a small adventure with various types of rooms, adding a custom command to show where the current location is nested at, and another to show the nesting status of every game location (including special locations like nowhere, my_game, etc.).

By adding this adventure to the test suite, any disruptions in the library behavior should immediately show up in the test logs.

[tajmone]

Add Initialization Sanity Checks

Although the DEFINITION_BLOCK is meant to be a singleton, and there should be only one instance of it (my_game), it would be a good idea to add to the nested-locations initialization block an IF this = my_game statement, to ensure that the nested-locations code is only run on my_game if, for whatever reason, there are other instances of the DEFINITION_BLOCK class.

I positively remember that, at one point, the StdLib contained multiple instances of DEFINITION_BLOCK, e.g. the Banner, which was later fixed to an instance of location. I also suspect that due to the Banner being a DEFINITION_BLOCK, the strategy of overriding verbs on my_game wasn't working, because I remember that I did try and it failed at some point in time.

The problem with multiple DEFINITION_BLOCK instances lies in the nested-locations initialization: whichever instance is declared last would then become the location in which all other locations are ultimately nested, because the initialization loops basically override their previous outcomes. Hence, adding a sanity check to ensure that the loops are only executed if the current instance is my_game would be an extra precaution to ensure that this feature doesn't break up if end users accidentally create multiple instances of DEFINITION_BLOCK.

[AnssiR66]

Yes, that's fine, and sensible. This shouldn't be too hard to implement?

[tajmone]

This shouldn't be too hard to implement?

It shouldn't — assuming it's indeed possible (haven't tried it, so I'm not 100% sure that ALAN would actually allow an IF statement referring to a specific instance from within a class definition.

But since we're dealing with the core class around which the whole library revolves, I want to create some dedicated tests before tweaking any code. I was thinking of adding a new tests/integrity/ folder with test adventures that will check that specific aspects of the library are as expected, in order to catch immediately any undesired side effects.

In this case, I'd like the adventure to list every location and its nesting status, and then add a second DEFINITION_BLOCK instance to see whether it interferes with verbs overridden on my_game. This way, we can first confirm the problem existence, then we confirm that the fix works, and finally we'll have an integrity check that will detect if the original problem (or another one, related to my_game) ever comes back again.

I'd rather be over-cautious than sorry when it comes to tweaking core features.

[tajmone]

Problem with indoor and outdoor

I was carrying out some tests on how nested locations are being initialized, and I bumped into a problem which deserves attention.

Suppose an author wants to create different game regions, e.g. "The Folks Town" and the "Sacred City", where different rules apply (e.g. you can't carry out some actions in the Sacred City, etc.). He would so by creating these regions as locations where he'd place the various rooms and sites of each region, and then apply some region-wide CHECKs to prevent some actions, etc.

The problem is that he'd then write something like:

The Hostel IsA room at FolksTown.
End the.

The 'Holy Chamber' IsA room at SacredCity.
End the.

When the DEFINITION_BLOCK is initialized we have a problem: the code correctly ensures that every room is moved at indoor (otherwise the room props won't work correctly), but the indoor can't be both AT FolksTown and AT SacredCity! so the initialization code will simply end up locating the indoor at the last region iterated through (i.e. it will first locate it in one, then in the other).

This is a design problem for which there's no solution! So I suggest that:

• [ ] We make this very clear in the documentation — i.e. that the special library locations ROOM, SITE, DARK_ROOM and DARK_SITE can't be nested in custom locations by the author.
• [x] Tweak the initialization code to also move any DARK_ROOM and DARK_SITE AT indoor and AT outdoor, respectively, since they follow the same basic rules as ROOM and SITE.
• [ ] We could also add some sanity checks at the initialization code, to inform authors of potential problems if a ROOM, SITE, DARK_ROOM or DARK_SITE instance is found not to be AT indoor or AT outdoor.

I'm not entirely sure about point 3 though. Maybe the sanity checks could be done via an extra "test module"? or would it be better to incorporate such checks directly in the library, so that errors are caught immediately?

Either way, it seems that if authors want to use rooms and sites in different ad hoc regions then they'll have to copy the source code and create different copies of these classes for each region — but that's beyond our control.

Alternative Approach to Room/Site Props?

I wonder if there was a better solution to the room/site approach, i.e. one that doesn't limit authors' freedom of dividing their game map into regions. The goal of the indoor and outdoor locations is just to bring into scope the various props like ceiling, floor, etc. An alternative way to achieve this (i.e. without having multiple instances of these props) could be to exploit the Entered statement on indoor/outdoor to move these object to the current location when the Hero enters. The only possible downside would be that they might behave differently since they'd end being not in an outer location but in the location of the Hero (which affects descriptions), but this could be worked around by locating them into the my_game location instead, which is always an outer location.

Using Entered to move in and out room and props from the current location wouldn't be an unusual technique in ALAN either, since it's often used to simulate a door being in two connected rooms, so there's no reason it couldn't be used to improve the current room/site system. Also, this might solve the current problem with verbs on room/site props not working as expected (see #138).

What to do for v2.2.0

Probably these fixes should be postponed to a next library release, otherwise we'll never reach v2.2.0, but we definitely need to document the current problems regarding the use of rooms and sites in custom regions. Once v2.2.0 is published, we can work on these improvements.

What do you think of these problems and solutions? Am I missing out something or do they make sense?

Anyhow, I'll now carry on creating the integrity tests without using custom regions for rooms and site (only for custom locations), so we can verify that the code is working correctly, and then try to fix the inconsistencies found and check that results are identical, and finally try to optimize it without breaking anything.

[tajmone]

Annotated Initialization Code

I've slightly tweaked the initialization code that handles nested locations, so it now also move into indoor/outdoor instance of DARK_ROOM and DARK_SITE, for consistency sake.

But I've also added annotated comments with the various problems that we spoke about here, plus others I've just noticed. Probably seeing the comments next to the actual code makes it easier to track the problems. For a quick view of the updated code, see this link:

• lib_definitions.i

As you'll see in my notes, I'm a bit skeptical about the order of the last two operations, since first it forces all rooms and sites into indoor/outdoor, but then restores the original nesting order through the nested attribute track record of step 1 — which basically can potentially undo the previous step.

It probably makes more sense to enforce the indoor/outdoor relocation as a last step instead. But there are still other aspects of the original code which seem suspicious, or I simply don't understand what they are trying to do — i.e. are they just safety measures?

Maybe if you could have a look at it, and then tell me what each step should accomplish, I could then devise another test, to check edge cases and how they are handled — but unless I fully understand what each safety guard does I can't test against it.

I think this initialization code is very important, since it's at the heart of the library and could either break some library features, or clash with some author devices, either way breaking an adventure. So we must be both firm and cautious about these steps, and make sure everything is working and well documented.

In the meantime, I want to carry out some local experiments, just to check how the alternative approach of exploiting Entered to move room and site props on the my_game instance would work, and if it solves some of the current problems. If it's straight forward to implement, and easy to test, we might just go for it, since it would be a good improvement.

[AnssiR66]

Ok. I will paste the code here and explain what I intended to achieve with every individual snippet, hope it helps with tackling with the problem:

1)

FOR EACH l IsA LOCATION
  DO
    EXCLUDE nowhere FROM nested OF l.
    IF COUNT IsA LOCATION, AT l > 0
      THEN
        FOR EACH x IsA LOCATION, AT l
          DO
            INCLUDE x IN nested OF l.
        END FOR.
    END IF.
END FOR.

Ok, first, "Nowhere", used as a dummy default for the "nested" set, is removed from the nested set of any location. But this was probably alright and no problem.

Next, each location that is declared by the author to be "at" another location, i.e. nested in another location, will also be included in the specific library-defined "nested" set of that mother location. This "double feature" is to ensure that nesting works ok when locations are "moved around" some lines later in the library code.

2)

FOR EACH l IsA LOCATION
  DO
    IF l <> my_game AND l <> nowhere
      THEN LOCATE l AT my_game.
    END IF.
END FOR.

Here, all locations are located at the my_game instance - straight-forward enough.

3)

FOR EACH r1 IsA ROOM
  DO
    LOCATE r1 AT indoor.
END FOR.

FOR EACH s1 IsA SITE
  DO
    LOCATE s1 AT outdoor.
END FOR.

This is also straight-forward enough. Now, all rooms are at "indoor" and sited at "outdoor".

4)

FOR EACH l IsA LOCATION
  DO
    IF nested OF l <> {} AND l <> my_game AND l <> nowhere
    THEN
      FOR EACH x IsA LOCATION, IN nested OF l
        DO
          IF l <> my_game AND x <> my_game
            THEN LOCATE x AT l.
          END IF.
      END FOR.
    END IF.
END FOR.

Here, if a location (other than my_game and nowhere) is a "mother location", i.e. has nested locations, then each of these nested locations is again located at the mother location (if they indeed ever were placed somewhere else at (3) above; I am not sure about this.) So, this is more like a way of ensuring that nested locations still are at the mother location. There is a passage in the manual that advises that if the nested locations are for example "Rooms", then the mother location (e.g. a house) should be also declared a Room, so that everything works ok.

I am not also sure if I follow completely why in the above example, you say that "indoor" is first located at FolksTown and then in SacredCity. Isn't it so that these two locations are rather located in "indoor", than vice versa? I mean, if these regions contain rooms, they should also be declared as "rooms", according to the manual. If these regions contain both indoor areas and outdoor areas, then a more specific division into various kinds of regions (indoor and outdoor) should be added, somewhere in the middle. (SacredCityIndoor ISA ROOM IN SacredCity, or the like).

[tajmone]

Ok. I will paste the code here and explain what I intended to achieve with every individual snippet, hope it helps with tackling with the problem:

Thanks, this helps a lot. The important think here is to:

1. Ensure that we document well each step, for the future.
2. Avoid tweaking any odd looking piece of code that might effectively have been added to fix some hard-to-catch bug in the course of time (the fact it looks odd doesn't mean it's not correct, but without comments explaining it it's hard to tell).

Please, also refer to the updated initialization code, which does the same things but contains annotations:

• lib_definitions.i

Step 1

Ok, first, "Nowhere", used as a dummy default for the "nested" set, is removed from the nested set of any location. But this was probably alright and no problem.

I'm just wondering why we should expect that the nowhere location might have ended up being nested into another location. This seems more of safe-guard, i.e. to prevent things breaking; if that's the case, then we should we annotate this in comments.

But this also brings up the question of whether the library should just silently revert intentional changes by the author — i.e. if the author decided to nest the nowhere into another location he might have some reasons to do so, and in any case shouldn't we warn him that in the actual games the world state has been changed?

Furthermore, while I agree that the nowhere location should not be nested in any other locations, for this could lead to broken mechanics, the same also applies to the my_game instance (if not even more so), since the whole purpose of my_game is being the outermost location, in order to override any library verbs. So, if we keep the nowhere safeguard, I suggest we also add one for my_game and document them both.

Next, each location that is declared by the author to be "at" another location, i.e. nested in another location, will also be included in the specific library-defined "nested" set of that mother location. This "double feature" is to ensure that nesting works ok when locations are "moved around" some lines later in the library code.

But here I see a problem with the actual code:

        FOR EACH x IsA LOCATION, AT l
          DO
            INCLUDE x IN nested OF l.
        END FOR.

which I think should instead be:

        FOR EACH x IsA LOCATION, DIRECTLY AT l
                                 ^^^^^^^^

otherwise we'll end up moving around too many locations, since in a case where location C is nested into B, and B into A, it's also true that C is AT A — because it's INDIRECTLY contained in it. I say that this iterator should only focus on locations directly into each other, which is safer and quicker. Furthermore, locations can only be AT another location (i.e. they can't be contained into THINGS) so there's no risk of skipping any nested location this way.

So, I'd like to tweak this, and the new integrity test should already be able to confirm that the code works as before.

Step 2

Here, all locations are located at the my_game instance - straight-forward enough.

Yes, this part look perfectly fine, agreed.

Step 3

This is also straight-forward enough. Now, all rooms are at "indoor" and sited at "outdoor".

This is actually problematic IMO, because Step 4 will undo any changes done by this step. If for some reason a room instance was placed by the author in a custom location (other than indoor) then this would have been tracked in the nested Set of the custom location, at Step 1, and will thus be restored to it's original (wrong) location at the next step (Step 4).

I'll add an actual test for this, providing a bad example where the author nests a room instance at a custom location, and see how the initialization goes.

If I've understood correctly, the goal of this step it to enforce every room and site being at indoor and outdoor respectively. If that's the case, we should probably add this as the last step, to be on the safe side.

Again, this is another safeguard, which silently undoes what the author intended. Even though the author is wrong in doing this, if we don't warn him he might end up running in circles trying to catch a bug that it's not there, since the problem is just that the library has changed the game-world without informing him — so if he keeps focusing on his adventure code, he'll never find out why the game is not as he expects, since the code that changes things is in the library sources, not in his own code.

I think that silently undoing authors' choices is a bad practice. Instead, we should inform authors about what they should and shouldn't do via the documentation, and let them be responsible for their own mistakes, without interfering in their code choices. After all, there are literally hundreds of ways an author can break the library or his adventure, and we can't possibly provide safeguards for each and every case, so probably the best approach is good documentation and not interfering with authors' choices, so they can discover their own bugs by looking at their own code — if the bug is caused by the library, chances are they will never find it (the StdLib code is huge, and quite complex to grasp for third parties who didn't study it).

Step 4

Here, if a location (other than my_game and nowhere) is a "mother location", i.e. has nested locations, then each of these nested locations is again located at the mother location (if they indeed ever were placed somewhere else at (3) above; I am not sure about this.)

I agree it's OK to skip my_game, but why nowhere? I personally use very often the trick of creating a custom location just to have some custom variables, and I usually place this fake-location AT the nowhere, just to ensure is out of reach to the player. So this practice is not only harmless (to the game) but actually useful, and (again) I see no reason for undoing author choices here. I might even have used this trick in the StdLib code, to store some new locations needed for some vars (not sure, I need to check), but indeed if the StdLib does define other custom locations for such hacks (other than my_game of course) then they ought to be located AT nowhere IMO.

Furthermore, unless we move the previous step to last position, Step 4 will undo all the safeguards of rooms and sites (i.e. if we need to keep them at all).

So, this is more like a way of ensuring that nested locations still are at the mother location. There is a passage in the manual that advises that if the nested locations are for example "Rooms", then the mother location (e.g. a house) should be also declared a Room, so that everything works ok.

I agree on this, no question. It's just that I think it's best to tweak Step 1 to act only on locations DIRECTLY nested into each other, and allow restoring locations nested into the nowhere too.

I am not also sure if I follow completely why in the above example, you say that "indoor" is first located at FolksTown and then in SacredCity. Isn't it so that these two locations are rather located in "indoor", than vice versa?

No, the problem is that in this case the author would be forgetting that by creating multiple separate regions, each containing rooms/sites, would result in this initialization code moving around the indoor and outdoor first in one region, then in another. Since Alan doesn't support polymorphism, each location instance can be at one location at the time, so it would be impossible for indoor to be both at FolksTown and SacredCity, it will ultimately just end up being in whichever region is iterated last.

I mean, if these regions contain rooms, they should also be declared as "rooms", according to the manual. If these regions contain both indoor areas and outdoor areas, then a more specific division into various kinds of regions (indoor and outdoor) should be added, somewhere in the middle. (SacredCityIndoor ISA ROOM IN SacredCity, or the like).

The problem is that currently the StdLib Manual doesn't warn about this, and also the initialization code silently breaks things without the author being made aware of this. Surely, we can't add safeguards for this case, since it's impossible to fix. I don't think it's possible to create some code that is able to check the integrity of such cases, because it would be hard to track subclasses of room and site. My best guess is that this point needs to be well documented in the chapter on Rooms and Sites (and Dark_Room and Dark_Sites).

Hence my idea of using a different approach to handle room/site props, by using the Entered statement on these classes to move around the props by making them available on my_class, which we know is going to be the outermost region during the game. I haven't tested this yet (e.g. we also need to move them out of my_game when the Hero leaves the room/site, and ensure they work even when the Hero is moved around by adventure locate statements, and other details).

[AnssiR66]

Some quick responses here:

I'm just wondering why we should expect that the nowhere location might have ended up being nested into another location. This seems more of safe-guard, i.e. to prevent things breaking; if that's the case, then we should we annotate this in comments.

The nowhere instance is removed because of the code in lib_definitions.i:

-- An attribute for keeping track of nested locations; used internally in the library (ignore).

ADD TO EVERY LOCATION HAS nested {nowhere}. END ADD TO.

So, the library nests 'nowhere' in every location initially. This is just to define that the nested set takes members of the class "location"; the ALAN parser needs to know what kind of members a set must take, and by having 'nowhere' as a default dummy member, which is then immediately removed, the parser knows that the nested set can/should only accept locations, and not for example objects. If we didn't have the above code in the library, there would be a parsing error amounting to something that "Members of the set 'nested' not defined" or something to that effect.

Yes, DIRECTLY would be needed at the place you pointed out, well spotted. That should be added.

The problem is that currently the StdLib Manual doesn't warn about this [that rooms should be nested only in rooms, sites only in sites], and also the initialization code silently breaks things without the author being made aware of this.

The manual originally does warn about this, this passage is in the nested locations chapter (p. 21 of the Manual v2.1., I am not sure about the most current version). If the author decides to code against this advice, then I originally took the stance it should be on the author's responsibility, and the library shouldn't be "responsible" or cater for coding of that kind.

[tajmone]

The nowhere instance is removed because of the code in lib_definitions.i:

ADD TO EVERY LOCATION
  HAS nested {nowhere}.
END ADD TO.

Good point! I forgot about it. We then just need to add a comment here, explaining why this is the case. (we should comment everything, because the codebase has grown huge, and sometimes even us struggle keeping up with all its minutiae).

Yes, DIRECTLY would be needed at the place you pointed out, well spotted. That should be added.

Ok, I'll do it, but first I want to make sure that we have solid tests coverage, so I don't accidentally break anything here (better cautious than sorry).

The manual originally does warn about this, this passage is in the nested locations chapter (p. 21 of the Manual v2.1., I am not sure about the most current version). If the author decides to code against this advice, then I originally took the stance it should be on the author's responsibility, and the library shouldn't be "responsible" or cater for coding of that kind.

Thanks for this! I'll then try to dig it out — if it was in the original it's still there, it's just that I didn't work on that part. But I think we should definitely add a warning note (with a link to this section) in all the sections on ROOM/DARK_ROOM and SITE/DARK_SITE, just to make sure it doesn't go unnoticed.

Personally, I think it's a bad limitation though, since custom regions are something that most authors will like to use in mid- to big-sized adventures, especially the Fantasy genre. Having to reduplicate all the rooms/sites classes for each region is a bit overkill IMO. If the alternative approach works it would be great, but that needs testing.

I think that in general, the less a library enforces predefined structure regarding locations hierarchy, the better it is, and if the same feature can be achieved via classes, attributes or some other non-invasive construct, than it's worth taking that path. The same proved true with the worn entity for storing Hero's worn items: the additional structure introduced bugs and limitations, and somehow went against the grain of idiomatic ALAN, especially in terms of third party libraries and extensions.

Anyhow I'll look into it!

Fixed Rooms/Sites Init!

Anssi, I've added a test to check whether Step 3 was being undone by Step 4, and indeed that was the case. So I've now inverted them, and Rooms/Sites relocation to indoor/outdoor is now done as the last step and works correctly — see commit 41c0d6a481e1d90.

[AnssiR66]

Great that you were able to fix the room/Sites initiation by changing the order of the code lines! Good thinking, and something that was hard for me to even notice in the first place.

[tajmone]

Now I think that all the problematic code has been dealt with.

There are still a couple of things that I think should be addressed though.

Excluding Dummy Preset

The line that removes the nowhere dummy placeholder at the beginning of the process:

EXCLUDE nowhere FROM nested OF l.

I think it should actually empty the whole set, not just remove the nowhere. First of all, emptying the set is safer, just in case the author tampered with the nested attribute set (which is supposed to be used only internally by the library). Second, emptying the set would render the code clearer to understand IMO, since the goal of this initialization block is to use the nested attribute for itself.

Dubious Final Step

Then there is the final step, which restores all locations to their original nesting place:

    FOR EACH l ISA LOCATION
      DO
        IF nested OF l <> {} AND l <> my_game AND l <> nowhere
        THEN
          FOR EACH x ISA LOCATION, IN nested OF l
            DO
              IF l <> my_game AND x <> my_game
                THEN LOCATE x AT l.
              END IF.
          END FOR.
        END IF.
    END FOR.

Where there's the already mentioned redundant check for l <> my_game in:

              IF l <> my_game AND x <> my_game

and, most important, I strongly believe that we should not exclude locations nested in nowhere from this process! As mentioned above, it makes perfectly sense that dummy location should be located AT the nowhere, and in some complex adventure authors might wish to park there also some switchable locations (e.g. dark vs lit location), e.g. in case they implement some verbs can target all map locations via ! (e.g. to implement quick travel), in which case such a verb might wish to exclude the nowere and any location nested therein (via conditional code).

I'd also like to keep all library internal objects and location at nowhere (except my_game, of course), for better separation of what's what in the codebase; and honestly, I see no reason to override any location nesting at nowhere that an author might have chosen to use — we should respect such choices, and if these break things up it's not our concern, really.

---

Once these last two points are agreed upon this Issue should be done with.

Chances are that we won't add tests coverage for these last two cases (besides some basic integrity checks, e.g. if we move into nowhere library locations) since we're dealing mostly with potential use cases that we'd rather not interfere with (as the current code does).