screepers / screeps-multimeter

The most useful tool on your screeps workbench.
MIT License
89 stars 28 forks source link

Logging plugin #14

Closed RvstFyth closed 4 years ago

RvstFyth commented 5 years ago

A plugin to save the console log lines to a file. Errors are stored in a separate file. Enable or disable with /log

CGamesPlay commented 5 years ago

Hey! Thanks so much for opening a pull request for this new plugin! Sorry for the delay, I'm traveling internationally and am not at a computer very frequently these days.

I think this plugin is a good idea! I have a few things I'd like to request you change before I include it with the default distribution of multimeter:

Instead of having a command /log to enable/disable, use the config file to set customizable filenames for the logs and automatically enable or disable logging based on that. This will mean that users don't have to run /log every time they start the app. You can look at the alias plugin for an example of how to access the multimeter config.

I suggest that a config key called log.filename could be used to enable the plugin and set the default log file, and another key log.errorFilename could be used to pull errors to a separate file. This way, if users only want a single log file it is very easy for them to do it, and if they want a separate errors file a you prefer, they can simply add the second key and have errors get pulled into it. Using a config file now will also make it easier to add different output formatting options in the future (e.g. stripping HTML / colors, timestamp formats, whatever we decide).

So, before I accept this:

Thanks again for writing this plugin! Hopefully we can get it merged in soon :)

RvstFyth commented 5 years ago

Hi,

Thanks for the feedback, all valid points! Will probably work on it this weekend.

How about keeping the enable/disable command so we can use it without restarting multimeter. logs should be enabled by default then (when specified in the config file). And the command should not work, if these parameters are not set.

CGamesPlay commented 5 years ago

This sounds like a great compromise. Thank you!

On Sat, Jul 27, 2019 at 17:09 Randy notifications@github.com wrote:

Hi,

Thanks for the feedback, all valid points! Will probably work on it this weekend.

How about keeping the enable/disable command so we can use it without restarting multimeter. logs should be enabled by default then (when specified in the config file). And the command should not work, if these parameters are not set.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/screepers/screeps-multimeter/pull/14?email_source=notifications&email_token=AAJJA4WK23KGGYKRP2DDNJ3QBRCH3A5CNFSM4IDZBMOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD26LFJY#issuecomment-515682983, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJJA4VXRUAFYF45X464A2DQBRCH3ANCNFSM4IDZBMOA .

-- ~ry

RavenX8 commented 5 years ago

Requesting a status update on this plugin.

RvstFyth commented 5 years ago

Sorry, i have been too busy with work related things. And as the screeps client is not working for me anymore for a few months already, i kinda lost interest. I have most of the things that are requested implemented already, so it's almost done. I will set a alarm for this weekend, to check and finish it.

pyrodogg commented 5 years ago

I'm actively using multimeter, and would appreciate logging. Willing to test and/or help finish this.

-Skyler

On Thu, 24 Oct 2019, 16:27 Randy, notifications@github.com wrote:

Sorry, i have been too busy with work related things. And as the screeps client is not working for me anymore for a few months already, i kinda lost interest. I have most of the things that are requested implemented already, so it's almost done. I will set a alarm for this weekend, to check and finish it.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/screepers/screeps-multimeter/pull/14?email_source=notifications&email_token=AACQMB7BO4STYKJRKWGA2P3QQGPE3A5CNFSM4IDZBMOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECFAUIY#issuecomment-545917475, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACQMBY4JO2KTQUIGKUUF7DQQGPE3ANCNFSM4IDZBMOA .

RvstFyth commented 5 years ago

I'll be home sunday morning again. I will finish it up then and notify you for testing.

jordansafer commented 4 years ago

Added fixes in PR #21 based on the comments- arguably screeps logging should be a separate package/setup since it's an always running process that doesn't need to be linked to a console, but this makes the multimeter even more useful.

CGamesPlay commented 4 years ago

Resolved by #21