-
Notifications
You must be signed in to change notification settings - Fork 190
Spaceship #190
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
base: main
Are you sure you want to change the base?
Spaceship #190
Conversation
@HakiRose the current version doesn't work for me. I can't run Also, it seems to be missing the logic for the type |
The first comment is exactly the issue I have, and I wanna to talk with you. |
You might have forgotten to |
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.
Here are a few comments.
textworld/envs/glulx/git_glulx_ml.py
Outdated
if mode == 'ansi': | ||
return outfile | ||
|
||
def seed(self, seed=4567, init_seed=None): |
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.
The signature of the seed
method should be the same as in textworld.core.Environment.seed
(https://github.com/microsoft/TextWorld/blob/master/textworld/core.py#L204)
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.
This function should simply set the in-game rng seed with the seed passed in argument.
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.
Changing the seed between the episode should be handled by the agent's code.
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 need to discuss this comment with you.
@@ -0,0 +1,964 @@ | |||
#---------------------------------------------------------------------------------------------------------------------------------- |
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.
This file should not be in textworld/data/text_grammars/
. Remove this one and keep, only the one in challenges.
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.
Done
quests.append(Quest(win_events=[win_quest], fail_events=[])) | ||
# fail_quest = Event(conditions={game.new_fact("unread/e", game._named_entities['laptop']), | ||
# game.new_fact("open", game._named_entities['door A'])}) | ||
fail_quest = Event(conditions={game.new_fact("unread/e", game._entities['cpu_0']), |
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 think the losing condition should be "unread email" and "the player has left the room" instead of the door being open.
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.
By this: "the player has left the room", do you mean the return msg from Inform7?
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.
No. More like at(P, new_room) & unread(email)
or something like that.
checking email is an action applying to nothing. | ||
|
||
Carry out checking email: | ||
if a CPU-like (called pc) is unread: |
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.
It's better to also say something when the email is read. Maybe something like "You've already read all today's emails."
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.
Good idea, done!
if a CPU-like (called pc) is unread: | ||
if a random chance of 1 in 4 succeeds: | ||
Now the pc is read; | ||
Say "Email: Your mission is started."; |
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.
Ideally, we want to describe the mission 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.
ohoom! I'll consider a good description for this part.
Anyways, I'd like to discuss another topic, but relevant to this, with you.
} | ||
|
||
code :: """ | ||
Understand "tw-set seed [a number]" as updating the new seed. |
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.
That seems to work great!
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.
Yes! But there is a minor thing I'd like to share with you and have your opinion :
@ git_glulx_ml.py:
GitGlulxMLEnvironment.seed()
The increment is not 1, rather it is a random int. What do you think?
} | ||
|
||
reverse_rules { | ||
check/e :: check/e; |
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.
Those are not valid reverse rules. You can't uncheck email nor unexamine cpu. Not all actions have a way to reverse their effects.
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.
Well my understanding about this was a bit different! Let me describe it, & pls correct me if I'm wrong:
I assumed that the reverse actions provides the return to previous state as ell as repetition of an action (e.g. examine/door :: examine/door). Since we need to have check email in the "available actions" array whenever the player is in Sleeping Station, I decided to have return rule for check/e. What do you think?
|
||
commands { | ||
check/e :: "check email" :: "checking email"; | ||
examine/cpu :: "examine {cpu}" :: "examining the {cpu}"; |
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 don't think you need examine/cpu
because it is inherited from examine/o
.
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.
Oh! so if I don't mention it, still it will be there in "action array"? Like inheritance in classes...?!
code :: """ | ||
Understand the command "check" as something new. | ||
Understand "check email" as checking email. | ||
checking email is an action applying to nothing. |
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.
Because "checking email" is an action that applies to nothing, it means that Inform/the game will understand the command "check email" everywhere in the game (i.e. not necessarily in front of the laptop).
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 think it would be better if "email" is an entity in the game that we can interact with. Or even better, maybe we should support a command like: "check laptop for email", "check email on/using laptop". It's not clear to me how we can do it though.
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'll implement it.
…- Push_button and Cloth are assumed simply as key and object, respectively.
…l) is implemented, revisions on FW for stochastic commands, some minor changes on a few other files.
… and refactoring, game files(json, ni, ulx), updated logic files, ...
The design of mid-level game is finalized.