Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds respawning handler for agents - MUST UPDATE OLD ENV-SPECS #462

Open
wants to merge 36 commits into
base: dev
Choose a base branch
from

Conversation

brandonhoughton
Copy link
Member

@brandonhoughton brandonhoughton commented Mar 10, 2021

fixes openai/openai#7445
fixes openai/openai#6220

Changes the default behavior of Minecraft to allow vanilla respawning (meaning player spawn rules are respected)
This does NOT run agentStartHandlers, so agent customization such as giving a random starting inventory will not persist after respawn but will be respected for each agent's first life

To keep the behavior from old experiments that end on death, clientQuitOnDeath and serverQuitOnDeath handlers have been added. These can be configured to end the episode for each agent after death, to end the episode for all agents when a single agent dies, and to end the episode once all agents die.

This PR is a WIP in the sense that the remaining experiments must have the serverQuitOnDeath(quit_when_any_agent_dies=true) specified to retain their original behavior.

@pep8speaks
Copy link

pep8speaks commented Mar 10, 2021

Hello @brandonhoughton! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 28:1: E302 expected 2 blank lines, found 1
Line 37:37: W291 trailing whitespace

Comment last updated at 2021-03-11 17:39:07 UTC

@brandonhoughton
Copy link
Member Author

Formatting made ClientStateMachine.java messy - important changes are at 1684+

@Overwrite
public void setIngameFocus()
{
if (!this.inGameHasFocus) {
this.inGameHasFocus = true;
this.displayGuiScreen((GuiScreen) null);
this.leftClickCounter = 10000;
this.leftClickCounter = 0;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we leave this 10000 the agent has to wait that long before it is able to interact with anything again. Potentially a better fix would be to set this to 0 on respawn, but this should not affect the behavior of agents

</xs:documentation>
</xs:annotation>
<xs:complexType>
<xs:attribute name="quitWhenAnyDead" type="xs:boolean" use="optional" default="false"/>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note the default behavior here is different then the current default behavior for experiments (they are more simmilar to quitWhenAnyDead=true)

@@ -3109,6 +3120,15 @@
</xs:complexType>
</xs:element>

<xs:element name="AgentQuitFromDeath">
<xs:annotation>
<xs:documentation>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for agent to quit means to not respawn, correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct - they will stop being in the episode and the number of active agents will decrease unless the server also wants to quit when any agent dies

Copy link
Collaborator

@pzhokhov pzhokhov Mar 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if AgentQuitFromDeath is true, <ServerQuitFromDeath quitWhenAnyDead=false>, and all agents die? Would the server restart in that case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The episode should end when there are 0 agents and the episode has started. I will check


@Override
public boolean parseParameters(Object params)
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick - your opening curly brackets seem inconsistent (sometimes they are on a new line, sometimes on the same line as function / class name

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK with either convention, I'll try and normalize. Off-topic should we lint all of the code base? I had a huge diff here because the code became auto-linted

@@ -1255,6 +1255,17 @@
</xs:complexType>
</xs:element>

<xs:element name="ServerQuitFromDeath">
Copy link
Collaborator

@pzhokhov pzhokhov Mar 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe <ServerTimelimit>? QuitFromDeath seems misleading, if this is supposed to be a timelimit

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants