Adding pickup and drop action to the agent

[sriyash421]

The environments in #34 #32 #26, need the agent to pick up and drop objects present in the environment. So I think we need to add an object field to the agent(considering it can pick up only one thing at a time). Currently, the door key env handles this by assuming that the agent picks up the key as it moves over its tile.

[findmyway]

Yes, and the type of that field needs to be added as a parameter type.

[sriyash421]

Proposed solution:

Base.@kwdef mutable struct Agent <: AbstractObject
    color::Symbol=:red
    dir::LRUD
    object::AbstractObject
end

function (w::AbstractGridWorld)(act::{Pickup})
    a = get_agent(w)
    obj = get_object(w,get_agent_pos(a))
    if obj.can_pickup :
        a.object = obj
    end
    w
end

function (w::AbstractGridWorld)(act::{Drop})
    a = get_agent(w)
    pos = get_agent_pos(w)
    if a.object && w[EMPTY,pos] == true
        w[a.object,pos] = true
        w[EMPTY,pos] = false
    end
    w
end

function (w::AbstractGridWorld)(act::{Toggle})
    ##specific to envs
end

[landrumb]

Did not see this proposed solution before my PR, my bad. I implemented in objects.jl, defining functions that have the agent (and for pickup, the object) as arguments and don't manipulate the board itself because I wanted to make it easy to implement nonstandard pickup-drop behavior in different environments. It would be relatively easy to also implement the interface you describe using calls to the functions I defined, although my PR uses a different abstract type for the objects that can be picked up rather than an obj.can_pickup attribute.

[Sid-Bhatia-0]

As pointed out in the suggestion by @sriyash421 and implemented in #38 by @landrumb , I agree that an agent would have an inventory field storing an object. If the agent is allowed to carry only one object at a time, this should work well.

@sriyash421 you are right in conceptualizing Pickup and Drop as actions. They indeed are actions that can be performed by an agent, so we will be using the functor interface. By the way, Pickup needs to check whether the agent already holds an object or not, as done by @landrumb in #38 , for example.

@landrumb Are you suggesting we create a new abstract type (Item in #38, for example) for a property like "pickability"? Although it works in for a single property, it would be difficult to work with the type hierarchy if we define new abstract types for each new property. For example, we could have other properties like "transparency" and so on added later (see here).

Instead, I suggest to use a function like is_pickable(o<:AbstractObject) rather than new abstract types or object fields like o.can_pickup, since later on, we might have other properties like "see_behind" for example (see here) and adding all such fields inside all types of objects may clutter them unnecessarily. Instead, if we use functions like is_pickable, we can have a default behavior for all objects (false in this case) and then overload it to return true only for specific objects like Keys.

Also, in the CollectGems environment, even though Gems are pickable objects, an agent can carry any number of gems, and not just one. So we might overload the pickup action to instead just keep a count of the number of Gems available with the agent. Or have an array inventory? Or we can just keep the environment by not having a Pickup action at all. Gems would get picked up automatically as the agent steps in that cell without the agent explicitly taking the Pickup action (as is done currently). Will have to think more about this.

[landrumb]

I didn't think of considering other attributes when I made the Item abstract type, but I would say that whether or not an object can be picked up is its most important attribute, and if any abstract class inheriting from AbstractObject is used, it should be for this attribute.

I like your idea of overloading functions that describe the behavior of different structs, but my one reservation would be that this would not be as readable. I like the idea of keeping all the information about a given struct together with its declaration, either as inheritance or an attribute.

A good way to reduce overhead while making the methods as simple as possible for environments where only one inventory space is needed would be to define another object with an array attribute, and overload pickup and drop to add and remove elements from this object as needed. Perhaps easier would be to make Agent abstract and define two structs, one for a one item inventory, one for an array inventory. Then, single item environments wouldn't have to initialize the array.

[Sid-Bhatia-0]

whether or not an object can be picked up is its most important attribute, and if any abstract class inheriting from AbstractObject is used, it should be for this attribute.

It depends. I am not so sure that we have a clear-cut winner.

I like the idea of keeping all the information about a given struct together with its declaration, either as inheritance or an attribute.

If we use subtyping for properties, a user won't be able to create a new property or alter an existing property for their own case (say if they want to change the pickability of an object type).

Something similar with using fields if a user wants to add new properties to an object type. But attributes are actually good as long as the users don't mind defining their own object structs if they want some extra properties.

Attributes would allow for each instance of an object to have its own field value for the same property (which is pretty general actually). One potential challenge could be to store each instance (with all it's field values) somewhere. But as long as we don't have too many instances or fields per object type, and if we keep object types immutable, my hunch is that this should not conflict with the efficiency offered by using a BitArray representation.

Right now we are using singleton parametric types to create Doors with different colors. The rest of the properties like pickability of each door would be the same for all Doors, for example. This means that we cannot have different properties for the same object type, something that is offered by using fields. On the other hand, if we wish to keep properties standardized across object instances (which would be singletons, as we have had so far), then using functions makes sense.

[findmyway]

Really thoughtful discussions!

I'll also add my thoughts here.

I didn't think of considering other attributes when I made the Item abstract type, but I would say that whether or not an object can be picked up is its most important attribute, and if any abstract class inheriting from AbstractObject is used, it should be for this attribute.

@landrumb I also agree that whether or not an object can be picked up is an important attribute. But creating an extra abstract class inheriting from AbstractObject may have some potential issues. For example, we may have another important attribute to specify whether an object is able to be opened (like Door or Box). But a box with a key inside can be both picked up and be opened. This leads to a diamond inheritance

         AbstractObject
          /            \
  PickableItem    OpenableItem
         \             /
           BoxWithKey

But Julia only supports single inheritance. So usually we use traits to address this issue. (In case you are not familiar with the concept of Traits, you can read the links mentioned in the first paragraph of SimpleTraits.jl)

And I think here a method of can_be_picked_up would be enough. Just like @Sid-Bhatia-0 said above.

A good way to reduce overhead while making the methods as simple as possible for environments where only one inventory space is needed would be to define another object with an array attribute, and overload pickup and drop to add and remove elements from this object as needed. Perhaps easier would be to make Agent abstract and define two structs, one for a one item inventory, one for an array inventory. Then, single item environments wouldn't have to initialize the array.

Agree! We can add different kinds of agents for different environments.