Tweak Listing of Worn Clothing Items

[tajmone]

I propose removing the code that handles showing the "(being worn)" text when listing clothing items that are worn by the Hero. This is currently used only in the wear verb (on clothing) when the wear action can't be carried out, to remind the player of the worn items (one of which is preventing the action).

The problem is that the text produced is redundant:

> wear jeans
You're already wearing your socks (being worn), your boxers (being worn), 
your undershirt (being worn), your sneakers (being worn), a pair of white
pantyhose (being worn) and a pair of dungarees. Trying to put the jeans 
on isn't very sensible.

... where the presence of "You're already wearing" is enough.

I had already looked into this previously, for the Italian library.

This fix would require removing from lib_messages.i the branching part that handles worns clothing in CONTAINS_* MESSAGES:

  'CONTAINS':
    IF parameter1 IS NOT plural
      THEN "$+1 contains"
      ELSE "$+1 contain"
    END IF.
  CONTAINS_COMMA: "$01"
    IF parameter1 ISA CLOTHING          <<< REMOVE THIS <<<
      THEN
        -- the following snippet adds "(being worn)" after all
        -- pieces of clothing worn by an NPC, at 'x [actor]'

        IF parameter1 IS donned
          THEN
            IF parameter1 NOT IN worn
              THEN "(being worn)"
            END IF.
        END IF.
    END IF.                             >>> REMOVE THIS >>>
    "$$,"
  CONTAINS_AND: "$01"
    IF parameter1 ISA CLOTHING          <<< REMOVE THIS <<<
      THEN
        -- the following snippet adds "(being worn)" after all
        -- pieces of clothing worn by an NPC, after 'x [actor]'

        IF parameter1 IS donned
          THEN
            IF parameter1 NOT IN worn
              THEN "(being worn)"
            END IF.
        END IF.
    END IF.                             >>> REMOVE THIS >>>

    "and"

  CONTAINS_END: "$01"
    IF parameter1 ISA CLOTHING          <<< REMOVE THIS <<<
      THEN
        -- the following snippet adds "(being worn)" after all
        -- pieces of clothing worn by an NPC, after 'x [actor]'

        IF parameter1 IS donned
          THEN
            IF parameter1 NOT IN worn
              THEN "(being worn)"
            END IF.
        END IF.
    END IF.                             >>> REMOVE THIS >>>
    "."

And then tweak the wear VERB in lib_classes.i:

    IF wear_flag OF hero >1
      THEN
        IF THIS NOT IN hero
          THEN "You pick up" SAY THE THIS. "."
        END IF.

        LOCATE THIS IN hero.
        EMPTY worn IN tempworn.     <<< REMOVE THIS <<<
        LIST tempworn.              --> Use 'LIST worn' instead

        "Trying to put" SAY THE THIS. "on isn't very sensible."

        EMPTY tempworn IN worn.     <<< REMOVE THIS <<<

Showing worn items via LIST worn is also used in the inventory VERB (i), but this doesn't currently produce the "(being worn)" text:

> inv
You are empty-handed. You are wearing your undershirt, your 
sneakers, your boxers and your socks.

... because of the IF parameter1 NOT IN worn condition in MESSAGES.

So, basically, removing the "(being worn)" code sholdn't affect other areas of the library.

I've did a thorough search in the library sources, and I couldn't find any other areas that might be affected by the proposed changes — except the mygame_import.i extra file, which would require to adapted to reflect these changes (that file might already be out of synch with the library anyhow).

Of course, I might be missing out some other intended uses for the "(being worn)" feature (eg. to allow authors to use it). But since it only works with clothes not in worn, it looks to me that it was designed specifically to work with a failing wear verb execution — and, possibly, with NPC actors.

If preserving "(being worn)" for NPC actors is required, we could just tweak the wear verb so that it doesn't move items from worn to tempworn. Otherwise, we might dispose of the MESSAGES code too.

So, if you agree on the above, and can confirm that no other areas would be impacted, and you give me the greenlight on this I could take care of tweaking the library and add more tests to check that all goes fine.

[tajmone]

As you mentioned in #57:

I would keep the "(being worn)" for now, because it is especially needed when examining NPCs [...]

