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

Cmd default msg share #1204

Closed
wants to merge 3 commits into from
Closed

Conversation

TehomCD
Copy link
Contributor

@TehomCD TehomCD commented Feb 10, 2017

Brief overview of PR changes/additions

For a while, I had been using 'self.msg' in all my commands as a shortcut to sending messages to the command's caller. After enabling multisession mode, players were confused about not receiving echoes to their other sessions from commands. I decided to put in a way to change the base behavior of the Command class by making it look in the settings file, while keeping existing behavior the same for anyone who doesn't change the defaults.

Motivation for adding to Evennia

Make Command defaults more customizable. The options I included are changing the default arg_regex, the default help_category, whether self.msg will be shared across sessions, and default locks.

Other info (issues closed, discussion etc)

N/A

@@ -347,6 +347,11 @@
# default class logs channel messages to a file and allows for /history.
# This setting allows to override the command class used with your own.
CHANNEL_COMMAND_CLASS = "evennia.comms.channelhandler.ChannelCommand"
# These specify defaults to the base Command parent class
Copy link
Member

Choose a reason for hiding this comment

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

All of these require individual comments. Stuff like MSG_SHARE is not intuitive!

@BlauFeuer
Copy link
Contributor

Would it be a better practice to ...

use character.msg to send the message to all sessions puppeting the character, and player.msg to send to all sessions of the player instead of having an alternate version of msg or having the new default msg checking default settings?

The confusion I have with the use of caller.msg or self.msg is not knowing without inspecting the command more fully if the caller or self is a specific session, or a player or character object.

@TehomCD
Copy link
Contributor Author

TehomCD commented Feb 11, 2017

You don't know from the base Command class what caller is, necessarily, it's just duck typing through the shared msg() method of player/character/session. We could get the behavior you describe by modifying how msg behaves in MuxCommand and MuxPlayerCommand, but I'm not sure if that would be more intuitive - I personally think having a clear default specified is much more explicit, and I'd rather be able to modify the default behavior globally than have to do it in every command or every call.

@Griatch
Copy link
Member

Griatch commented Feb 11, 2017

@BlauFeuer The reason for Command.msg to exist is in order to get access to the session using the command. The Command knows this, the Player/Character objects does not. Using self.msg in a Command is a shorthand for self.caller.msg(text, session=self.session), which in turn is a shorthand for checking if caller.player exists and send to that and otherwise send to an object.
The main purpose for using self.msg is specifically to not have to send to all Sessions connected to the Player; Evennia allows to puppet multiple Characters from different Sessions, getting both their returns to all Sessions would make that arrangement unplayable.

@TehomCD I presume you are making this session suggestion since Arx does not use the full power of MULTISESSION=3, where multiple Sessions can connect to one of multiple characters? If you did, this change looks to be able to confuse returns from several different Characters. But anyway, I guess it could be good to make it up to the dev how they want it. I don't have time to fully review and merge this right now, but I'll look at it soon.

@TehomCD
Copy link
Contributor Author

TehomCD commented Feb 11, 2017

@Griatch I switched Arx to using MULTISESSION=1 at player request, since people liked being able to connect from multiple devices, and people immediately began to complain about what they saw as unexpected behavior. While people might argue changing the default entirely would make sense, I thought the best approach would just be to let people customize it in whatever way fits their game while leaving the existing default behavior unchanged.

@Griatch
Copy link
Member

Griatch commented Feb 11, 2017

I guess I can see it being confusing in MULTISESSION_MODE=1 in very limited situations. The thing is that even though you can only control one Character at a time in this mode, you can go OOC in one Session while staying connected to your character with another. If your OOC level has some functionality (like chargen, options, whatever) I suspect that you will be very confused if the two sessions (one OOC, the other IC) all receive the same return.
I'm not arguing against letting this be optional in the settings as you suggest, but it sure should not be the default for any mode.

@Griatch
Copy link
Member

Griatch commented Feb 12, 2017

Merged with a rebase and some further tweaks to the names and docstrings (see 80f7bb1).

@Griatch Griatch closed this Feb 12, 2017
@TehomCD TehomCD deleted the cmd_default_msg_share branch February 12, 2017 10:15
@BlauFeuer
Copy link
Contributor

@TehomCD said "For a while, I had been using 'self.msg' in all my commands as a shortcut to sending messages to the command's caller."

@Griatch said "The main purpose for using self.msg is specifically to not have to send to all Sessions connected to the Player."

Most of the time, the session that initiated the command is the only session that should see the output, and based on what you've both said, this is the default behavior.

