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

LambdaMUD-Project #132

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

Conversation

upsmancsr
Copy link

Copy link

@TheDeterminator TheDeterminator left a comment

Choose a reason for hiding this comment

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

Cameron, this project looks great! You implemented your code well, you have detailed and frequent commits and you thought ahead by creating functionality for secret_key functionality which is no doubt an aspect of the project that craves reusability. Only thing I want to say is to remind you that the format for naming your pull requests should be: [FirstName LastName] - Name-of-Project. Thanks and great job!

@@ -1,3 +1,5 @@
Cameron's Trello Board URL: https://trello.com/b/cjRCp0Co/lambdamud

Choose a reason for hiding this comment

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

Good that you added a link to your URL here. It's a small thing but many students forget/ignore that instruction.

message = data['message']
room = player.room()
currentPlayerUUIDs = room.playerUUIDs(player_id)
for p_uuid in currentPlayerUUIDs:

Choose a reason for hiding this comment

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

This say command looks great! It might be nice to have a few more comments. Also have you thought about how you might handle the case where a user tries to send an empty message?

@@ -0,0 +1,3 @@
import random
key = ''.join([random.SystemRandom().choice('abcdefghijklmnopqrstuvwxyz0123456789!@#$%^&*(-_=+)') for i in range(50)])

Choose a reason for hiding this comment

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

This is cool, I like how you set a file to do this secret key generation. It's good reusability and accessibility.

@upsmancsr
Copy link
Author

upsmancsr commented Dec 19, 2018 via email

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