Add shard support to Watch plugin (Fixes #32)

[InValidFire]

This fixes #32, however it does not expand the /shard command as suggested in that issue. I opted to create a secondary command /wshard in the Watch plugin to manage shard selection for the plugin. The functionality for the command is identical to the original /shard command on the user's end.

My reasoning for this is to keep plugin territory separate from the core application while accomplishing what we needed.

Upon /wshard's usage, it will also save the given shard to the config file under the name watchShard. I mirrored this functionality to the original /shard command, keeps these values updated.

[SystemParadox]

Hi, thanks very much for this.

I've been wondering how best to integrate this with /shard, or whether it should be integrated at all. Keeping this totally separate is an interesting idea. I think there's a strong case for wanting to run commands on one shard but keep watching another shard, particularly if like me you've got most of your empire on one "primary" shard.

My only concern is whether it might be confusing having extra commands and two different states. What do you think? Are you specifically wanting them separate, or would it still make sense to you if watch tied in with the existing /shard command?

Some other thoughts:

• If we do decide to go with separate commands, my vote would be for /watchshard rather than /wshard
• I don't like the config file autosaving. I'd prefer the config file to store the "default" shard(s) so you always start in a known state. Autosaving is also annoying if you've committed the config file to the repository (obviously this is only valid for private repositories, if you do this in a public repository it will expose your API token!). Perhaps we could consider a --save option for the shard commands?

@CGamesPlay, you might want to weigh in on this one!

[InValidFire]

Tie /wshard and /shard together; There are two things I dislike about this:

• While working on this patch I initially implemented it this way as a modification to /shard, syntax was modified to be the following "/shard MODE SHARD", however I decided against this in the committed version as it felt messier, having the core application control specific functionality of a specific module (whether built-in or not) doesn't seem right if we're trying to keep plugins their own thing.
• I also dislike the idea of completely changing already established command syntax... I feel that might confuse users more than splitting it up.
• It is definitely doable, the implementation of this command solely depends on the design choices we wish to make here, which is out of my jurisdiction alone.

Avoiding confusion; I made a few changes to ensure this didn't happen:

• Watch will send the message, 'Watch plugin disabled: no expressions found for active shard.' in the event it sees your memory data, but no expressions are found. (aka you're on a shard without expressions)
  • We could easily add '(Use /wshard to change the active shard)' to the end of the message.
• I modified the text that both /shard and /wshard output when listing the active shard to be 'Command Shard: SHARD' and 'Watch Shard: SHARD' respectively, to further illustrate that the two are separate values.
• Then there's the simple help text:
  • /shard - Set or show the destination shard for commands.
  • /wshard - Set or show the shard used by the Watch plugin.

Saving:

• I do see the appeal in a default config, the idea that someone would use it that way completely slipped my mind. I can either add the --save option or just move these changes back and modify how Watch reads the active shard a bit.

I appreciate the feedback, definitely look through the code if y'all haven't already, always helps to have a second set of eyes.

Oh yeah, with regard to changing the command name, it doesn't really matter at the end of the day what it's called, /wshard is just faster to type. Since it's part of the watch plugin, I felt we didn't need to spell it out haha, but this can of course change.

[SystemParadox]

I initially implemented it this way as a modification to /shard, syntax was modified to be the following "/shard MODE SHARD", however I decided against this in the committed version as it felt messier

If we're thinking of tying it together I meant completely tied, so /shard shard0 would set the command shard and the watch shard, and there would be no way of having them be different.

One other suggestion if we're keeping them separate would be to add this to /watch, so the plugin still provides a single command:

/watch console [expr] /watch status [expr] /watch shard [shardName]

So you would do /shard 0 to change the command shard and /watch shard 0 to change the watch shard.

Avoiding confusion; I made a few changes to ensure this didn't happen:

• Watch will send the message, 'Watch plugin disabled: no expressions found for active shard.' in the event it sees your memory data, but no expressions are found. (aka you're on a shard without expressions)
• I modified the text that both /shard and /wshard output when listing the active shard to be 'Command Shard: SHARD' and 'Watch Shard: SHARD' respectively, to further illustrate that the two are separate values.

If we decide to go for separate commands then this definitely helps to clarify what's going on :). Adding the hint to the "no expressions found" message is a good idea as well.

So the options at the moment are:

1. /shard 0 changes both command shard and watch shard at the same time
2. /watch shard 0
3. /wshard 0

[InValidFire]

Ah, if tying it together means removing the individual shard control I'd prefer we do otherwise. As you mentioned in the first comment, I like the idea of being able to watch one shard but issue commands on another should the user desire.

An interesting idea for tying it together and still allowing the user to specify shards if needed that I didn't think of before would be modifying the /shard command to accept flags for the target to change.

1. /shard 0 - set both the watch and command shard
2. /shard 1 -w - set the watch shard
3. /shard 2 -c - set the command shard

This would fit well with the -save flag we may implement with this.

The only possible problem with this method is if you don't know about these flags the feature doesn't exist, but i'm sure that'd be easily fixable with some help text modification.

However, any option that completely removes the ability of specifying the individual shard to change should be avoided.

[CGamesPlay]

Hi @InValidFire, thanks for contributing! After reading over the discussion happening here I'm wondering if maybe a more holistic view on shards should be taken?

• Presently, log lines show which shard they originate from. Do watch messages not have this behavior? If not, perhaps they should.
• Is there presently anything in the interface that shows which shard a command will be issued on? Perhaps it makes sense to add a decoration to the input bar that indicates which shard the command will be directed to.
• Is there a technical reason why I cannot have a watch on each shard? That way I would simply use /watch shard0 foo and /watch shard1 bar and each watch would run independently, and as indicated above it would be clear which shard the command was issued from.

In the more-distant future it might make sense to switch to a tabbed interface like slack, where each each is a different "channel". This might help alleviate all of these issues. I'm not asking for this to be implemented here, but that mindset (shards are like completely separate instances of screeps and multimeter can just read from all of them) might help make the implementation of this more intuitive.

[InValidFire]

The reasons for the secondary command originally, as you saw was partially because of me being unwilling to change any of the established command syntax if I could avoid it, no need for the added confusion there. If we're all good with changing the syntax for either the /shard or /watch command I can get to work on modifying those once we figure out how we want to change it.

While I really like the idea of listening to all shards from a broad standpoint. The limitations I can think of with this mostly comes down to the current implementation of the status bar. I feel like having each shard's watch data being sent to one bar fighting for dominance would be a bad idea. If we wanted to go the route of listening to all shards like the console, we'd have to figure out how to address the issue with the status bar. There are no issues that come to mind with handling console messages via the plugin. (however I feel the status bar is the more used feature of the Watch plugin, at the very least personally)

[CGamesPlay]

I agree with your assessment about the status bar. I don't understand why there should be 2 commands, and while I agree that /shard shouldn't know about the existence of /watch, I don't understand why /watch shouldn't look at the current command shard to dictate its behavior.

So, to rephrase my original comment in more actionable terms, I believe it should look like this:

• Console watches log the shard they come from, like normal console messages do.
• /watch console affects only the current command shard, but you could switch to another shard and run another watch for that shard.
• The input bar should show the current command shard (this might be another PR, it's just a matter of modifying a property).

Then there's a choice about the status bar. I only see 3 reasonable options:

• If /watch status always executes on the current command shard:
  • The input bar having the current command shard will prevent anything on screen from being confusing.
• If /watch status always stays on the same shard even after changing the command shard:
  • The status bar should include the name of the shard that it's running on.
• If multimeter had a new interface where each shard was like a separate slack channel:
  • Each tab should have a unique status bar. Google for pictures of "irssi" to get an idea of what I'm imagining.
  • (This is not a real suggestion, we should absolutely merge this PR without blocking on a complete app redesign.)

I don't have a preference about which behavior is taken, because I haven't used multiple screeps shards much in the past. Both seem reasonable to me, so perhaps if one is technically easier to implement it makes sense to just go with that option.

[InValidFire]

Here's my suggestion on how we handle this:

• Console watches all shards and log which messages come from where.
• Input Bar showing current active shard should be a separate PR as that change doesn't directly relate to the issue this one aims to fix.
• The watch and command shards stay separate, and the watch status bar displays the watch shard. both are modifiable through the /shard command. The new syntax for this command should be as follows:
  • /shard SHARD - set the command shard and watch shard if plugin is active.
  • /shard SHARD -w - set the watch shard.
  • /shard SHARD -c - set the command shard.
  • /shard {args} -save - save the shard(s) that would be set by the given args to the config file.

Currently the only way I know we can do this is to add the logic directly to the function that controls the /shard command, breaking my original intent of keeping the command shard logic and watch shard logic completely separate.

I'd suggest later down the line that we add the ability for plugins to wrap other command functions somehow (probably with checks to ensure no abuse by 3rd party plug-ins), thus being able to move the watch related shard logic outside of the /shard function. However for now, until we get something like this implemented... I personally would be happy with what we have here.

[SystemParadox]

If we can avoid two having different shard states then I'd really like to do that.

It seems like a reasonable compromise to subscribe to all shards for "watch console", but have "watch status" use the current shard set with /shard. This still gives you the ability to watch something on a different shard, just not the status.

That said, is there a reason why we can't just watch status for all shards as well? Perhaps with an option to display the values as columns on one line, or as separate lines, depending on how long your status information is?

I think we will need to add a watchShards option to tell watch which shards it should subscribe to, because I don't see any way to tell which shards are even available on the server, let alone which the player is using. We could auto-add this to the config file when adding a new watch expression perhaps?