Problems with take ALL and drop ALL (5.8 beta)

KVonGit commented 6 years ago

Default behavior

You are in a room. You can see a table (on which there is a basket (containing a bag (containing a ball))).

> get all basket: You pick it up. bag: You pick it up. ball: You pick it up.

> drop all basket: You drop it. bag: You drop it. ball: You drop it.

> undo Undo: drop all

> undo Undo: get all

> l You are in a room. You can see a table (on which there is a basket (containing a bag (containing a ball))).

> get basket You pick it up.

> i You are carrying a basket (containing a bag (containing a ball)).

> drop all basket: You drop it. bag: You are not carrying it. ball: You are not carrying it.

My "fixes"

EDIT This is no good!

This may not be the best way to handle this, but it seems to work ~~VERY well~~ UNLESS directly dealing with an object in a held container.

Take

if (multiple and ListCount(object) = 0) {
 msg ("Nothing here to take.")
}
else {
 foreach (obj, object) {
   if (not ListContains(GetAllChildObjects(game.pov),obj)) {
     DoTake (obj, multiple)
   }
 }
}

Drop

if (multiple and ListCount(object) = 0) {
  msg ("You are not carrying anything.")
}
else {
  foreach (obj, object) {
    if (obj.parent = game.pov) {
      DoDrop (obj, multiple)
    }
  }
}

You are in a room. You can see a table (on which there is a basket (containing a bag (containing a ball))).

> get all basket: You pick it up.

> i You are carrying a basket (containing a bag (containing a ball)).

> drop all basket: You drop it.

> i You are not carrying anything.

> l You are in a room. You can see a table and a basket (containing a bag (containing a ball)).

> get all basket: You pick it up.

> i You are carrying a basket (containing a bag (containing a ball)).

> l You are in a room. You can see a table.

> put basket on table Done.

> i You are not carrying anything.

> l You are in a room. You can see a table (on which there is a basket (containing a bag (containing a ball))).

KVonGit commented 6 years ago

UPDATE

My "fixes" don't print anything when trying to take an object from inside of a held container.

KVonGit commented 6 years ago

NOTE

I think this is how GET ALL should behave. It's how it behaves in Inform, TADS and most Infocom games.