Then I asked:
"Would it be a better practice to use character.msg to send the message to all sessions puppeting the character, and player.msg to send to all sessions of the player instead of having an alternate version of msg or having the new default msg checking default settings?"

The reason I asked is, when output should go to all sessions connected to a character or all sessions going to a player, using self.msg or caller.msg isn't the way to do it, correct?

My question still seems unanswered to me, probably because I was confused before this PR. Additionally, I'm confused because I don't understand how changing the default behavior of self.msg/caller.msg globally makes sense over just choosing to send to all player or all character sessions in the context of the command instead of sending only to a single session on the calling object.

I just haven't found a clear statement documenting the these three situations, which for clarity are listed below with my guess. Please help me and explain if I'm right, wrong, or somewhere in between:

  1. Sending a message only to the calling session from within the command:
    self.msg or caller.msg
  2. Sending a message to all sessions of the player who triggered the command from within the command:
    self.player.msg This is the same as player.msg, correct?
  3. Sending a message to all sessions puppeting a character from within the command:
    self.character.msg This is the same as character.msg, correct?

@Griatch
Copy link
Member

Griatch commented Feb 12, 2017

@BlauFeuer

The reason for the differences is that the unique Session is not available to all objects at all times.

  1. Only self.msg will send specifically to the calling Session since the Command itself has access to the Session and set up the connection under the hood (this is what the Tehom's setting turns off). If you use self.caller.msg and caller is a Character, you will send to all Sessions attached to puppet this Character (if mutlisession mode 3). If you want to specify a single Session from inside the Command you must do so manually with self.caller.msg(text, session=self.session). If caller is a Player it's even vaguer, since the Player stores all Sessions, many of which (in some multisession modes) may point to completely different puppets. If you don't specify the session explicitly as self.caller.msg(text, session=self.session) your return will go to all Sessions connected to the Player since there is no way for the Player to know which you want otherwise. Only if caller is a Session will self.caller.msg(text) automatically go to the right Session, since well, it's running on the Session itself.
  2. self.player.msg is indeed identical to Player.msg, self.player is just a stored reference to the player puppeting the character this command sits on (or the player itself if the command sits directly on the player). This is set by MuxCommand.parse, it's not default for the base Command.
  3. self.character.msg is the same call as Object.msg, the self.character is a back-reference to an eventual Character running the command (or None if the command sits on the Player). This is also something set up by MuxCommand, it's not standard on a base Command.

The Command's self.msg(text) is a convenience shortcut for caller.msg(text, session=self.session) in order to send to specific Session. For Multisession mode=0 and 1 it is not very important but with multiple sessions it starts to matter just where the return data is going. In MULTISESSION_MODE > 0 you need to be aware of this for Commands on the Player cmdset (for example you may not want a channel's text to echo the same to all your sessions). In MULTISESSION_MODE 3 you need to be careful also on the Character cmdset since multiple Sessions can connect to the same Character and may not all want to receive the same returns there either.

@BlauFeuer
Copy link
Contributor

So all of the confusion stems from the fact that caller is sometimes a session, sometimes a puppet object, and sometimes a player. This PR does, among other things allow a setting to change whether self.msg will be shared across sessions - but that doesn't mitigate the ambiguity of what the calling object actually is in the context of any given command.

What's a better way, or a best practice then? Obviously if it tripped up Tehom, then the expected behavior isn't always the actual behavior.

@Griatch
Copy link
Member

Griatch commented Feb 12, 2017

(Edit: I removed the previous post, I remembered it wrong). Basically, if you know at which level you store your Command (Session, Player or Object) then you also know what self.caller is. It's only for commands that are intended to be used for, say, both IC and OOC where you need to worry about this. Again, this is not what Tehom made the PR for however.

@BlauFeuer
Copy link
Contributor

Agreed, "whether self.msg will be shared across sessions" is only part of what this PR is for, but …

that particular behavior varies per use unless, now thanks to a section of this PR, it is set globally. I don't always know at what object level the Command is stored, which is why I am asking what is the best practice when coding. It's more obvious to me where the message is going when the object variable isn't caller or self, but character or player; that is the only way I know for sure when I later look back on code I have written.

I'm assuming that the same command can be stored at different levels on different objects, because a single command can be included in multiple command sets, or the same command set can be stored on different objects. It takes awhile for an initiate to understand the subtleties.

@Griatch
Copy link
Member

Griatch commented Feb 12, 2017

The MuxCommand introduces the Command.character and Command.player variables just for the case where one want to always get one or the other. It basically analyzes the generic self.caller for you to produce this split. This is not in the base level Command since in principle Commands could well sit on some other sort of structure with another relation than Evennia'a default Player->Character duality (such as a custom model).

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.

None yet

3 participants