AnssiR66 / AlanStdLib

The Standard Library for ALAN Interactive Fiction Language
Other
5 stars 2 forks source link

Unlit Location Bug + Improvements to Dark Locations #96

Closed tajmone closed 3 years ago

tajmone commented 3 years ago

@AnssiR66, I've realized there was a bug with the location library class: even if the the location IS NOT lit the default DESCRIPTION would show, instead of the "It's pitch dark..." message.

I've already fixed it locally, I only needed to add the same DESCRIPTION CHECK as found in DARK_LOCATION:

ADD TO EVERY LOCATION
  IS lit.

  DESCRIPTION                            --> Added fix!
    CHECK THIS IS lit                    --> (same as for DARK_LOCATION)
      ELSE SAY dark_loc_desc OF my_game. -->
END ADD TO.

It's clear from the Manual that this is supposed to be the default behavior (just as verbs that require light are also suppressed in unlit locations), so I just went ahead and fixed it.

What I'm not sure about is whether we also need to add some checks for the ENTERED clause, like there are in DARK_LOCATION:

EVERY dark_location ISA LOCATION
  IS NOT lit.

  ENTERED

    IF COUNT ISA LIGHTSOURCE, IS lit, HERE > 0
      THEN MAKE THIS lit.
        IF CURRENT ACTOR <> hero
          THEN LOOK.
        END IF.
    END IF.

From the above, it's clear that if the runtime context of ENTERED is an NPC (scripted, or otherwise dislocated at current location) then a LOOK statement is executed _if the DARKLOCATION has a light source — should something similar also apply to locations, checking if it's lit and IF CURRENT ACTOR <> hero???

AnssiR66 commented 3 years ago

Thanks for noticing and fixing that. Now, normal Locations and Dark_locations differ in the way that normal locations don't need a Lightsource to be lit (as explained in the manual). However, it would be illogical if a "Not lit" normal location would not turn lit when someone enters with a Lightsource. So adding also the Entered statement, like you suggested, would speak for it. That, however, brings the question - do we need the Dark_location class at all, if we can just add all of that code to the location class level already? Well, yes, we do, because otherwise all normal locations would always need a Lightsource to be present to be lit. So, please, add the Entered statement to the location level as well and if tests prove that everything works, then fine...

tajmone commented 3 years ago

However, it would be illogical if a "Not lit" normal location would not turn lit when someone enters with a Lightsource. So adding also the Entered statement, like you suggested, would speak for it.

In this case, we also need to look at the various EVENTS associated with the lit attribute, because I think they only address DARK_LOCATIONs.

But, in the above library snippet, why does the ENTERED clause only perform a LOOK if an NPC has entered the DARK_LOCATION (and if there's a lightsource)? I don't quite understand the purpose of that LOOK.

That, however, brings the question - do we need the Dark_location class at all, if we can just add all of that code to the location class level already? Well, yes, we do, because otherwise all normal locations would always need a Lightsource to be present to be lit. So, please, add the Entered statement to the location level as well and if tests prove that everything works, then fine...

That's something I was wondering about myself too. While reading and editing the sections on locations I'm trying to figure out all the possible implications of the differences and similarities between unlit locations and DARK_LOCATIONs, in order to cover them fully in the revised text.

I guess that the best approach right now is to add to the test suite some stress tests for both unlit locations and DARK_LOCATIONs, trying out all possible combinations of light sources being carried by the Hero and NPCs that come and go from both, and see the results.

As for the logic that a NOT lit location should also be lit by the presence of a lightsource, it might not be necessarily true from a game implementation point of view, where the author wants to ensure that the specific location only becomes lit via whatever game mechanics he/she has designed in order for that to happen — e.g. finding a secret switch that turns on the lights, etc. In these cases, the author might need reassurance that the player won't be able to circumvent the story mechanics by an escamotage (e.g. bringing into the unlit-room puzzle a lightsource designed for a DARK_LOCATION from elsewhere in the map). So, from a designer's perspective, the difference between the two type of implementable dark places makes sense.

Also, I believe that while any location that becomes dark should indeed behave accordingly, the opposite doesn't always hold true — e.g. a candle shouldn't be able to light up a stadium. Since light sources don't have attributes defining degrees of light intensity, if lightsource objects could illuminate any unlit location we might face problems in many contexts. Since the lit attribute is implemented directly on the location core class, I wouldn't go beyond ensuring that a dark location behaves accordingly — the DARK_LOCATION is the viable alternative for doing so, which authors are free to tweak or sub-class if they need special cases.

AnssiR66 commented 3 years ago

The reason the Look applies only to NPCs in the code snipped above: when the hero enters, there is an automatic "Look". That's why, if we don't restrict the Look in the code snippet to apply to NPCs only, the location description would be shown twice when the hero enters with a lightsource. I didn't test that thoroughly, though, but that's the way I figured it would work.

tajmone commented 3 years ago

@AnssiR66:

if we don't restrict the Look in the code snippet to apply to NPCs only, the location description would be shown twice when the hero enters with a lightsource. I didn't test that thoroughly, though, but that's the way I figured it would work.

Thanks for the explanation. I wasn't sure if (and why) the hero's entrance is being handled elsewhere, but now it makes sense. Of course, I'll also add tests for this in the test suite, just to ensure that things are working as expected, and they won't break in the future.

New Intro on Unlit Location vs DARK_LOCATION

I'd like you to have a look at the introductory section on "Dark Places" which I just wrote:

I'm not sure whether my conclusions match your original intentions for the creation of a dedicated DARK_LOCATION class (or, indeed, whether they are correct or just my misunderstanding), but the natural conclusion I came to is that, ultimately, the main difference between a NOT lit location and a DARK_LOCATION instance is (for all practical purposes) a matter of who's in control of the location's lighting mechanism — the author or the player?

I'd like your feedback on my introduction, and whether it's correct, in some way relevant, or whether it might just be adding confusion to the topic.

(PS: after this post I'll have to leave the PC for the rest of the morning)

tajmone commented 3 years ago

New Dark Places Test

@AnssiR66, I've added to the test suite a new adventure to test all possible contexts with dark places:

Locations Bug

As you can see from the test transcript, there's a bug wit unlit locations:

Since I'm not sure whether normal locations (i.e. non-DARK_LOCATION) should become lit or not by the presence of a light source, I didn't try to fix the bug nor create a longer test transcript.

You can compile dark-places.alan and quickly test manually the similarities and differences between unlit locations and DARK_LOCATION instances — you can invoke and dismiss NPCs with and without a light source in their inventory, to test how their entrance and exit affect both type of dark places; as well as having an electric torch in the Hero's inventory.

Having played around with the test myself, I think that the problem is that some darkness related events in lib_locations.i are currently affecting only DARK_LOCATION instances. So, before attempting any fix, we need to decide what the behavior on normal unlit locations should be in relation to lightsource object — should they ignore these entirely?

It might not be an easy choice, either way. If we make both locations types behave identically, the whole purpose of having a DARK_LOCATION class vanishes. On the other hand, if normal locations are indifferent to light sources, it could be confusing for both the player as well as authors.

What should we do about it?

AnssiR66 commented 3 years ago

Yes, there are some problems there. The main point is that normal locations indeed are indifferent to light sources. Normal locations are lit by default and that's why dark_locations are needed separately - only a lightsource object can make them lit. The "(not) lit" attribute, however, was defined for all location instances in the library, for the reasons stated in the library manual - to make for example several locations lit at once by flipping a main switch, etc. This should be a choice that the author makes - if the author means that only a light source can light the room, then he must use dark_locations, but if it is meant that a (remote) switch can make a room lit, then the "(not) lit" attribute for normal locations should be used. But, from the test example you provided, these can sometimes clash, and the author might not always be aware that this happens. So, the library should ideally cater for these situations. However, it is not possible in the library level (at least I don't see how it would be possible) to track whether a normal room was originally unlit or lit and then when the hero visits it with a lightsource and leaves again, to restore it to its former state. This would be something that the game author needs to check in his own code, if he insists that he uses merely the not lit attribute, and not dark_locations, when lightsources are involved. I would say this is just something that needs to be pointed out in the manual -dark_locations and lightsources go together, using merely not_lit should be reserved for situations when no lightsource object defines whether the room is lit or not. I seem to remember this was already pointed out in the manual?

tajmone commented 3 years ago

Yes, there are some problems there.

After the v2.2.0 release we must start working on a StdLib Design Doc to explain all the design goals, choices and implementation details, and the check via the test suite that all features are implemented coherently. The library has grown huge and it's becoming difficult to manually track its implementation detail, and since every new small change could have a myriad of unexpected consequences, we need both a solid tests foundation and a carefully documented design specs reference.

The main point is that normal locations indeed are indifferent to light sources.

Fixing normal locations should be fairly easy, all the code handling that should be in the events inside lib_locations.i (maybe also in some lib_classes.iverbs).

Normal locations are lit by default and that's why dark_locations are needed separately - only a lightsource object can make them lit. [...] I would say this is just something that needs to be pointed out in the manual -dark_locations and lightsources go together, using merely not_lit should be reserved for situations when no lightsource object defines whether the room is lit or not. I seem to remember this was already pointed out in the manual?

This last statement is a bit confusing — and the Manual doesn't really clarify this point. But the confusion I'm seeing is more on a practical level, as the following example will clarify.

Since the lit attribute can only be controlled by the author, shouldn't he be free to also implement other means of setting the lit attributing back on? E.g. imagine a forest which is lit in daytime and dark at night, the player will need a light source to illuminate forest locations at night, but the game should automatically illuminate them when daytime is due. Of course, being a forest, there wouldn't be any other means for the player to illuminate it except light sources (no light switches in nature).

We need to decide on this last point, and also check whether the current darkness events would actually interfere with this.

So, the library should ideally cater for these situations. However, it is not possible in the library level (at least I don't see how it would be possible) to track whether a normal room was originally unlit or lit and then when the hero visits it with a lightsource and leaves again, to restore it to its former state.

I don't see why not, the events in lib_locations.it are taking care of this. The current problem is that some of them act on all locations, while others only carry out checks for dark_location instances:

WHEN location OF hero IS NOT lit          <-- This works on *ANY* location!
  AND COUNT ISA lightsource, IS lit, AT hero > 0
THEN MAKE location OF hero lit.
  SCHEDULE light_on AT hero AFTER 0.

EVENT light_on
  LOOK.
END EVENT.

WHEN location OF hero ISA dark_location  <-- This works on DARK_LOCATIONs only!
  AND location OF hero IS lit
  AND COUNT ISA lightsource, IS lit, AT hero = 0
THEN MAKE location OF hero NOT lit.
  SCHEDULE light_off AT hero AFTER 0.

EVENT light_off
  SAY light_goes_off OF my_game.
END EVENT.

-- We need to ensure that a dark_locations will become dark again after the hero
-- leaves, if her took the lightsource with him:

EVENT check_darkness
  FOR EACH dl ISA dark_location, IS lit
  DO
    IF COUNT ISA LIGHTSOURCE, AT dl = 0
      THEN MAKE dl NOT lit.
    END IF.
  END FOR.
  SCHEDULE check_darkness AFTER 1.
END EVENT.

Depending on what we decide, we'll have to either extend those events to any location (if we decide to allow light source to illuminate normal locations too) or make all events specific to DARK_LOCATION.

You're right though, it seems like these events would take over any author attempt to light a DARK_LOCATION by other means, for they'd unlit automatically if a light source is not present.

A final thought on DARK_LOCATION ... I can't avoid noticing that using this class deprives the author of the benefits of using a ROOM or SITE (having walls, ground, etc.). Wouldn't it make sense to create sub-classes of DARK_LOCATION: DARK_ROOM and DARK_SITE that would also come equipped with room- and site-object? At least this wouldn't break consistency in the adventure design style.

I have to admit that dark locations is an aspect of the StdLib with which I've struggled quite a lot during the Italian translation, never quite grasping the original intentions of this features and its implementation details and limits.

tajmone commented 3 years ago

Possible Solution

Here's a proposal to fix the unlit-location vs DARK_LOCATION issues:

This will effectively mean that normal locations can only by illuminated or turned dark by manipulating the lit attributes.

As for DARK_LOCATIONs:

As for the forest example mentioned above, the problem is that these EVENTs would automatically make dark again a DARK_LOCATION if the author has set it lit but there's no light source present.

There's a an easy fix to this: adding to DARK_LOCATION a new attribute, IS NOT force_lit, and then tweak the EVENTs that are in charge of making the location dark again to skip the action if the location is forced-lit:

WHEN location OF hero ISA dark_location
  AND location OF hero IS lit
  AND location OF hero IS NOT forced_lit            <-- the new check!
  AND COUNT ISA lightsource, IS lit, AT hero = 0
THEN MAKE location OF hero NOT lit.
  SCHEDULE light_off AT hero AFTER 0.

(and so on with other similar checks elsewhere).

This way, the author could implement the day-and-night forest and prevent it from becoming dark during the day (if there are no light-source present) simply by setting the forest to IS lit. IS forced_lit when it's daytime, and then to IS NOT lit. IS NOT forced_lit during night-time.

This is a simple workaround, and it doesn't break the main-use of the class (which is supposed to take over any manually lit settings), but allows authors to easily implement exceptions to the rule via a simple attribute.

As for the DARK_ROOM and DARK_SITE, these should only require a single class declaration placing them AT indoor and AT outdoor, respectively — all the rest should work automatically, i.e. room- and site-objects. In the worst case, a few added line of code should fix any missed-out issue.

AnssiR66 commented 3 years ago

A solution to the day-time forest would also be to make the forest a dark_location, implement the sun, a lit lightsource object which is present in the forest when it's daytime, and then locate it to nowhere when it's nighttime. Then, regardless of what the hero carries, the forest stays lit in the daytime. Also, is there an actual need for dark_room and dark_site? Because we could instruct the author to implement instead "The basement Isa Dark_location AT Indoor" and "The night_forest Isa Dark_location At Outdoor", saving the trouble of implementing new classes. But your approach is maybe anyway preferable because "indoor" and "outdoor" are internal attributes in the library and not meant to be bothered by the game author.

tajmone commented 3 years ago

A solution to the day-time forest would also be to make the forest a dark_location, implement the sun, a lit lightsource object which is present in the forest when it's daytime, and then locate it to nowhere when it's nighttime. Then, regardless of what the hero carries, the forest stays lit in the daytime.

That's a good solution, didn't think of it. Unless there are other real case scenarios we aren't thinking about, this should be a fine approach in most circumstance (including a NOT natural light source, for lamp posts, etc., which can be lit and unlit as required).

Also, is there an actual need for dark_room and dark_site? Because we could instruct the author to implement instead The basement Isa Dark_location AT Indoor and The night_forest Isa Dark_location At Outdoor, saving the trouble of implementing new classes. But your approach is maybe anyway preferable because "indoor" and "outdoor" are internal attributes in the library and not meant to be bothered by the game author.

It's true that we could leave that to author, but the same argument could be used to justify implementing those two classes directly in library, for completeness sake — after all, is little work on our side too, but at least we can carry out extensive tests and guarantee that it works as expected. Also, classes by themselves don't consume any memory if there are no instances in the final game, so ... one more argument in favor of us implementing them.

I am in favor of providing symmetrical and consistent built-in classes. As far as the StdLib Manual goes, we won't even need to spend too much words for explaining dark_room and dark_site either, for we could just mention that they work like dark_location but come with all the room-/site-objects of room and site, so adding them to the StdLib doesn't imply too much extra work on the Manual from this point of view. I also believe that these will be fairly easy to understand and remember for authors, once they've learned how room, site and dark_location work.

tajmone commented 3 years ago

@AnssiR66:

A solution to the day-time forest would also be to make the forest a dark_location, implement the sun, a lit lightsource object which is present in the forest when it's daytime

Actually, after thinking again about the above proposed solution I'm having second thoughts, and think that adding the force_lit attribute would be better for a number of reasons:

  1. Flipping the lit attribute is something that is totally under the author's control, whereas creating a light_source object (e.g. the sun) is exposing the mechanism to the player's interactions, which introduces a number of risk factors and potential headaches:
    • The light source can be targeted by player verbs, which can lead to unexpected results, especially if the author adds new verbs that don't take proper care of protecting the light source; but also the library has turned out to contain some checks holes in some places for unexpected verbs interactions with objects with special attributes.
    • Adding a "sun" light-source would entail that the light_source specific verbs become active on the object (be it a natural or innatural light-source), whereas using the internal lit attribute dispenses from the need of adding a light source — the author might wish to add a "scenery/prop" sun for added realism, but it would not partake in the illumination mechanism.
    • Going beyond the forest example, the concept of daytime could apply globally to an adventure, involving both indoor and outdoor locations — e.g. the rooms of a castle should also be lit during the day, although the sun is not there. So it would be more practical to rely on the lit attribute, not specific light-source objects (and the Manual even advises this approach).
  2. My understanding is that it's a better design choice to keep entirely separate the use of the lit attribute and light_source objects, using the former when the author wants total control over the lit attribute (i.e. keeping the mechanism wholly internally), and using the latter when desiring to hand over control of a location's illumination to the player instead.

With hindsight, one of the most frequent causes of bugs in the library has been excessive reliance on the fact that verbs would be properly checking and setting the various attributes, but it turned out to be rather difficult to put in practice as the scale of the library grew (e.g., see all the issues that popped up with the old clothing system, were it was possible to move around worn clothes, with the result that some clothing items' attributes no longer matched their worn/unwarned status).

In this specific case, we have only to worry about a single event that might clear the lit attribute of a dark_location (at every turn) is there are no light sources in the location. Adopting the force_lit attribute would allow us to control this specific behavior in a single place (event) in the library code, drastically reducing the chances of unexpected results due to attributes being mishandled by verbs. IMO, this is the cleaner solution, and it should be preferable to using a light source object — even if the latter approach is doable, we should be advocating safe solutions, not only internally to the library, but also on the authors' side.

AnssiR66 commented 3 years ago

Ok! We can go ahead with the force_lit attribute.

tajmone commented 3 years ago

Bug Fixes and Consistent Behavior

@AnssiR66, right now I've enforced that normal locations are unaffected by light sources, and fixed some more bugs that I've encountered:

You can see the current test results here:

Before adding the force_lit attribute I wanted to first ensure that the core behaviors are all in place and working correctly, and covered by the test suite — and there are still a few pending questions (see below).

A Few More Considerations

Missing LOOK and Lights-Off Message in Normal Locations

I've also noticed that (unlike for DARK_LOCATION) with normal locations:

  1. When the light comes on in a NOT lit location, there's no automatic LOOK execution
  2. Likewise, when the light goes off you don't see the light_goes_off message either.

(see the test transcript for details)

Is this supposed to be the case? or should locations behave like DARK_LOCATION in this respect?

Background Checks for DARK_LOCATIONs Other Than the Hero's

Another thing worth mentioning is that the darkness RULEs and EVENTs only really take care of ensuring the proper status of the DARK_LOCATION were the Hero is — i.e. they don't ensure that the lit state of other locations is properly updated based on the presence or absence of light source.

I was thinking that maybe end users might implement scripted actors that might to check if the NPCs DARK_LOCATION is lit or not, in order to decide the course of action. So it might be worth ensuring that the library also caters for locations other than the hero's, just in case authors rely on the accurateness of an NPCs's location (in scripts, rules, or whatever mechanism)

AnssiR66 commented 3 years ago

Based on these observations, I was thinking: would it be anyway better if we don't extend the "lit" attribute to normal locations at all? Only dark_locations would be lit/unlit, checked for this, have messages for getting lit, getting dark, etc. That would make things clearer and simpler. If we for example need to make several location lit at once, through a main switch or such, the game author can nest these dark_locations in a mother_location that has one lightsource (invisible to the player) that then lights all those locations. And flipping a switch at the top of the stairs makes a (visible or invisible to the player) lightsource in the basement lit. The day/night cycle in a forest would depend on the sun instance working as a lightsource (and again, the sun could be in more than one location if all needed locations are nested in a mother location where the sun is present). Globally (= in all locations of the game), the author could just have "THE sun ISA LIGHTSOURCE At my_game. Is lit." which would have the sun in all locations of the game (because all locations of an Alan game are always nested in my_game). What would you think?

AnssiR66 commented 3 years ago

Another thing worth mentioning is that the darkness RULEs and EVENTs only really take care of ensuring the proper status of the DARK_LOCATION were the Hero is — i.e. they don't ensure that the lit state of other locations is properly updated based on the presence or absence of light source.

Maybe this should be implemented. I don't think it's a big issue. For example "FOR EACH d Isa Dark_location DO IF SUM OF LIGHTSOURCE Is Lit, Here = 0 THEN MAKE d NOt lit. ELSE Make d lit. END EACH." (or something similar?) and then make this check happen every turn. I wonder why it was not already included in the library, but I must have overseen it.

tajmone commented 3 years ago

I'm not convinced about this. Attributes are in general a safer way to implement game features in ALAN, so switching the whole illumination system toward the usage of special class objects (lightsource) can more easily lead to design break-up and inconsistent behavior — an entanglement situation similar to that of the old clothing system, where all the set attributes and special containing instances ended up falling apart. Not to mention all the potential issues with multiple- and omnipotent-parameters verbs, which can be insidious to predict and handle, especially with hidden objects.

IMO, chances are that an author will mostly use the lit attribute and normal locations, and rely on dark_location and light sources only for special outdoor situations. Sure, the fact that a light source doesn't illuminate a normal location seems strange, but in case an author uses both of them in the same adventure he/she'll have to work out some tricks to make light sources unavailable in unlit locations (e.g. consuming them after they've played their part, or forbidding the hero from taking them out of the places where they belong), or alternatively provide some explanation why these don't illuminate an unlit location — e.g. by stating that the light is too feeble, that the light source magically goes off (batteries die out, etc).

I don't know if you're read my new Intro "Dark Places" in the Manual, were I state that the main difference in the two type of dark places lies in whether it's the author who controls the illumination system or it's the player who does — i.e. the lit attribute is more under the author's control, whereas light sources provide more freedom to the player (e.g. if there are multiple different source of light). The two system can result in quite different game designs, if taken to their extent, so I think it's a strong point of the library that it provides alternative ways to achieve this (even though there are some discrepancies if both are adopted at once).

I also think that light_source objects shouldn't be implemented beyond their original intention (a torch, a candle), because anything beyond that might expose the adventure to unexpected interactions with the player's verbs. When adding new verbs, classes, etc., it's so easy to miss out these potential side effects, and most authors will only test for the most obvious actions, at the risk of not covering senseless interactions (eat sun, pour water on sun, etc.) which could potentially break up the internal state of an adventure and make it unwinnable.

We've come across plenty of similar undetected issues in the course of the past years, so I'm strongly in favor of adopting simple and safe implementation routes whenever possible. The current locations darkness system might not be perfect for all cases, but if used as intended is fairly use to handle and robust, and we're able to stress-test it for its intended use.

tajmone commented 3 years ago

Maybe this should be implemented. I don't think it's a big issue. For example "FOR EACH d Isa Dark_location DO IF SUM OF LIGHTSOURCE Is Lit, Here = 0 THEN MAKE d NOt lit. ELSE Make d lit. END EACH." (or something similar?) and then make this check happen every turn. I wonder why it was not already included in the library, but I must have overseen it.

The problem is ensuring that the different RULEs and EVENTs don't cross each other's path, leading to multiple triggers of the lights on/off event within a turn. We should try to keep a single EVENT (or as few as possible) to handle all these, and test them rigorously. In theory we shouldn't encounter multiple triggers, because ALAN adopts "flanked triggers" to ensure that an expression is evaluated just once per turn for each status change, unless we really do flip states accidentally multiple times during a turn.

A bit harder might be the case of handling changes in darkness status in normal locations (since every DARK_LOCATION is also a location) — i.e. if we want the lights on/off LOOK/message to execute also for normal location. But if we use a single event/rule to handle both we can just branch the code depending of whether we're dealing with a DARK_LOCATION or not, and therefore use a single event/rule for both.

But before tweaking this we need to decide whether normal locations should executed a LOOK when they become lit, and show the lights_out message when they become dark.

AnssiR66 commented 3 years ago

Normal locations should execute a Look when they become lit, just as dark_locations. Similarly lights_out. That's the best thing to do when thinking from the player's point of view.

AnssiR66 commented 3 years ago

I did read the passage about the normal locations and dark_locations becoming lit /not lit, that you wrote into the manual, and I think it's good. If you think it's good to keep the lit attribute for normal locations, and not remove it, then we can go that way, no problem.

tajmone commented 3 years ago

Renouncing to Light/Darkness Checks for Location

@AnssiR66, as I had imagined, carrying out the lights on/off checks for normal locations is a problematic task introducing too many variables to be a feasible feature to implement.

Since the lit attribute is entirely under the author's control, I suggest we leave it to him/her the burden of adding a LOOK statement or triggering the lights_out event (or printing a custom message) in the appropriate part of the code that flips the lit attribute (VERB, EVENT, or otherwise). We only need to mention this clearly in the Manual, remind the reader that (unlike for DARK_LOCATION) there's no automatic LOOK or lights out message on normal locations that change their illumination state during the game.

As for the difficulties encountered, it mainly boils down (as I predicted earlier on) on the fact that location is the ancestor of DARK_LOCATION — i.e. it's complicate to have EVENTs and RULEs acting on both.

This type of feature would ideally require creating a new class for unlit location (e.g. LITTABLE_LOCATION) so that the lit attribute is on the common location ancestor, but EVENTs, RULEs and other checks on the different type of locations allowing darkness can be kept separate. Since ALAN doesn't provide polymorphic inheritance, nor macros, this would also mean having to duplicate commonly shared parts of the code — which is an overkill burden, compared to asking the author to add a LOOK or custom message whenever he flips the lit state of a normal location.

Unfortunately, it seems impossible to perform a negative check on the class type (at least, I couldn't find any way to do it) — so, you can't use IF X NOT ISA dark_location (or variants thereof) to ensure that an EVENT or RULE will act only on normal locations.

I've even tried using an ELSE block, but the problem is that you still need to provide the THEN part, and couldn't find any elegant way of doing-nothing in it — inside RULEs and EVENTs you can't print an empty string, due to runtime context limitations. Matter of fact, today I'll ask Thomas to add a NOP (or DO NOTHING) instruction to handle similar situations.

Furthermore, there's also the problem that using RULEs to check if the Hero's location is lit is not reliable, even with flanked rules execution, because every time the Hero enters a lit location the RULE triggers! I've tried adding checks via visited, but it lead to unexpected results.

To make a long story short, even if implementing lights on/off tasks for normal locations could be done, it would require some horrible hacks to the library, whereas it's much simpler to just ask the author to add the task in his code, whenever he changes the location lit status, leading to much cleaner code, without the runtime costs of having multiple rules and events running in the background.

So, I suggest we leave things as they are, straighten up the StdLib Manual sections on dark locations, making it en par with the actual code and behavior, and remind the reader that for normal location he needs to execute a LOOK or print a message (if he wishes to do so) when the light changes in normal locations.

After this, I would like to try implementing the forced_lit attribute, and see how it works out in the test suite. I expect it to be quite straightforward, since we only need to tweak the DARK_LOCATION code that turns the location dark, by preventing the action if the location is forced_lit, nothing more really.

tajmone commented 3 years ago

All Done, Except Updating the Manual

OK @AnssiR66, this morning I've finally finished to implement all the discussed features of dark locations, including the forced_lit attribute and the DARK_ROOM and DARK_SITE classes (all of them being thoroughly tested in the test suite).

The new forced_lit.alan adventure tests both the forced_lit attribute and the new DARK_SITE class. You can view the results in forced_lit.a3log.

As previously mentioned, it was not possible (or feasible) to ensure that normal location changing their lit state would execute a LOOK or print the light_goes_off message (but it's easy for authors to do that when they toggle the lit attribute).

All that is left now is updating the StdLib Manual accordingly — that is, unless there's something that I've missed out, implemented wrongly, or that you would otherwise need me to fix in the library code.

AnssiR66 commented 3 years ago

Great! So, how about the updating of the manual? You've already explained the dark_locations / "(not) lit" locations difference and usage, so I guess only the force_lit attribute needs to be covered? As you coined it and know how to explain it the best, could you write also that bit? Or would you like me to write some parts anyway?

tajmone commented 3 years ago

Surely, I'll do it today, I just didn't find the time to work on it yesterday. I'll also have to update the Library classes diagram, to add the new dark_room and dark_site classes.