The library distinguishes only between what the hero is carrying or wearing

Fine then, we might still consider tweaking the failed wear VERB part, using just LIST worn instead of shifting worn items to temp_worn. This would achive two things:

1. Remove the redundant "(being worn)" in the failed wear message.
2. Avoid triggering an EXTRACT when moving clothes to temp_worn.

Before any changes are attempted, I should add some tests to cover all occurences of worn clothing being handled by special MESSAGEs, so that the impact of any tweaks will show up in the tests log.

Add tests to:

• [x] examine NPC actor carrying AND wearing clothing items.
• [x] examine Hero carrying AND wearing clothing items.
• [x] inventory of Hero carrying AND wearing clothing items.
• [x] fail wearing a clothing item, to trigger response listing worn items.

Also, at some point we'll need to selectively squash merge the contents of clothing-tests-prep into master, so that we can have these tests when creating new dev branches to work on the various clothing problems. Just to avoid too many long-living branches and risking conflicts at the end of the work.

Furthermore, I'd like to add to the test suite an extra module with various "debug" verbs (ie. port the module I'm using in the Italian library), and move there the cloth debuggin verb currently in the clothing.alan test file. This would allow obtaining more detailed instances info in the logs, and add better checks on clothing across all test files.

If you'r OK with it, and give me the geenlight, I'd like to squash clothing-tests-prep in master, as-is, now.

[AnssiR66]

OK, you can go ahead and merge it, and do the testing there, as you described... Thanks! -Anssi

---

From: Tristano Ajmone notifications@github.com Sent: Wednesday, January 30, 2019 3:49 PM To: AnssiR66/AlanStdLib Cc: Subscribed Subject: Re: [AnssiR66/AlanStdLib] Tweak Listing of Worn Clothing Items (#58)

As you mentioned in #57https://github.com/AnssiR66/AlanStdLib/issues/57:

I would keep the "(being worn)" for now, because it is especially needed when examining NPCs [...]

The library distinguishes only between what the hero is carrying or wearing

Fine then, we might still consider tweaking the failed wear VERB part, using just LIST worn instead of shifting worn items to temp_worn. This would achive two things:

1. Remove the redundant "(being worn)" in the failed wear message.
2. Avoid triggering an EXTRACT when moving clothes to temp_worn.

Before any changes are attempted, I should add some tests to cover all occurences of worn clothing being handled by special MESSAGEs, so that the impact of any tweaks will show up in the tests log.

Add tests to:

• examine NPC actor carrying AND wearing clothing items.
• examine Hero carrying AND wearing clothing items.
• inventory of Hero carrying AND wearing clothing items.
• fail wearing a clothing item, to trigger response listing worn items.

Also, at some point we'll need to selectively squash merge the contents of clothing-tests-prep into master, so that we can have these tests when creating new dev branches to work on the various clothing problems. Just to avoid too many long-living branches and risking conflicts at the end of the work.

Furthermore, I'd like to add to the test suite an extra module with various "debug" verbs (ie. port the module I'm using in the Italian library), and move there the cloth debuggin verb currently in the clothing.alan test file. This would allow obtaining more detailed instances info in the logs, and add better checks on clothing across all test files.

If you'r OK with it, and give me the geenlight, I'd like to squash clothing-tests-prep in master, as-is, now.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/AnssiR66/AlanStdLib/issues/58#issuecomment-458949813, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AF2lHkhxgED-CIHz7LOlIc94JqTEjT6nks5vIaLggaJpZM4aZ4Ej.

[tajmone]

Merged!

Before deleting the branch I'll need to fix some of the commit-specific links in the discussion of #57, or just leave the branch there untill the issue is open (I'm not sure if the commits would still be linkable after branch deletion).

Branch deleted, links updated in the Issue thread to preserve references.

[AnssiR66]

Adding the following code snippet to the implicit taking code of a verb (after the already existing implicit taking code, in the respective verbs in lib_verbs.i) fixes the problem about implicitly taking worn clothing items. The crash won't occur, and the npc is described correctly.

IF obj ISA CLOTHING THEN MAKE obj NOT donned.

