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

Feature Request: Make EvMenu plugin functions part of class #1205

Closed
Griatch opened this Issue Feb 10, 2017 · 2 comments

Comments

Projects
1 participant
@Griatch
Member

Griatch commented Feb 10, 2017

Description of requested feature:

Currently the EvMenu class takes a slew of input kwargs to manipulate its function:

  • nodetext_formatter
  • options_formatter,
  • node_formatter
  • input_parser

The suggestion is to (again) move these back as methods in the EvMenu object and instead of supplying them as functions, simply make a child class from EvMenu and replace the methods as needed.

Reasons for adding feature:

Pros:

  • Class inheritance is how Evennia works elsewhere. EvMenu is the only major system plugging in functions in the __init__ call like this. It makes for an inconsistent API that would be resolved by moving the methods into the EvMenu object.
  • It's a lot harder to read the code when the formatters/parsers are elsewhere; the formatters also defaults to long names (dedent_strip_nodetext_formatter etc) that are hard to read.
  • Inheriting from the EvMenu class is more powerful and flexible. A node formatter cannot be inherited while as a method you could use super() to use part of the parent's functionality.
  • Inheriting also makes it clear to devs that you could tweak other parts of EvMenu to your needs.

Cons:

  • This is a backwards incompatible change. Existing EvMenu customizations need to be updated to use child classes of EvMenu instead.

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

This is something that will go into the development branch for Evennia 0.7.

@Griatch Griatch added this to TODO in Evennia 0.7 Feb 10, 2017

@Griatch

This comment has been minimized.

Show comment
Hide comment
@Griatch

Griatch Feb 13, 2017

Member

The EvEditor does also have functions along these lines, in the form of the save/load/code/quitfuncs. But those tend to be considerably less complex than the formatting functions of the EvMenu.

Member

Griatch commented Feb 13, 2017

The EvEditor does also have functions along these lines, in the form of the save/load/code/quitfuncs. But those tend to be considerably less complex than the formatting functions of the EvMenu.

Griatch added a commit that referenced this issue Feb 17, 2017

@Griatch Griatch moved this from TODO to Done on devel branch in Evennia 0.7 Feb 17, 2017

@Griatch Griatch added the implemented label Feb 17, 2017

@Griatch

This comment has been minimized.

Show comment
Hide comment
@Griatch

Griatch Sep 20, 2017

Member

Closed with the merger of devel branch.

Member

Griatch commented Sep 20, 2017

Closed with the merger of devel branch.

@Griatch Griatch closed this Sep 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment