AnssiR66 / AlanStdLib

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

Custom Descriptions for Room/Site Objects Is Not Working #83

Closed tajmone closed 4 years ago

tajmone commented 4 years ago

While adding dynamic examples (from real ALAN sources) to the StdLib Manual, I realized that overriding descriptions of foor, ceiling and walls isn't working at all.

The example in StdLib Manual §3.1. ROOM (which now contains real sources and real transcripts generated from a test adventure):

THE livingroom ISA ROOM
  HAS floor_desc "There is an exquisite white carpet on the floor.".
  HAS walls_desc "The wallpaper has a nice flower pattern.".
  HAS ceiling_desc "A huge chandelier is hanging from the ceiling.".
END THE livingroom.

produced the following game transcript:

> x floor
You notice nothing unusual about the floor.

> x ceiling
You notice nothing unusual about the ceiling.

> x walls
You notice nothing unusual about the wall.

You can see the results live in the Manual, at the above link.

This needs to be fixed — either that, or remove the feature all together.

PS 1: This is an example of the benefits of using dynamic adventures and transcripts in the Manual sources, for they reveal broken functionality via the document contents.

PS 2: I believe there are many other broken features (probably dozens, or more) which I've come across during my Alan Italian tests, but didn't create an Issue here for them — maybe I've created a bug Issue on the Italian repo, I'll check. These were the reason why I never ultimated the Italian port.

tajmone commented 4 years ago

I've peeked at the library sources to understand why custom descriptions for room objects are not being shown on examine. I couldn't actually find in lib_locations.i any VERB overriding examine for this objects:

ADD TO EVERY room_object

  VERB put_against
    WHEN bulk
      CHECK THIS = wall
        ELSE "That's not possible."
  END VERB put_against.

  VERB put_behind, put_near, put_under
    WHEN bulk
      DOES ONLY "That's not possible."
  END VERB put_behind.

  VERB look_behind, look_through, look_under
    DOES ONLY "That's not possible."
  END VERB look_behind.

END ADD TO.

ADD TO EVERY site_object

  VERB put_against, put_behind, put_near, put_under
    WHEN bulk
      DOES ONLY "That's not possible."
  END VERB put_against.

  VERB look_behind, look_through, look_under
    DOES ONLY "That's not possible."
  END VERB look_behind.

END ADD TO.

And in the main VERB examine definition in lib_verbs.i there are no conditions anywhere regarding room_object and the like.

It's not quite clear to me how the floor_desc, walls_desc and ceiling_desc attributes are supposed to be intercepted when examining a room_object, since these are added to the room class, not the room_object:

EVERY room ISA LOCATION AT indoor
  HAS floor_desc "".    --| If these values are left unchanged, the descriptions
  HAS walls_desc "".    --| of the walls, floor and ceiling will be the default:
  HAS ceiling_desc "".  --| "You notice nothing unusual about the [object]."
END EVERY.

For an examine VERB implemented on the room_object class, in order to work as expected for a specific room, it would have to CHECK that the floor_desc (etc.) attributes of the current location of the Hero are empty string, and if not then print the attribute and abort the verb (to prevent the main examine to carry on). It seems rather intricate.

Can you remember how this originally worked? (i.e. at the time you wrote the Manual) And, what might have changed since then, causing it to longer work?

AnssiR66 commented 4 years ago

Yes, seems like the examine verb is not defined here like it should. It was probably something that I started but then never got around to finishing. Maybe we could just dump these attributes altogether? You're correct that they wouldn't work anyway, being added to the room/site class and then anyway should apply to objects They are not even mentioned in the library manual. Let's just forget them?

tajmone commented 4 years ago

Let's just forget them?

I've found a solution, it worked with floor. I only need to implement it for all other room/site objects and then I'll push it and explain it.

tajmone commented 4 years ago

Problem-Fix

OK @AnssiR66, here's the solution I came up with. Let me explain it in detail.

The goal is to allow authors to define custom descriptions for the wall, ceiling and/or floor. of a specific room instance (or, similarly, with site, for sky and/or ground). To do this, the StdLib offers some special attributes (walls_desc, floor_desc, etc.).

If the users doesn't set this attributes on a room instance, they will just inherit the default empty string value, which will lead to the default "You see nothing special..." message, generated by the main examine VERB from lib_verbs.i.

The problem is that these room/site objects are implemented on indoor/outdoor, which are large regions containing every room/site instance, whereas the various *_desc attributes are being added to the room/site class instead, and the user will be overriding them on specific instances of these classes (which is what allows each room to have a different wall description, even though there's a single wall object in the whole game.

My solution was to override the examine CHECKs on the specific room/site objects — e.g. with floor:

THE floor ISA room_object
  IS NOT takeable.
  [...]

  -- Honor a user-defined 'floor_desc', if present:
  VERB examine
    CHECK floor_desc OF CURRENT LOCATION = ""
      ELSE SAY floor_desc OF CURRENT LOCATION.
  END VERB examine.
  1. These checks will be carried out on floor instances before the generic examine checks from lib_verbs.i, due to higher specificity in inheritance.
  2. We check that the floor_desc attribute is an empty string for CURRENT LOCATION (i.e. the current room instance):
    • if it's unset (empty), the check passes and the main examine verb will carry on as usual (leading to the default message).
    • if the string is not empty, the check fails, we print the floor_desc of the current room and the verb aborts (no further processing nor messages).

The same needs to be done for each room/sit object.

The problem is (was) that in order to do this I needed to move all the *_desc attributes from room/site to the location class, otherwise ALAN would refuse to compile the line CHECK floor_desc OF CURRENT LOCATION = "" due the attribute non being available on every location!

Here's how I've done it:

ADD TO EVERY location
  -- ==========================================
  -- Used internally for 'room' instances only:
  -- ==========================================
  HAS floor_desc "".    --| These can set on any room for custom descriptions of
  HAS walls_desc "".    --| its walls, floor or ceiling, instead of the default
  HAS ceiling_desc "".  --| "You notice nothing unusual about the [object]."
  -- ==========================================
  -- Used internally for 'site' instances only:
  -- ==========================================
  HAS ground_desc "".   --| As above, but for descriptions of ground and sky in
  HAS sky_desc "".      --| sites instances.
END ADD TO location.

EVERY room ISA LOCATION AT indoor
END EVERY.

EVERY site ISA LOCATION AT outdoor
END EVERY.

It's safe to assume that, although these attributes are now available on every location, they won't interfere with the game logic since they are used (by the library and authors alike) only in conjunction with rooms an sites — so it should be fine.

Do you see any potential problems with this? Some interferences that I might not have thought about?

Ready to Commit and Push

I'm ready to commit these changes to the dev-2.2.0 branch, if it's OK with you, so the tests suite will be fixed, and then I can cherry pick the commit into the dev-2.2.0-docs branch in order to fix the Manual as well.

tajmone commented 4 years ago

Preview Branch for the Fixes

You can check the fixes in a the new dev branch dev-2.2.0_fix-room-site-objects-desc which I've just pushed.

When you give me the greenlight I'll merge it into the dev-2.2.0 branch.

AnssiR66 commented 4 years ago

OK, it looks good Tristano! Please go ahead by all means.