FOR EACH ac ISA ACTOR DO IF obj IN wearing OF ac THEN EXCLUDE obj FROM wearing OF ac. END IF. END FOR.

END IF.

So, the implicit taking code for example for 'give' looks like:

VERB give CHECK..... DOES -- implicit taking: IF obj NOT DIRECTLY IN hero THEN SAY implicit_taking_message OF my_game. LOCATE obj IN hero. END IF.

IF obj ISA CLOTHING THEN MAKE obj NOT donned. FOR EACH ac ISA ACTOR DO IF obj IN wearing OF ac THEN EXCLUDE obj FROM wearing OF ac. END IF. END FOR. END IF.

-- end of implicit taking.

LOCATE obj IN recipient. "You give" SAY THE obj. "to" SAY THE recipient. "." END VERB.

The question is: should I add this to all verbs using implicit taking? Probably (or maybe not to 'eat' and 'drink', because clothing items are not edible/drinkable anyway). This code ensures that I can give a piece of clothing I'm wearing, to an NPC. I can also take a piece of clothing from an NPC when the NPC is carrying it and is compliant. I can even take implicitly clothing items from a compliant actor when they are wearing them. (This might call for some adjustment/discussion, but at least it works.)

And how about if the author coins new verbs of his own? I could just add the whole implicit taking code, above, to the library manual and say that that is the code they need to add if they want implicit taking to work ok.

So, this workaround doesn't use any EXTRACTs, just adds a couple of lines to the implicit taking code.

-Anssi

---

From: Tristano Ajmone notifications@github.com Sent: Wednesday, January 30, 2019 3:49 PM To: AnssiR66/AlanStdLib Cc: Subscribed Subject: Re: [AnssiR66/AlanStdLib] Tweak Listing of Worn Clothing Items (#58)

As you mentioned in #57https://github.com/AnssiR66/AlanStdLib/issues/57:

I would keep the "(being worn)" for now, because it is especially needed when examining NPCs [...]

The library distinguishes only between what the hero is carrying or wearing

Fine then, we might still consider tweaking the failed wear VERB part, using just LIST worn instead of shifting worn items to temp_worn. This would achive two things:

1. Remove the redundant "(being worn)" in the failed wear message.
2. Avoid triggering an EXTRACT when moving clothes to temp_worn.

Before any changes are attempted, I should add some tests to cover all occurences of worn clothing being handled by special MESSAGEs, so that the impact of any tweaks will show up in the tests log.

Add tests to:

• examine NPC actor carrying AND wearing clothing items.
• examine Hero carrying AND wearing clothing items.
• inventory of Hero carrying AND wearing clothing items.
• fail wearing a clothing item, to trigger response listing worn items.

Also, at some point we'll need to selectively squash merge the contents of clothing-tests-prep into master, so that we can have these tests when creating new dev branches to work on the various clothing problems. Just to avoid too many long-living branches and risking conflicts at the end of the work.

Furthermore, I'd like to add to the test suite an extra module with various "debug" verbs (ie. port the module I'm using in the Italian library), and move there the cloth debuggin verb currently in the clothing.alan test file. This would allow obtaining more detailed instances info in the logs, and add better checks on clothing across all test files.

If you'r OK with it, and give me the geenlight, I'd like to squash clothing-tests-prep in master, as-is, now.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/AnssiR66/AlanStdLib/issues/58#issuecomment-458949813, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AF2lHkhxgED-CIHz7LOlIc94JqTEjT6nks5vIaLggaJpZM4aZ4Ej.

[AnssiR66]

I fixed the "being worn" showing needlessly by editing the Contains, Contains_comma and Contains_end messages in lib_messages.i by

IF parameter1 IS donned AND parameter1 NOT IN wearing OF hero THEN IF parameter1 NOT IN worn THEN "(being worn)" END IF.

(instead of just: "IF parameter1 IS donned")

But as there already is the "IF parameter1 NOT IN worn" since before, which should amount to the same as "parameter1 not in wearing OF hero" there is some hole in the logic somewhere in the library as to how the wearing attrubute should work. So, I didn't make this change to the library yet but will investigate first why the above addition was needed to fix the problem.

-Anssi

---

From: Anssi Räisänen anssir66@hotmail.com Sent: Wednesday, January 30, 2019 9:38 PM To: AnssiR66/AlanStdLib Subject: Re: [AnssiR66/AlanStdLib] Tweak Listing of Worn Clothing Items (#58)

Adding the following code snippet to the implicit taking code of a verb (after the already existing implicit taking code, in the respective verbs in lib_verbs.i) fixes the problem about implicitly taking worn clothing items. The crash won't occur, and the npc is described correctly.

IF obj ISA CLOTHING THEN MAKE obj NOT donned.

FOR EACH ac ISA ACTOR DO IF obj IN wearing OF ac THEN EXCLUDE obj FROM wearing OF ac. END IF. END FOR.

END IF.

So, the implicit taking code for example for 'give' looks like:

VERB give CHECK..... DOES -- implicit taking: IF obj NOT DIRECTLY IN hero THEN SAY implicit_taking_message OF my_game. LOCATE obj IN hero. END IF.

IF obj ISA CLOTHING THEN MAKE obj NOT donned. FOR EACH ac ISA ACTOR DO IF obj IN wearing OF ac THEN EXCLUDE obj FROM wearing OF ac. END IF. END FOR. END IF.

-- end of implicit taking.

LOCATE obj IN recipient. "You give" SAY THE obj. "to" SAY THE recipient. "." END VERB.

The question is: should I add this to all verbs using implicit taking? Probably (or maybe not to 'eat' and 'drink', because clothing items are not edible/drinkable anyway). This code ensures that I can give a piece of clothing I'm wearing, to an NPC. I can also take a piece of clothing from an NPC when the NPC is carrying it and is compliant. I can even take implicitly clothing items from a compliant actor when they are wearing them. (This might call for some adjustment/discussion, but at least it works.)

And how about if the author coins new verbs of his own? I could just add the whole implicit taking code, above, to the library manual and say that that is the code they need to add if they want implicit taking to work ok.

So, this workaround doesn't use any EXTRACTs, just adds a couple of lines to the implicit taking code.

-Anssi

---

From: Tristano Ajmone notifications@github.com Sent: Wednesday, January 30, 2019 3:49 PM To: AnssiR66/AlanStdLib Cc: Subscribed Subject: Re: [AnssiR66/AlanStdLib] Tweak Listing of Worn Clothing Items (#58)

As you mentioned in #57https://github.com/AnssiR66/AlanStdLib/issues/57:

I would keep the "(being worn)" for now, because it is especially needed when examining NPCs [...]

The library distinguishes only between what the hero is carrying or wearing

Fine then, we might still consider tweaking the failed wear VERB part, using just LIST worn instead of shifting worn items to temp_worn. This would achive two things:

1. Remove the redundant "(being worn)" in the failed wear message.
2. Avoid triggering an EXTRACT when moving clothes to temp_worn.

Before any changes are attempted, I should add some tests to cover all occurences of worn clothing being handled by special MESSAGEs, so that the impact of any tweaks will show up in the tests log.

Add tests to:

• examine NPC actor carrying AND wearing clothing items.
• examine Hero carrying AND wearing clothing items.
• inventory of Hero carrying AND wearing clothing items.
• fail wearing a clothing item, to trigger response listing worn items.

Also, at some point we'll need to selectively squash merge the contents of clothing-tests-prep into master, so that we can have these tests when creating new dev branches to work on the various clothing problems. Just to avoid too many long-living branches and risking conflicts at the end of the work.

Furthermore, I'd like to add to the test suite an extra module with various "debug" verbs (ie. port the module I'm using in the Italian library), and move there the cloth debuggin verb currently in the clothing.alan test file. This would allow obtaining more detailed instances info in the logs, and add better checks on clothing across all test files.

If you'r OK with it, and give me the geenlight, I'd like to squash clothing-tests-prep in master, as-is, now.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/AnssiR66/AlanStdLib/issues/58#issuecomment-458949813, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AF2lHkhxgED-CIHz7LOlIc94JqTEjT6nks5vIaLggaJpZM4aZ4Ej.

[tajmone]

Sorry for the late reply, but I've been working all afternoon at the reorganization of the tests suite (see PR #59). With the new tests suite organization it's going to be a lot cleaner to work on any tests, for it allows separating them into single folder per topic. (please, merge ASAP!)

there is some hole in the logic somewhere in the library as to how the wearing attrubute should work.

I agree with this, for the more I look into it the more complex it seems. But I know it's fixable because I had explored it and did some testing for the Italian library, but ultimately I discarded them because I wanted to first make sure all the important parts were fixed.

I was really hoping to find a solution to moving around worn items that could work with any verb, including user defined verbs, instead of just fixing the library verbs. This would have prevent problems when authors create their own verbs (which can easily involve clothings, by default).

Something similar was achieved with liquids and vessels, via EVENTs, but here we have the limitation of the stack bug that triggers on EXTRACT caused by EVENTS, so chances are we should settle for a compromise.

I haven't yet worked out how many verbs could affect worn clothing, but they might be quite a few others.

Tomorrow I want to test an alternative route: dispose of the clothing Event and try to use only EXTRACT to control worn clothes. After all, we're only interested in clothing being removed (extracted) from either ACTORs (NPCs) or worn (for Hero). Most verbs and implicit actions use LOCATE, so hopefully this might not trigger the bug, for they are not EVENTs.

I'll let you know. (but for today I'm done)

