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

EvMenu custom exit node runs cmd_on_exit twice #1428

Closed
RyanStein opened this issue Sep 13, 2017 · 6 comments
Closed

EvMenu custom exit node runs cmd_on_exit twice #1428

RyanStein opened this issue Sep 13, 2017 · 6 comments
Labels
bug An actual error or unwanted behavior.

Comments

@RyanStein
Copy link
Contributor

Brief summary of issue / Description of requested feature:

On Evennia 0.6 at least, using a custom exit node runs cmd_on_exit twice.

Steps to reproduce the issue / Reasons for adding feature:

Load the following code into @py/edit and execute.

from evennia.utils.evmenu import EvMenu

def start(caller):
    return "Hello world!", {"desc": "Custom exit node", "goto": "exit"}

def exit(caller):
    return None, None

EvMenu(self, {"start": start, "exit": exit}, startnode="start")

Error output / Expected result of feature

Two look commands are run by the character. This does not occur if you use the default quit command.

Extra information, such as Evennia revision/repo/branch, operating system and ideas for how to solve / implement:

My guess is that close_menu() is being hit twice, possibly in evtable_parse_input and EvMenu.goto.

@Griatch Griatch added the bug An actual error or unwanted behavior. label Sep 13, 2017
@ghost
Copy link

ghost commented Oct 5, 2017

I'll dibs this. It's probably an unexpected result of the last PR I did on EvMenu. Previously single node EvMenus would bug out. It treats the first node differently to the rest of the nodes and didn't expect an EvMenu to end on the first node. I suspect my solution to that problem is running close_menu() twice resulting in the doubling up of the cmd_on_exit. It should be a simple fix.

@ghost
Copy link

ghost commented Oct 6, 2017

So my immediate thought was to simply remove line 408, that is the portion of the evtable_parse_input() that calls menu.close_menu():

def evtable_parse_input(menuobject, raw_string, caller):

    cmd = raw_string.strip().lower()

    if cmd in menuobject.options:
        # this will take precedence over the default commands below
        goto, callback = menuobject.options[cmd]
        menuobject.callback_goto(callback, goto, raw_string)
       *SNIP*
    else:
        caller.msg(_HELP_NO_OPTION_MATCH, session=menuobject._session)

    # if not (menuobject.options or menuobject.default):
    #     # no options - we are at the end of the menu.
    #     menuobject.close_menu()

In favour of line 859 where menu.goto() calls menu.close_menu():

    def goto(self, nodename, raw_string):
    * SNIP*
        self.display_nodetext()

        if not options:
            self.close_menu()

This works as expected but then I think it subverts the initial idea of using the evtable_parse_input(). That being putting the choice in the hands of the user who can design and pass their own parser to EvMenu.

Another option would be removing display_nodetext() and the close_menu() lines all together from goto() and instead calling an altered parser with the look command:

def evtable_parse_input(menuobject, raw_string, caller):

    cmd = raw_string.strip().lower()

    if cmd in menuobject.options:
        goto, callback = menuobject.options[cmd]
        menuobject.callback_goto(callback, goto, raw_string)
    elif menuobject.auto_look and cmd in ("look", "l"):
        menuobject.display_nodetext()
        if not (menuobject.options or menuobject.default):
            # no options - we are at the end of the menu.
            menuobject.close_menu()
       *SNIP*
       # menuobject.close_menu() removed there after.

This mirrors how the unloggedin portion of Evennia works but would act inconsistently depending on which command set merge types is used.

Thoughts? I'm trying to get single node EvMenus to work without significantly reshuffling the code.

@Griatch
Copy link
Member

Griatch commented Oct 11, 2017

@CloudKeeper1 Hm, it's a tough call. I think I prefer the modified parser here though, since a parser seems like a reasonable place to act on input. It also homogenizes the calling of this to one location. Not sure if this would have any other unexpected effects though.

@texdarkstar
Copy link

@Cloudkeeper1 is this still active?

@Griatch
Copy link
Member

Griatch commented Jan 8, 2018

@texdarkstar Do you still see this issue with latest EvMenu? There has been some changes to evmenu in the interim, so one would need to test if this is still a thing.

@texdarkstar
Copy link

I do.
The above code replicates it for me. I am definitely on the latest evennia.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An actual error or unwanted behavior.
Projects
None yet
Development

No branches or pull requests

3 participants