mail.py #1145
mail.py #1145
Conversation
| A simple Brandymail style @mail system that uses the Msg class from Evennia Core. | ||
|
|
||
| Installation: | ||
| Import this module into the default Player or Character commandset. |
There was a problem hiding this comment.
I guess this should be "import MailCommand from this module into the default Player or Character command set"?
| HEAD_CHAR = "|015-|n" | ||
| SUB_HEAD_CHAR = "-" | ||
|
|
||
| class MailCommand(default_cmds.MuxCommand): |
There was a problem hiding this comment.
For a contrib it's probably best to go with the convention and name it CmdMail for the class name.
| Commands that allow either IC or OOC communications | ||
|
|
||
| Usage: | ||
| {w@mail{n |
There was a problem hiding this comment.
Picky - but since this is new code, it should be using |w and |n rather than {w and {n (I'm really bad with this myself, but I try to slowly convert things, the {-style is deprecated).
| Usage: | ||
| {w@mail{n | ||
| - Displays all the mail a player has in their mailbox | ||
|
|
There was a problem hiding this comment.
Excellent docstring, but please make it in the style of the Evennia Command documentation policy - basically make a summary of the Switches and move the examples to the end. This is not just me being picky - doing so will help the API2md generator automatically create a nice API wiki page for the contrib.
| else: | ||
| self.caller.msg("Sorry, you don't have any messages. What a pathetic loser!") | ||
|
|
||
| def get_all_mail(self): |
There was a problem hiding this comment.
This needs a docstring in Google format. See the Evennia code style guide.
| messages = [m for m in mail_messages if self.caller.player in m.receivers] | ||
| return messages | ||
|
|
||
| def send_mail(self, recipients, subject, message, caller): |
There was a problem hiding this comment.
This needs a docstring in Google format. See the Evennia code style guide.
| from evennia.comms.models import Msg | ||
|
|
||
|
|
||
| HEAD_CHAR = "|015-|n" |
There was a problem hiding this comment.
Since these global variables are not meant to accessible from the outside or listed as contents of the module, a good idea is to start their names with _, so _HEAD_CHAR etc.
|
Also, we don't currently request that contribs come with unit tests, but we are slowly adding ones to the contrib folder as per Issue 1105 so those will eventually have to be written also for this. For now though, I won't require it for merging. :) |
| Commands that allow either IC or OOC communications | ||
|
|
||
| Usage: | ||
| {w@mail{n |
There was a problem hiding this comment.
The { were not corrected, but looking at it, I think it's probably better to ditch the coloration in the help entry. It will not work well with the API docs.
|
|
||
| messageForm = [] | ||
| if message: | ||
| messageForm.append(HEAD_CHAR * _WIDTH) |
There was a problem hiding this comment.
HEAD_CHAR should be _HEAD_CHAR (several more examples in the file - best do a search & replace)
| self.caller.msg("No Message ID given. Unable to delete.") | ||
| return | ||
| else: | ||
| if self.get_all_mail()[int(self.lhs) - 1]: |
There was a problem hiding this comment.
All the cases where you do int(self.lhs) is susceptible to a traceback if the user enters something other than a number. For example @mail blah currently gives a traceback. There are many such crashes in the code below due to invalidated input.
|
|
||
| Usage: | ||
| {w@mail{n | ||
| - Displays all the mail a player has in their mailbox |
There was a problem hiding this comment.
Much better docstring, but it's so long it doesn't fit in a normal-sized terminal. Would it be possible to shorten it down?
|
Actually; scratch removing coloring if you don't want to; this is an optional contrib after all. |
| messages = [m for m in mail_messages if self.caller.player in m.receivers] | ||
| return messages | ||
|
|
||
| def send_mail(self, recipients, subject, message, caller): |
There was a problem hiding this comment.
As a small idea, you may want to do list(Msg.objects.filter(db_receivers_players=self.caller.player, db_tags__db_category="mail").distinct()) rather than doing the in m.receivers check in the list comprehension - having a for loop check membership and hit the database each time could make the command run really slowly if you have a lot of objects, while a filter by player name would only hit it once. I ran into a similar situation using Msg objects for bulletin boards and checking receivers to see read posts.
|
Nitpick: "update mail.py" is not a very informative commit message. Please try to make commit messages useful for your own sanity and ours. :) |
|
My bad. |
In-game mail system.
Brief overview of PR changes/additions
Motivation for adding to Evennia
Other info (issues closed, discussion etc)