[tajmone]

I've updated the tests (in a dev branch) and added a new test script that covers all the cases when worn items are listed:

• tests/clothing/worn-items-listing.a3log

So, when tweaking the library code we'll get live feedaback from that log.

I think that these are all the situations in which worn clothing are being handled by the library (correct if I'm wrong):

context	behavior
x actor	"The <actor> is carrying ... item1 (being worn)"	✔
inventory	"You are wearing ... item1, item2"	✔
wear item (fail)	"You're already wearing... item1 (being worn)"	✔ *
remove item (fail)	"You are wearing ... item1, item2"	✔

* UPDATE — "wear item (fail)" was fixed in c4e0b5b8.

[tajmone]

Fixes Proposal

Anssi, if it's OK with you I'd start with those fixes that are safe to apply and won't cross-disrupt anything else.

Right now, I'd say that in order to preserve the "(being worn)" message for when examining NPCs, the best fix would be just to make a failed wear action list worn items without the "(being worn)" message, like it's done for a failed removed action.

This is how wear reports a failed action:

    IF wear_flag OF hero >1
      THEN
        IF THIS NOT IN hero
          THEN "You pick up" SAY THE THIS. "."
        END IF.

        LOCATE THIS IN hero.
        EMPTY worn IN tempworn.   --> REMOVE!
        LIST tempworn.            --> CHANGE TO:  LIST worn.

        "Trying to put" SAY THE THIS. "on isn't very sensible."

        EMPTY tempworn IN worn.   --> REMOVE!

And this is how remove does it:

  IF wear_flag OF hero > 0
    THEN
      LIST worn.                  --> Doesn't rely on tempworn! (correct)
      "Trying to take" SAY THE THIS. "off isn't very sensible."

The "(being worn)" message is only shown for clothing NOT IN worn, so if we want to preserve this in examining NPCs then all we need to do is have the failed wear keep them in worn (like remove). The inventory verb is already working fine for the player, so the above fix should settle the problem of cloth listing in all places.

If you give the greenlight I'll do these changes and commit them to this temp branch, so we can further test them before merging.

[tajmone]

But as there already is the "IF parameter1 NOT IN worn" since before, which should amount to the same as "parameter1 not in wearing OF hero" there is some hole in the logic somewhere in the library as to how the wearing attrubute should work. So, I didn't make this change to the library yet but will investigate first why the above addition was needed to fix the problem.

The problem right now is that some verbs are interfering with this whole process. For example putting a worn item into a box effectively removes it from the worn entity, but the item is still seen as donned and in the wearing of Hero.

Until we find a safe way to work around the newly discovered bug of the Even that is handling this, we'd better not make structural changes. Definitely, the worn_clothing_check EVENT must be dropped since it crashes Alan. So we'll need to decide on which strategy to adopt to ensure that donned, wearing and worn are properly handled by verbs.

I think that the MESSAGES part is actually fine, and with the above fix I proposed it should work as originally inteded. Examining actors could be improved, to show a message like the Hero inventory, where held items and worn clothing are listed separately, but this is just an issue of elegance, not a bug or anything intrinsically wrong.

[AnssiR66]

Hi Tristano, sounds good, you can try that out. So, remember I sent the message the other day where I had made changes to the lib_messages.i file (Message Contains etc), I will then change these back in my own file and we will try out if the problem is fixed with your method. -Anssi

---

From: Tristano Ajmone notifications@github.com Sent: Friday, February 1, 2019 11:49 AM To: AnssiR66/AlanStdLib Cc: AnssiR66; Comment Subject: Re: [AnssiR66/AlanStdLib] Tweak Listing of Worn Clothing Items (#58)

Fixes Proposal

Anssi, if it's OK with you I'd start with those fixes that are safe to apply and won't cross-disrupt anything else.

Right now, I'd say that in order to preserve the "(being worn)" message for when examining NPCs, the best fix would be just to make a failed wear action list worn items without the "(being worn)" message, like it's done for a failed removed action.

This is how wear reports a failed action:

IF wear_flag OF hero >1
  THEN
    IF THIS NOT IN hero
      THEN "You pick up" SAY THE THIS. "."
    END IF.

    LOCATE THIS IN hero.
    EMPTY worn IN tempworn.   --> REMOVE!
    LIST tempworn.            --> CHANGE TO:  LIST worn.

    "Trying to put" SAY THE THIS. "on isn't very sensible."

    EMPTY tempworn IN worn.   --> REMOVE!

And this is how remove does it:

IF wear_flag OF hero > 0 THEN LIST worn. --> Doesn't rely on tempworn! (correct) "Trying to take" SAY THE THIS. "off isn't very sensible."

The "(being worn)" message is only shown for clothing NOT IN worn, so if we want to preserve this in examining NPCs then all we need to do is have the failed wear keep them in worn (like remove). The inventory verb is already working fine for the player, so the above fix should settle the problem of cloth listing in all places.

If you give the greenlight I'll do these changes and commit them to this temp branch, so we can further test them before merging.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/AnssiR66/AlanStdLib/issues/58#issuecomment-459666270, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AF2lHpeVm9E7MmoD7PghSqZ0QUzsmUpcks5vJA2QgaJpZM4aZ4Ej.

[tajmone]

Fixed!

Now the output is:

> wear jeans
You pick up the jeans. You are wearing your undershirt, your sneakers, 
your khakis, your boxers and your socks. Trying to put the jeans on isn't
very sensible.

All other context seems fine too:

• worn-items-listing.a3log

(at least, I'm not aware of any other contexts beside those mentioned in the table sketeched above).

[tajmone]

Delete tempworn?

It looks like the tempworn was only used in the wear verb to allows the list of worn items to show as "(being worn)". I did a case-insenstive search for "tempworn" in all the library sources and I can't see it being used elsewhere.

I therefore assume we could safely delete it from lib_classes.i (598–606 in commit 46f8be8):

--------------------------------------------------------------------
-- A container used to provide a temporary storage space - ignore!
--------------------------------------------------------------------

THE tempworn ISA OBJECT
  CONTAINER TAKING CLOTHING.
  HEADER "You're already wearing"
END THE tempworn.

[AnssiR66]

I commented about the tempworn container in the other issue. I suggest we leave it be for now, for the reasons I stated there...

[tajmone]

I commented about the tempworn container in the other issue. I suggest we leave it be for now, for the reasons I stated there...

True, it doesn't do any arm either (just an object floating nowhere). Also, authors could use it in their own verbs if they need to list Hero's worn items with "(being worn)". Beside this, I'm 100% positive is not currently used anywhere.

Improve Examine Player: Separate Carried and Worn

Ok, now the all the listings of worn items are fixed. But I didn't close this issue yet because I though we could still attempt to improve "examine actor" so that it produces a message in which the list of carried and worn items is explicitly separated (like in inventory). Like:

> examine Bob Bob is carrying a torch. He's wearing a hat, a shirt and a coat.

It's not a big deal, but it would make the output more elegant and the library more consistent.

I'm going to try that on the Italian library anyhow, so I'll let you know how it went.