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

Adds menus tutorial to wiki #447

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

srpg
Copy link
Contributor

@srpg srpg commented May 29, 2022

Adds at wiki a tutorial how to make menu.
Paged, Simple and List menu

Copy link
Contributor

@jordanbriere jordanbriere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Here are some points I've noticed while hovering the changes.

That said, I'm not sure how I feel about the examples themselves. Menus should be globalized and re-used, and these examples currently build them on-the-fly which encourages bad practices in my opinion. Perhaps we could leverage the examples that were provided on the forums which also happens to demonstrate more concepts and features:

https://forums.sourcepython.com/viewtopic.php?f=5&t=567

Copy link
Contributor

@jordanbriere jordanbriere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have now updated the wiki tutorial based of your comments.

Thank you! Here are some minor points.

Should in wiki menu tutorial include using WeaponClassIter for making buymenu?
As example looping the weapons showing for their name and their price and it would check player can afford to buy it

I don't think wiki tutorials should showcase more than minimal examples. Both for simplicity and straightforwardness so that they don't bifurcate too much into off-topic modules or become too much games specific. Is there anything pertaining to menus such example would demonstrate that is not already by the current ones?

In any case, this sounds like a good idea for the Code examples / Cookbook section, though.

@@ -0,0 +1,117 @@
Menus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be all lowercase, so that the presentation is consistent with the other tutorials on the index:

Index

Comment on lines 31 to 35
menu = PagedMenu(title='Welcome menu', description='Choose an option:', select_callback=my_select_callback)

# Add options from 1 to 20
for i in range(1, 20):
menu.append(PagedOption(f'{i}', i))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be declared after my_select_callback because it currently won't load and produce a NameError. Moreover, the first line is too long and does not conform with PEP 8, which we try to follow and promote for this project.

# Add in menu options
menu.append(SimpleOption(1, 'Yes', 'yes'))
menu.append(SimpleOption(2, 'No', 'no'))
menu.select_callback=my_menu_select_callback
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above; this should be declared after my_menu_select_callback. And it should also have spaces surrounding the operator:

variable = value

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have now more updated the wiki based of your comments.
I also added when selects close button, it registers it instead closing the menu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants