Picked up objects cannot be mutated by GridWorld

[thaije]

Describe the bug When a (human)agent has picked up an object, it is removed from the grid_world.environment_objects dictionary. However, this also prevents the gridworld from making changes to that object, such as for instance a battery decreasing of the picked up object.

To Reproduce Create two objects of a specific type, and create a god agent that changes something in objects of that type every tick. Now let a human agent pick up one of the objects, the picked up object is now unreachable for the god agent and does not change, while the object that was not picked up does change.

Expected behavior The property of the picked up object changing as well, just like all non-picked up objects of the same type.

Additional context Discovered this bug while testing the AIMS experiment, where picked up patients did not degrade in health as long as you were carrying them.

[jwaa]

Sounds like quite a bug. Either 1) the world needs to track all picked up objects in a seperate dictionary on which actions can also be performed. Or, 2) the degredation of patients occurs in the EnvObject.update method instead of being done by a God agent. The only change for MATRX is again an additional "picked up objects" dictionary, but one that is only used to loop through and call their update methods. A final option; 3) change nothing in MATRX and let the God agent in AIMS track how many ticks were not degredated and apply those as soon as the patient is dropped.

Option 1) requires an additional dictionary in the GridWorld, a change in the getters and setters for objects in the GridWorld, additional testing of all object actions, and a change in the PickUp and Drop actions.

Option 2) requires an additional dictionary in the GridWorld and a change in the game loop. However, this may also force AIMS to redevelop their patient degrade functionality which may be a lot of work.

Option 3) requires no change to MATRX but a change for AIMS. In addition it does not really solve the issue as its just "catches up" on a drop. It is not an option at all when patients have to degrade while being held (e.g. because the human/participant needs to check on patients while carrying them).

@thaije Any other solution ideas? If not, a preference for option 1, 2 or 3?

[thaije]

Hey Jasper, For AIMS this is not really an issue that needs to be fixed immediately. Victims only deteriorate in health every 3,5 minutes, while someone only holds a victim for a few seconds mostly. So the chance of missing the deterioration in health is quite low, and does not invalidate the experiments results so it is not a deal breaker. So option 3 is not really needed.

Option 2 could be a solution, however, it would require quite some changes in the code. Every action that wants to change an object would need to hook into the EnvObject.update function, even if that action only wants to change that object a single time.

I think option 1 would be a good solution.

As another solution, we could also not delete carried objects from the gridworld. The object and agent still get a carried_by and is_carrying property that point to eachother. The GUI checks for this property and ignores drawing these objects in the frontend, or alternatively could draw them smaller on top of the agent to indicate it is being carried. Another change required would be is a check for the carried_by property in the is_possible function of the pickUp and RemoveObject actions, such that those are not possible if they are being carried. By not deleting carried objects from the gridworld we don't have the update issue. Personally, I like this one the best, also because we can visualize the object being carried which is more clear to the user :)

[jwaa]

What kind of actions need to hook in the update method? AIMS specific actions? Why not just make a Patient object with a simple update that deteriorates its health every 3.5 minutes? If you want to change a property of a Patient (e.g. provide treatment), you need a ChangePropertyAction which is on our backlog anyway I think. I don't see which other actions would be needed.

Option 1 seems a lot of work though, as it involves rigorous testing of actions and potentially adjusting the is_possible methods (which are quite a mess at the moment for some actions). Though it might be the most robust solution.

You suggestion may introduce other issues. Currently you can already draw the inventory of an agent; the object properties are simply stored in the agent's state as opposed as a seperate object in the world that is hidden. In addition, it needs a change to the get_state functions in the GridWorld to ignore any objects that are being carried. Otherwise an agent might have an object in its state that is not really there at that location. Also, what if the object is is_traversable=False but is_movable=True? This would mess with movement actions as well, as some movement actions to intraversable locations suddenly becomes traversable because the object is not really there. Finally, if you drop the object it should be relocated from its old position to its new.

These are not issues that we cannot fix, but similar to as option 1 it includes quite some effort and I don't see the advantage over option 1. Furthermore it looks to me as spreading this functionality over different methods and classes, creating a nice spaghetti (although this may be the case for any solution) :P

Since there is no pressure for this, I would suggest we spend a bit more time on rethinking the drop/pickup functionality of MATRX. Better to do it right the second time :)

[jwaa]

I created #170 to brainstorm/implement a redesign of the pickup/drop functionality. Let's continue the discussion on solutions there.