Command rewrite - Githubissues

Here's a chatlog about the prorposed change for a rewrite of the commands.
[14:48:15] +wgma◕‿◕: you understand UML diagrams right
[14:50:06] @Quite Quiet: y
[14:54:08] +wgma◕‿◕: alright, I was just thinking about the command rewrite and wanted some opinions
[14:56:24] @Quite Quiet: remember that any command rewrite you do will be useless for my usage unless you support the pokemon command as it is right now
[15:02:37] +wgma◕‿◕: as in like .charizard
[15:02:39] +wgma◕‿◕: ?
[15:02:42] @Quite Quiet: yes
[15:02:49] +wgma◕‿◕: oh yeah that's fine
[15:02:59] +wgma◕‿◕: I'm just trying to encapsulate all of the commands
[15:03:09] +wgma◕‿◕: I'll show you an example in a sec
[15:03:17] +wgma◕‿◕: with the pokemon command
[15:03:46] @Quite Quiet: the only commands that aren't fine are those in commands-py
[15:03:55] @Quite Quiet: I'm fairly okay with how plugins use their commands
[15:04:05] @Quite Quiet: since those are located where you use them
[15:11:35] +wgma◕‿◕: http://imgur.com/a/ySaZ5
[15:11:38] +wgma◕‿◕: so something like this
[15:12:15] +wgma◕‿◕: I kinda added one paramater cause I was also thinking about porting it over to IRC
[15:12:27] @Quite Quiet: pokemon_name_regex
[15:12:31] @Quite Quiet: how are you building this
[15:12:42] +wgma◕‿◕: when you intialize the class
[15:12:45] +wgma◕‿◕: like form an import
[15:12:50] +wgma◕‿◕: the init is called
[15:13:02] +wgma◕‿◕: i.e.
[15:13:10] +wgma◕‿◕: from commands import Pokemon as pkmn
[15:13:19] @Quite Quiet: you realize this regex would be absurdly large right?
[15:13:26] @Quite Quiet: like in the kb range
[15:13:30] +wgma◕‿◕: um
[15:13:39] +wgma◕‿◕: we only ever declare that object once
[15:13:42] @Quite Quiet: yes
[15:13:48] @Quite Quiet: but regex preformance
[15:13:54] @Quite Quiet: on a kb-sized regex
[15:13:54] +wgma◕‿◕: oh wait
[15:13:59] @Quite Quiet: is abyssmal
[15:14:02] +wgma◕‿◕: well I guess
[15:14:08] +wgma◕‿◕: if you wanna just be removed on the stack
[15:14:10] +wgma◕‿◕: that's fine
[15:14:23] +wgma◕‿◕: I just wanted it there so you didn't have to compile it every time
[15:15:02] +wgma◕‿◕: `` pkmn_name_regex = re.sub('-(?:mega(?:-(x|y))?|primal|xl|l)','', cmd, flags=re.I) ``
[15:15:12] +wgma◕‿◕: pkmn_name_regex = re.sub('-(?:mega(?:-(x|y))?|primal|xl|l)','', cmd, flags=re.I)
[15:15:22] +wgma◕‿◕: like I think this gets compiled on each call to Command?
[15:15:46] @Quite Quiet: depends on python optimization
[15:16:17] @Quite Quiet: but probably it gets compiled once at the start of the loop yes
[15:17:06] +wgma◕‿◕: this just an example of how the hierarchy should work btw
[15:17:24] +wgma◕‿◕: actual implementation of the commands is dependent on how you wanna do it
[15:17:45] +wgma◕‿◕: but my motivation for the rewrite is to have something like, better error messages on some commands
[15:18:07] +wgma◕‿◕: so the way it is now, when we detect something wrong lets say in the response method
[15:18:22] +wgma◕‿◕: we can make a call to the error method and return something alright
[15:18:26] +wgma◕‿◕: I mean
[15:18:30] +wgma◕‿◕: a useful error message
[15:19:02] +wgma◕‿◕: and I also wanted to encapsulate some stuff related to these commands only
[15:19:18] +wgma◕‿◕: i.e. in my version I currently have a dict for markov chains per room
[15:19:24] +wgma◕‿◕: which is in the Robot class
[15:19:26] @Quite Quiet: tbh, sounds like you more want to expand on the reply object
[15:19:32] @Quite Quiet: than the actual commands themselves
[15:19:35] +wgma◕‿◕: but it really shouldn't be there
[15:19:56] +wgma◕‿◕: but that's not the functionality of replyobject though
[15:20:05] +wgma◕‿◕: it's not meant to handle the logic of the command itself
[15:20:18] @Quite Quiet: no but it might as well reply with an error
[15:20:18] +wgma◕‿◕: it's just a way to format the response to the user
[15:20:24] @Quite Quiet: as an actual response
[15:20:34] @Quite Quiet: an error is a response
[15:20:38] @Quite Quiet: per definition
[15:20:43] +wgma◕‿◕: sure yeah
[15:20:58] +wgma◕‿◕: but we have replyobject as a container right for a message
[15:21:46] +wgma◕‿◕: I want to handle each command's logic in it's own class, seperate from the main Command function
[15:22:11] +wgma◕‿◕: because otherwise it's going to get alot more messy as we add stuff more complicated commands
[15:22:26] @Quite Quiet: so this? https://github.com/QuiteQuiet/PokemonShowdownBot/blob/master/plugins/init.py
[15:22:37] +wgma◕‿◕: basically yeah
[15:22:43] +wgma◕‿◕: wait
[15:22:49] +wgma◕‿◕: what I mean is
[15:22:58] +wgma◕‿◕: we could move most of the commands to the plugins folder
[15:23:17] @Quite Quiet: that's already possible, bar the pokemon command
[15:23:29] @Quite Quiet: without changing anything
[15:23:47] +wgma◕‿◕: yeah agreed
[15:23:58] +wgma◕‿◕: we could move everything and have it works like the game class
[15:24:01] @Quite Quiet: if python supported anonymous functions in dicts I would already have done it
[15:24:14] @Quite Quiet: but it doesn't and I don't want to def all of those
[15:24:16] +wgma◕‿◕: where we map each command to the response method of the game
[15:24:52] +wgma◕‿◕: what do you mean by def
[15:25:34] @Quite Quiet: the dict in plugins is function pointers called by the dispatcher in commands.py, but you can't put anoynymous functions in a dict
[15:25:40] @Quite Quiet: so you have to define them with a name
[15:25:43] @Quite Quiet: then put them it
[15:25:44] @Quite Quiet: in*
[15:25:53] +wgma◕‿◕: hold on
[15:26:08] +wgma◕‿◕: by anonymous functions you mean like lambdas?
[15:26:11] @Quite Quiet: yh
[15:26:22] @Quite Quiet: but lambdas are single-statement
[15:26:30] @Quite Quiet: so they're not useful for much
[15:26:33] @Quite Quiet: :(
[15:26:36] +wgma◕‿◕: h
[15:26:38] +wgma◕‿◕: uh
[15:26:46] @Quite Quiet: not for that usage anyway
[15:27:16] +wgma◕‿◕: https://github.com/wgma00/quadbot/blob/master/plugins/games/anagram/anagram.py#L157
[15:27:18] +wgma◕‿◕: you should be able to
[15:27:23] +wgma◕‿◕: but you have to declare them first
[15:27:56] @Quite Quiet: but that's the same as defining the function with def
[15:28:04] @Quite Quiet: which was what I didn't wanna have to do
[15:28:15] +wgma◕‿◕: isn't that how lambdas are suppose to work?
[15:29:09] @Quite Quiet: you can do {'test':lambda x: x**2} just fine
[15:29:16] @Quite Quiet: without naming it
[15:30:15] +wgma◕‿◕: huh
[15:31:33] @Quite Quiet: https://puu.sh/waQya.png
[15:32:05] +wgma◕‿◕: oh ok
[15:32:14] +wgma◕‿◕: but uh
[15:32:29] +wgma◕‿◕: what were you saying about dicts not supporting anonymous functions?
[15:32:33] +wgma◕‿◕: this is a clear example
[15:32:46] @Quite Quiet: not supporting multi-line functions
[15:32:53] @Quite Quiet: it supports lambdas, fine
[15:32:59] +wgma◕‿◕: oh
[15:33:04] @Quite Quiet: but I can't do anything that is more than a single statement
[15:33:10] +wgma◕‿◕: well uh
[15:33:19] @Quite Quiet: in a lambda
[15:33:43] +wgma◕‿◕: I don't think lambdas should be more than one line
[15:34:12] @Quite Quiet: no no I agree, but I would've liked if I could do something like javascript
[15:34:31] +wgma◕‿◕: I'm not familiar enough with javascript to know that was a thing :p
[15:34:45] @Quite Quiet: PS uses it everywhere
[15:35:32] +wgma◕‿◕: sure
[15:35:56] +wgma◕‿◕: can we talk more about that replyobject bit, about extending it's functionality with errors
[15:36:03] @Quite Quiet: sure
[15:36:17] +wgma◕‿◕: so
[15:36:26] @Quite Quiet: though, it depends what errors you're looking to catch
[15:36:36] @Quite Quiet: user errors or program errors, or both?
[15:36:43] +wgma◕‿◕: both
[15:36:57] +wgma◕‿◕: sometimes in my latex commands where I make system calls
[15:37:00] +wgma◕‿◕: I get an error
[15:37:12] +wgma◕‿◕: so something like "compilation error" would be a nice indicator
[15:37:17] +wgma◕‿◕: that something went wrong
[15:37:20] +wgma◕‿◕: instead of just getting silence
[15:38:15] @Quite Quiet: well, the easiest that I can think of is catching every exception inside the command itself, and reply with the exception message python sends
[15:38:55] +wgma◕‿◕: sure
[15:39:18] +wgma◕‿◕: but I feel that would look messy the way it is now
[15:39:31] +wgma◕‿◕: since I at least mostly put things into the Command function itself
[15:40:04] +wgma◕‿◕: so if we move everything we have currently to plugins
[15:40:08] @Quite Quiet: I did this less than a week ago https://github.com/QuiteQuiet/PokemonShowdownBot/blob/master/plugins/battling/battleHandler.py#L196
[15:40:21] @Quite Quiet: it wouldn't be very messy to take a try block around the important bits here
[15:40:39] +wgma◕‿◕: well I mean
[15:40:45] +wgma◕‿◕: if you do validation for every command
[15:40:51] +wgma◕‿◕: and leave it in that one file
[15:40:57] +wgma◕‿◕: you're gonna see a whole lot of try blocks
[15:41:15] +wgma◕‿◕: and it'd just take away the flow aspect of the code
[15:41:26] @Quite Quiet: well there's an alternative you can do then
[15:41:38] @Quite Quiet: which would probably do it a bit better
[15:41:51] @Quite Quiet: (but it'd still mean you have to make use of the plugins for commands)
[15:42:54] +wgma◕‿◕: I also think this way would be better since we treat everything is a standalone program anway, so might as well encapsulate the commands in their own thingy
[15:43:09] +wgma◕‿◕: meant to say that a while back
[15:43:19] @Quite Quiet: https://hastebin.com/erahukaqad.py
[15:43:27] @Quite Quiet: only as a reason to show what I meant
[15:43:36] @Quite Quiet: when I say you can generalize it
[15:43:55] +wgma◕‿◕: > # External commands from plugins (and also room.py)
[15:44:01] +wgma◕‿◕: there are commands in room.py ? O.o
[15:44:08] @Quite Quiet: 1 command
[15:44:15] +wgma◕‿◕: the tour thingie?
[15:44:16] @Quite Quiet: which I had no other place to put it
[15:44:17] @Quite Quiet: y
[15:44:20] @Quite Quiet: it made sense there
[15:44:20] @Quite Quiet: :(
[15:44:30] +wgma◕‿◕: hmm I see
[15:44:34] @Quite Quiet: it was a command that affected room permissions
[15:44:42] @Quite Quiet: so it was in the place where rooms were
[15:44:44] +wgma◕‿◕: ah
[15:45:01] +wgma◕‿◕: but uh,
[15:45:19] @Quite Quiet: if you replace that # do something with something more sensible
[15:45:23] +wgma◕‿◕: you could also affect room permisions from outside the room.py
[15:45:30] @Quite Quiet: yes
[15:45:34] @Quite Quiet: if you wanted to
[15:45:42] @Quite Quiet: but that's true for anything atm
[15:45:50] +wgma◕‿◕: yeah
[15:45:54] @Quite Quiet: and even your solution wouldn't entirely solve that
[15:45:54] +wgma◕‿◕: and in general for python :p
[15:46:02] +wgma◕‿◕: it's not meant to
[15:46:09] +wgma◕‿◕: it's just meant to be a way to organize things better
[15:46:29] +wgma◕‿◕: organization through encapsulation I think
[15:47:00] +wgma◕‿◕: though I'm thinking about functions having side affects now ._. which is worrisome
[15:47:08] @Quite Quiet: the only thing is a lot of commands make no sense without a object to work on
[15:47:18] @Quite Quiet: so something like anagram
[15:47:19] +wgma◕‿◕: which object?
[15:47:22] @Quite Quiet: can't be entirely hidden
[15:47:36] +wgma◕‿◕: like the PSRobot object?
[15:47:45] @Quite Quiet: no like a game object
[15:47:51] +wgma◕‿◕: oh
[15:47:57] +wgma◕‿◕: yeah I was thinking about that
[15:48:05] @Quite Quiet: you could make it as static methods in the class
[15:48:10] @Quite Quiet: but it doesn't change much
[15:48:11] +wgma◕‿◕: but I feel like I'm thinking into a too java-ish way
[15:48:25] +wgma◕‿◕: but we could have game as like an abstract class of sorts
[15:48:29] +wgma◕‿◕: then just extend it
[15:48:33] +wgma◕‿◕: but idk if that's a thing in python
[15:48:42] @Quite Quiet: that /kinda/ what is being done atm tho
[15:48:42] +wgma◕‿◕: or if python supports multiple inheritance
[15:49:00] @Quite Quiet: https://github.com/wgma00/quadbot/blob/master/plugins/games/anagram/anagram.py#L44
[15:49:09] @Quite Quiet: Anagram inherits from GenericGame
[15:50:06] +wgma◕‿◕: could't we refactor the GenericGame into the CommandBase class I want to implement
[15:50:22] +wgma◕‿◕: it's like 1 if statement
[15:50:26] +wgma◕‿◕: or I mean, 1 bool
[15:50:38] +wgma◕‿◕: and the way we'd have it
[15:50:44] +wgma◕‿◕: games wouldn't be much different from other commands
[15:50:45] @Quite Quiet: not everything in plugins make use of it
[15:50:48] @Quite Quiet: :(
[15:51:01] @Quite Quiet: the moderation block for example isn't even a class
[15:51:07] +wgma◕‿◕: oh right
[15:51:11] @Quite Quiet: even though I should probably make it to one
[15:51:12] +wgma◕‿◕: but moderation isn't a command
[15:51:18] +wgma◕‿◕: or is it
[15:51:19] +wgma◕‿◕: ?
[15:51:25] +wgma◕‿◕: oh .ab
[15:51:36] @Quite Quiet: there's commands for moderation
[15:52:36] +wgma◕‿◕: oh right, another thing we could do is like
[15:52:46] +wgma◕‿◕: add commandchar to the comandbase
[15:52:52] @Quite Quiet: I did
[15:53:00] @Quite Quiet: already
[15:53:07] +wgma◕‿◕: wait
[15:53:17] +wgma◕‿◕: I don't have this in my repo do I
[15:53:32] +wgma◕‿◕: cause I mean, my version we still assume that you use ~ as commandchar
[15:53:33] @Quite Quiet: https://github.com/QuiteQuiet/PokemonShowdownBot/commit/2e9f4b51cec44964dfeddebd35c707fc69842ee4
[15:53:38] +wgma◕‿◕: and it confuses people since I use .
[15:53:54] @Quite Quiet: I did that a while back
[15:54:37] +wgma◕‿◕: uh
[15:54:52] +wgma◕‿◕: but that would require I pass the robotclass itself
[15:54:56] +wgma◕‿◕: to some commands
[15:55:03] @Quite Quiet: nah it's a module global
[15:55:05] +wgma◕‿◕: though I guess that's how you have it right now
[15:55:21] @Quite Quiet: and every command needs to import the reply object anyway
[15:55:29] @Quite Quiet: so you import the command char as well
[15:55:44] +wgma◕‿◕: wait
[15:55:52] +wgma◕‿◕: but this is in the psb class
[15:56:01] +wgma◕‿◕: I don't see how it's in the replyobject
[15:56:21] @Quite Quiet: robot.py have the reply ojbect class definition in it
[15:56:27] @Quite Quiet: object*
[15:56:42] +wgma◕‿◕: oh right
[15:56:50] +wgma◕‿◕: should it though?
[15:57:17] @Quite Quiet: I was lazy
[15:57:19] @Quite Quiet: :)
[15:57:22] +wgma◕‿◕: :p
[15:58:03] +wgma◕‿◕: uh, how long are you gonna be here for?
[15:58:07] +wgma◕‿◕: I gotta do some laundry
[15:58:17] @Quite Quiet: idk I might sleep in like 50 mins or so
[15:58:29] +wgma◕‿◕: ah alright, I guess we can continue some other time then
[15:59:37] @Quite Quiet: just to make things clear, I'm not against the change, I just want to get something that's actually strictly more useful than the current correct use
[16:00:36] +wgma◕‿◕: yeah that's fine, I wanted to figure out how this current model would play out with the current code
[16:00:42] +wgma◕‿◕: *new model
[16:02:03] +wgma◕‿◕: uh do you mind if I post an issue to github with this chatlog?
[16:02:16] @Quite Quiet: I guess not
wgranados / qbot

Command rewrite #16
