Eigenbahn / ai-dungeon-cli

:european_castle: A cli client to play.aidungeon.io
MIT License
149 stars 33 forks source link

add resume story support #18

Closed Zer0xFF closed 4 years ago

p3r7 commented 4 years ago

Hey!

Thanks for the feature.

A few remarks:

I propose something like the following instead (untested):

    def resume_story(self, session_id: str):
        r = self.session.get(
            "https://api.aidungeon.io/sessions"
        )
        r.raise_for_status()
        sessions_response = r.json()

        valid_id_list = [sess[id] for sess in sessions_response]
        if not session_id in valid_id_list:
            print_handler("No session found for id " + session_id
                          + ". Valid values: " + ', '.join(valid_id_list))
            return

        story_session = next(sess for sess in sessions_response if story['id'] == session_id)

        self.user_id = story["userId"]
        self.session_id = story["id"]
        self.public_id = story["publicId"]
        story_timeline = story["story"]
        i = len(story_timeline) - 1
        while(i > 0):
            if(story_timeline[i]['type'] == "output"):
                break
            i -= 1
        self.prompt_iteration = i

        last_story_output = story_timeline[self.prompt_iteration]["value"]
        self.prompt_iteration += 2
        print_handler(last_story_output)
Zer0xFF commented 4 years ago

my editor tells me id is a reserved keyword and should not be used a variable name

I can't find anything about that online or python doc, but i can make the change

we cannot call r.raise_for_status() if we call directly .json() on the requests response object

👍

I'm not super fan of printing every session_id line by line without any context

oops, that was there for debug only, i don't think we should return valid ids in that call, if thats needed it should be its own function*, while leads me to the next point, if we're not printing the ids, we dont need that part of the code and we can just id validity through story_session

* rough estimate, I have 50+ session, returning them is essentially spam

p3r7 commented 4 years ago

Yes indeed, if the backends can store that many sessions we should not display them.

I'll do a bit of testing and will come back to you.