...but everyone else on the forum seemed to disagree. (I can't find the old thread.)

ThePix commented 6 years ago

I think the drop script is fine. Here is a take script; see how it looks to you. Works for me.

if (multiple and ListCount(object) = 0) {
  msg ("Nothing here to take.")
}
else {
  foreach (obj, object) {
    // if this is multiple then we should skip anything in a container that will be taken
    // and anything held by an NPC
    if (not multiple or (not obj.parent.take and not DoesInherit(obj.parent, "npc_type"))) {
      DoTake (obj, multiple)
    }
  }
}

This is the thread: http://textadventures.co.uk/forum/quest/topic/pl1aur8l3usk2lrn_rzy1g/meaning-of-take-all-and-drop-all

KVonGit commented 6 years ago

Does this not happen to you when holding a container with child objects?

> i You are carrying a basket (containing a bag (containing a ball)).

> drop all basket: You drop it. bag: You are not carrying it. ball: You are not carrying it.

ThePix commented 6 years ago

Sorry, I meant the drop script you have above, not the one in the beta.

KVonGit commented 6 years ago

Put an egg in a basket. Pick up the basket. Enter DROP EGG. (There is no response and nothing happens.)

KVonGit commented 6 years ago

These 2 (it's the same TAKE) seem to work fine:

  <command name="take">
    <pattern>take #object#; get #object#; pick up #object#</pattern>
    <allow_all />
    <scope>notheld</scope>
    <script>
      if (multiple and ListCount(object) = 0) {
        msg ("Nothing here to take.")
      }
      else {
        foreach (obj, object) {
          // if this is multiple then we should skip anything in a container that will be taken
          // and anything held by an NPC
          if (not multiple or (not obj.parent.take and not DoesInherit(obj.parent, "npc_type"))) {
            DoTake (obj, multiple)
          }
        }
      }
    </script>
  </command>

REMOVED BAD Take CODE

KVonGit commented 6 years ago

Sorry, that ignored objects in objects which were held!

This seems foolproof (which can't be true, can it?):

  <command name="drop">
    <pattern>drop #object#</pattern>
    <allow_all />
    <scope>inventory</scope>
    <script>
      if (multiple and ListCount(object) = 0) {
        msg ("You are not carrying anything.")
      }
      else {
        foreach (obj, object) {
          if (Contains(game.pov,obj)) {
            DoDrop (obj, multiple)
          }
        }
      }
    </script>
  </command>

ThePix commented 6 years ago

DROP ALL:

That looks good. I was worried that an item that appears in the game file before the container would be listed before the container in object, but that is not the case, so this works as it should.

ThePix commented 6 years ago

I have uploaded a modified CoreCommands to Github. I have also modified it so the blocking message is customisable (as a good way to give NPC items is to have the NPC as a transparent container); in a later version I will add something to the editor for that too.

KVonGit commented 6 years ago

Sounds like this Issue is [SOLVED]!

KVonGit commented 6 years ago

Sorry to reopen this.

I have a table in a room. It is a surface (of course). I consider the table a backdrop. Being that it doesn't make sense to try to take the table, the table is not_all.

Now, I have an object on the table which SHOULD be included in TAKE ALL.

This TAKE works as I expect:

  <command name="take">
    <pattern>take #object#; get #object#; pick up #object#</pattern>
    <allow_all />
    <scope>notheld</scope>
    <script>
      if (multiple and ListCount(object) = 0) {
        msg ("Nothing here to take.")
      }
      else {
        foreach (obj, object) {
          // if this is multiple then we should skip anything in a container that will be taken
          // and anything held by an NPC
          if (not multiple or (not obj.parent.take and not DoesInherit(obj.parent, "npc_type"))) {
            DoTake (obj, multiple)
          }
        }
      }
    </script>
  </command>

The new TAKE (which is currently in CoreCommands) ignores the items on my table.

KVonGit commented 6 years ago

I should have included the example game:

<!--Saved by Quest 5.8.6729.18213-->
<asl version="580">
  <include ref="English.aslx" />
  <include ref="Core.aslx" />
  <game name="beta7 Drop and Take ALL Tester">
    <gameid>89dcbac6-772d-4c91-a44a-e1e346733b29</gameid>
    <version>1.0</version>
    <firstpublished>2018</firstpublished>
    <feature_lightdark />
  </game>
  <command name="take">
    <pattern>take #object#; get #object#; pick up #object#</pattern>
    <allow_all />
    <scope>notheld</scope>
    <script>
      if (multiple and ListCount(object) = 0) {
        msg ("Nothing here to take.")
      }
      else {
        foreach (obj, object) {
          // if this is multiple then we should skip anything in a container that will be taken
          // and anything held by an NPC
          if (not multiple or (not obj.parent.take and not DoesInherit(obj.parent, "npc_type"))) {
            DoTake (obj, multiple)
          }
        }
      }
    </script>
  </command>
  <command name="drop">
    <pattern>drop #object#</pattern>
    <allow_all />
    <scope>inventory</scope>
    <script>
      if (multiple and ListCount(object) = 0) {
        msg ("You are not carrying anything.")
      }
      else {
        foreach (obj, object) {
          if (Contains(game.pov,obj)) {
            DoDrop (obj, multiple)
          }
        }
      }
    </script>
  </command>
  <object name="room">
    <inherit name="editor_room" />
    <isroom />
    <object name="player">
      <inherit name="editor_object" />
      <inherit name="editor_player" />
    </object>
    <object name="backpack">
      <inherit name="editor_object" />
      <inherit name="container_open" />
      <feature_container />
      <take />
      <object name="book">
        <inherit name="editor_object" />
        <take />
      </object>
    </object>
    <object name="table">
      <inherit name="editor_object" />
      <inherit name="surface" />
      <feature_container />
      <listchildren />
      <not_all />
      <object name="napkin">
        <inherit name="editor_object" />
        <take />
      </object>
    </object>
  </object>
</asl>

This is the expected behavior:

ThePix commented 6 years ago

Do not tick the table as "not_all". It will not be listed, as it is scenery, but the apple should be.

KVonGit commented 6 years ago

If I make the table scenery, the room description won't list it or it's contents, though; will it?

KVonGit commented 6 years ago

Ha!

That doesn't list the table or the apple which is on it, but TAKE ALL does take the apple...

EDIT

I think the table should be listed, though. It's important, because it has an apple on it and you can put things on it. So the room description should handle its contents. But it can't be taken, so TAKE ALL should ignore it.

Maybe I'm just overthinking this...

KVonGit commented 6 years ago

I can even add Bob, greasy hamburger in hand, to the room with the take script you posted in this thread and everything works perfectly with the table not scenery, but not_all, and Bob is a male surface.

<!--Saved by Quest 5.8.6729.18213-->
<asl version="580">
  <include ref="English.aslx" />
  <include ref="Core.aslx" />
  <game name="beta7 Drop and Take ALL Tester">
    <gameid>89dcbac6-772d-4c91-a44a-e1e346733b29</gameid>
    <version>1.0</version>
    <firstpublished>2018</firstpublished>
    <feature_lightdark />
  </game>
  <command name="take">
    <pattern>take #object#; get #object#; pick up #object#</pattern>
    <allow_all />
    <scope>notheld</scope>
    <script>
      if (multiple and ListCount(object) = 0) {
        msg ("Nothing here to take.")
      }
      else {
        foreach (obj, object) {
          // if this is multiple then we should skip anything in a container that will be taken
          // and anything held by an NPC
          if (not multiple or (not obj.parent.take and not DoesInherit(obj.parent, "npc_type"))) {
            DoTake (obj, multiple)
          }
        }
      }
    </script>
  </command>
  <command name="drop">
    <pattern>drop #object#</pattern>
    <allow_all />
    <scope>inventory</scope>
    <script>
      if (multiple and ListCount(object) = 0) {
        msg ("You are not carrying anything.")
      }
      else {
        foreach (obj, object) {
          if (Contains(game.pov,obj)) {
            DoDrop (obj, multiple)
          }
        }
      }
    </script>
  </command>
  <object name="room">
    <inherit name="editor_room" />
    <isroom />
    <object name="player">
      <inherit name="editor_object" />
      <inherit name="editor_player" />
      <object name="no tea">
        <inherit name="editor_object" />
        <visible type="boolean">false</visible>
      </object>
    </object>
    <object name="backpack">
      <inherit name="editor_object" />
      <inherit name="container_open" />
      <feature_container />
      <take />
      <object name="book">
        <inherit name="editor_object" />
        <take />
      </object>
      <object name="handle">
        <inherit name="editor_object" />
        <scenery />
        <look>An ordinary handle, made of nylon or something similar.</look>
      </object>
    </object>
    <object name="table">
      <inherit name="editor_object" />
      <inherit name="surface" />
      <feature_container />
      <listchildren />
      <not_all />
      <object name="apple">
        <inherit name="editor_object" />
        <take />
      </object>
    </object>
    <object name="Bob">
      <inherit name="editor_object" />
      <inherit name="namedmale" />
      <inherit name="surface" />
      <feature_container />
      <contentsprefix>who is holding</contentsprefix>
      <listchildren />
      <listchildrenprefix>He is carrying</listchildrenprefix>
      <addscript type="script">
        msg ("Bob probably wouldn't like that.")
      </addscript>
      <object name="burger">
        <inherit name="editor_object" />
        <alias>big, greasy hamburger</alias>
        <look>It's the greasiest double-cheesburger you've ever seen!</look>
      </object>
    </object>
  </object>
</asl>

ThePix commented 6 years ago

So...

If the table is scenery, I do not think the room description should list what is on it... I think. Anyway that is a whole different can of worms.

Whether or not the table is scenery, the apple is taken, if the table cannot be. I think that is right.

Bob has not_all set, being a namedmale, and that means he is not listed, and neither is his burger. Again, I think that is what should happen, though I guess the latter can be argued.

KVonGit commented 6 years ago

If the table is scenery, I do not think the room description should list what is on it.

Correct. I agree that this is working correctly.

Whether or not the table is scenery, the apple is taken, if the table cannot be. I think that is right.

Yes...

But the point of not_all (as far as I understand it) is to exempt things like this table while not making it scenery. The table needs to be listed in the room description to let the player know what is on the table at any given given moment, but it is pointless to print a "You can't take it." message for the table any time the player enters GET ALL.

When playing a text adventure, most players just enter GET ALL when they enter rooms. It's a nice corner-cutter, especially when playing a large game. It doesn't always cover all the bases, but it does usually acquire a few items which would normally take more time to find.

Bob has not_all set, being a namedmale, and that means he is not listed, and neither is his burger. Again, I think that is what should happen, though I guess the latter can be argued.

I agree that this bit works perfectly. I just threw Bob in to show that.

KVonGit commented 6 years ago

This is the TAKE I vote for:

https://github.com/textadventures/quest/issues/1017#issuecomment-395363489

I'm just casting my vote. I'm not saying my way is the best way; it's just seems like it should work this way to me.

ThePix commented 6 years ago

Sounds like we need three states for not_all, so we can include contained items or not.

KVonGit commented 6 years ago

Sounds like we need three states for not_all, so we can include contained items or not.

I think I must be overlooking something...

I can't find anything this TAKE doesn't handle properly:

  <command name="take">
    <pattern>take #object#; get #object#; pick up #object#</pattern>
    <allow_all />
    <scope>notheld</scope>
    <script>
      if (multiple and ListCount(object) = 0) {
        msg ("Nothing here to take.")
      }
      else {
        foreach (obj, object) {
          // if this is multiple then we should skip anything in a container that will be taken
          // and anything held by an NPC
          if (not multiple or (not obj.parent.take and not DoesInherit(obj.parent, "npc_type"))) {
            DoTake (obj, multiple)
          }
        }
      }
    </script>
  </command>

PS

I'm not being argumentative. I seriously think I'm overlooking something.

ThePix commented 6 years ago

I have a dog called Rover. He is a "namedmale", but he can be picked up. How do I handle that? With my version, you could just untick "not_all".

[someone did bring this up on the thread, btw]

ThePix commented 6 years ago

Thinking more, I think that if the table is listed in the room description, then it should also be listed with ALL.

ThePix commented 6 years ago

I am updating the docs as I go along. This is what I have on this topic, which may make my thinking clearer:

Some commands will allow the player to give a list of items to apply the action to, or to just say ALL. For example, the player can type GET ALL or DROP BAT, BALL AND HAT.

Handling ALL is not straightforward, as we need to consider exactly what items to consider.

Note that turnscripts will only fire once per command, rather than once per object.

TAKE ALL

Let us suppose there is a rucksack with a book in it, an open cupboard with a ball of string in it, a character called Mary, who is holding a cup, and a table with an apple on it. There is also a door that is mentioned in the room description, and is implemented, but is just scenery, and not listed when the player types LOOK.

You can see a rucksack (containing a book), a cupboard (containing a ball of string), a Mary (carrying a cup) and a table (on which there is an apple).

GET ALL rucksack: You pick it up. cupboard: You can't take it. ball of string: You pick it up. table: You can't take it. apple: You pick it up.

Note that Quest does not try to take the book; it is inside the rucksack, and that has been picked up already. It will get the string and the apple, though, as they are in containers that cannot be taken. There is no attempt to take the door, as it is scenery.

Quest also does not try to take Mary, as she is a character. It can be useful to set up characters as surfaces or (as in the example above) transparent containers so the player can see what they are carrying. GET ALL will also ignore any item carried by a character (but note that items inside items held by characters are not properly supported!).

Excluding other items

Just as Mary was excluded from the ALL list, you can exclude other items, just by ticking the "Object is excluded..." box on the Inventory tab (behind the scenes this sets their "not_all" flag to true).

Note that this will cause any contained objects to also be excluded, and it may be better to flag the container as scenery instead.

Characters that can be taken

Conversely, you may want a character to be taken. Perhaps Mary is a poodle that the player can pick up. Just untick the "Object is excluded..." box. You will also need to tick the "Object can be taken" box as normal.

DROP ALL

What gets dropped is considerably easier

i You are carrying a rucksack (containing a book), a ball of string, a purse and an apple.

drop all rucksack: You drop it. ball of string: You drop it. purse: You drop it. apple: You drop it.

The only thing to note is that the book is dropped inside the rucksack, so is not mentioned.

Handling ALL in your own commands

For the majority of commands, it is not necessary to add the facility for ALL, and most of the built-in commands do not support it. However, if you want to allow it for your custom command, here is what you must do:

The command must have a Boolean attribute called "allow_all" set to true.

You need to set the scope. The tells Quest where to look for objects, and is a good idea for all commands.

You also also need to modify the script. For any command with "allow_all" set to true, the object variable will be a list of objects, rather than one object - even if the player only specifies a single object.

The script will also have access to a second variable, multiple, which will be true if the player said ALL or gave a list of items.

By way of an example, we will look at the script for TAKE:

if (multiple and ListCount(object) = 0) {
  msg ("Nothing here to take.")
}
else {
  foreach (obj, object) {
    if (not multiple or (not Contains(game.pov, obj) and not obj.parent.not_all)) {
      DoTake (obj, multiple)
    }
  }
}

The three lines handle when the player says GET ALL and there is nothing to take. In that event, multiple is true, and the length of the list, object, is zero, and a message is printed.

Otherwise we go though each member of the list.

We now need to consider if the item should be included in an ALL list. If multiple is false, we need to handle it whatever. If it is true, there are some situations where we should not handle it (in this case, if the container has already been taken or if flagged as "not_all", but it will be different for you).

Then the action is done. In this case, another function is called. Inside that functio, if multiple is true, the object name and a colon are prefixed to the response.

KVonGit commented 6 years ago

Thinking more, I think that if the table is listed in the room description, then it should also be listed with ALL.

That's the whole purpose of not_all. It gives the author the ultimate say-so.

In the real world, you are in a room. You can see a table (on which is an apple) and a basket (in which is an apple).

If I say, "Pix, grab everything in that room," are you going to try to take the table?

KVonGit commented 6 years ago

I call my dog Spuds, but I make him a named male and uncheck not_all.

    <object name="Spuds">
      <inherit name="editor_object" />
      <inherit name="namedmale" />
      <inherit name="surface" />
      <feature_container />
      <contentsprefix>who is wearing</contentsprefix>
      <listchildren />
      <listchildrenprefix>He is wearing</listchildrenprefix>
      <addscript type="script">
        msg ("Spuds probably wouldn't appreciate that.")
      </addscript>
      <look>He is a white dog with a black spot over his right eye.</look>
      <take />
      <attr name="not_all" type="boolean">false</attr>
      <object name="collar">
        <inherit name="editor_object" />
        <look>The dog's collar has "Spuds" written on it.</look>
      </object>
    </object>

This example game has my preferred version of TAKE (which you wrote).

See if you can break it.

<!--Saved by Quest 5.8.6729.18213-->
<asl version="580">
  <include ref="English.aslx" />
  <include ref="Core.aslx" />
  <game name="beta7 Drop and Take ALL Tester">
    <gameid>89dcbac6-772d-4c91-a44a-e1e346733b29</gameid>
    <version>1.0</version>
    <firstpublished>2018</firstpublished>
    <feature_lightdark />
  </game>
  <command name="take">
    <pattern>take #object#; get #object#; pick up #object#</pattern>
    <allow_all />
    <scope>notheld</scope>
    <script>
      if (multiple and ListCount(object) = 0) {
        msg ("Nothing here to take.")
      }
      else {
        foreach (obj, object) {
          // if this is multiple then we should skip anything in a container that will be taken
          // and anything held by an NPC
          if (not multiple or (not obj.parent.take and not DoesInherit(obj.parent, "npc_type"))) {
            DoTake (obj, multiple)
          }
        }
      }
    </script>
  </command>
  <command name="drop">
    <pattern>drop #object#</pattern>
    <allow_all />
    <scope>inventory</scope>
    <script>
      if (multiple and ListCount(object) = 0) {
        msg ("You are not carrying anything.")
      }
      else {
        foreach (obj, object) {
          if (Contains(game.pov,obj)) {
            DoDrop (obj, multiple)
          }
        }
      }
    </script>
  </command>
  <object name="room">
    <inherit name="editor_room" />
    <isroom />
    <object name="player">
      <inherit name="editor_object" />
      <inherit name="editor_player" />
      <object name="no tea">
        <inherit name="editor_object" />
        <visible type="boolean">false</visible>
      </object>
    </object>
    <object name="backpack">
      <inherit name="editor_object" />
      <inherit name="container_open" />
      <feature_container />
      <take />
      <object name="book">
        <inherit name="editor_object" />
        <take />
      </object>
      <object name="handle">
        <inherit name="editor_object" />
        <scenery />
        <look>An ordinary handle, made of nylon or something similar.</look>
      </object>
    </object>
    <object name="table">
      <inherit name="editor_object" />
      <inherit name="surface" />
      <feature_container />
      <listchildren />
      <not_all />
      <object name="apple">
        <inherit name="editor_object" />
        <take />
      </object>
    </object>
    <object name="Bob">
      <inherit name="editor_object" />
      <inherit name="namedmale" />
      <inherit name="surface" />
      <feature_container />
      <contentsprefix>who is holding</contentsprefix>
      <listchildren />
      <listchildrenprefix>He is carrying</listchildrenprefix>
      <addscript type="script">
        msg ("Bob probably wouldn't like that.")
      </addscript>
      <object name="burger">
        <inherit name="editor_object" />
        <alias>big, greasy hamburger</alias>
        <look>It's the greasiest double-cheesburger you've ever seen!</look>
      </object>
    </object>
    <object name="Spuds">
      <inherit name="editor_object" />
      <inherit name="namedmale" />
      <inherit name="surface" />
      <feature_container />
      <contentsprefix>who is wearing</contentsprefix>
      <listchildren />
      <listchildrenprefix>He is wearing</listchildrenprefix>
      <addscript type="script">
        msg ("Spuds probably wouldn't appreciate that.")
      </addscript>
      <look>He is a white dog with a black spot over his right eye.</look>
      <take />
      <attr name="not_all" type="boolean">false</attr>
      <object name="collar">
        <inherit name="editor_object" />
        <look>The dog's collar has "Spuds" written on it.</look>
      </object>
    </object>
  </object>
</asl>

KVonGit commented 6 years ago

As far as this table is concerned:

It is not scenery
It is fixed in place, therefore should not be included in TAKE ALL
Any objects on the table should be included in TAKE ALL unless they are deemed fixed-in-place

EDIT

Here's how Inform explains it:

http://inform7.com/learn/man/WI_18_36.html

KVonGit commented 6 years ago

In the end, I guess it all boils down to the individual player's expectations (not even really the author's expectations).

I'll be using the TAKE in the examples I'm posting in my CoreCommands file, and anyone else could just as easily customize their own if they don't like the way the default script handles things.

Quest doesn't have a fixedinplace attribute, so I use not_all to cover that.

ThePix commented 6 years ago

How about this for TAKE:

if (multiple and ListCount(object) = 0) {
  msg ("Nothing here to take.")
}
else {
  foreach (obj, object) {
    // if this is multiple then we should skip anything in a container that will be taken
    // and anything held by an NPC
    msg (obj.parent.take)
    msg (DoesInherit(obj.parent, "npc_type"))
    if (not multiple or (not Contains(game.pov, obj.parent) and not DoesInherit(obj.parent, "npc_type"))) {
      DoTake (obj, multiple)
    }
  }
}

Checking the "take" attribute is dodgy as it might be a script.

KVonGit commented 6 years ago

Checking the "take" attribute is dodgy as it might be a script.

Good thinking!

And that works perfectly!

KVonGit commented 6 years ago

I spoke too soon.

This seems to fix it:

if (multiple and ListCount(object) = 0) {
  msg ("Nothing here to take.")
}
else {
  took_something = false
  foreach (obj, object) {
    // if this is multiple then we should skip anything in a container that will be taken
    // and anything held by an NPC
    if (not multiple or (not Contains(game.pov, obj.parent) and not DoesInherit(obj.parent, "npc_type"))) {
      DoTake (obj, multiple)
      took_something = true
    }
  }
  if (not took_something) {
    msg ("Nothing here to take.")
  }
}

KVonGit commented 6 years ago

FINAL THOUGHT

I'd go with "There is no 'all' to take."

...but that's easily customizable.

ThePix commented 6 years ago

I think we can rearrange it. And use a template:


took_something = false
foreach (obj, object) {
  // if this is multiple then we should skip anything in a container that will be taken
  // and anything held by an NPC
  if (not multiple or (not Contains(game.pov, obj.parent) and not DoesInherit(obj.parent, "npc_type"))) {
    DoTake (obj, multiple)
    took_something = true
  }
}
if (multiple and not took_something) {
  msg ("[NothingToTake]")
}

KVonGit commented 6 years ago

That's some nice-looking code!

textadventures / quest