-
Notifications
You must be signed in to change notification settings - Fork 600
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
Prep37 #764
Conversation
|
||
ns = "{http://ProjectMalmo.microsoft.com}" | ||
|
||
all_continuous = ["jump", "move", "pitch", "strafe", "turn", "crouch", "attack", "use"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if all this needs to be hard-coded, or whether it can be scraped directly from the XSD files? The block-type-test does something similar to extract all the block types. It would make this code more maintainable. Sadly the schemas don't define the parameter types for each command, so that information still needs to be hand coded (unless you fancy adding it to the schemas in some way, but I suspect that will be more hassle than it's worth, since only this code will actually use that information).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hoping to not have the XSDs bundled with the client (no MALMO_XSD_PATH) and overhead of parsing means this would have to be done at "build time". There are constants in the C code too. Could add item/issue to backlog.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's non-urgent.
MalmoEnv/malmoenv/core.py
Outdated
self.agent_count = len(self.xml.findall(self.ns + 'AgentSection')) | ||
turn_based = self.xml.find('.//' + self.ns + 'TurnBasedCommands') is not None | ||
if turn_based: | ||
self.turn_key = 'AKWasHere' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't that be AKWozEre?
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. | ||
// --------------------------------------------------------------------------------------------------package com.microsoft.Malmo.MissionHandlers; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happened here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed there was no header on this file but I've added a second package declaration into the comment by mistake. Will remove.
This seems good, barring a few niggles. I haven't checked the score stuff, but the python Gym env stuff looks good. I'd be interested to see how the performances compares to the "legacy" AgentHost code. |
Roll up of changes for 0.37