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

Log Fixes and Improvements #118

Closed
rkathey opened this issue Jul 14, 2016 · 15 comments
Closed

Log Fixes and Improvements #118

rkathey opened this issue Jul 14, 2016 · 15 comments
Labels
bug feature Adding functionality that adds value tested This issue has been QA tested by someone other than the developer.

Comments

@rkathey
Copy link
Contributor

rkathey commented Jul 14, 2016

  1. Fix the console problem with spaces being in the java path
  2. add macro function to write data to log file - something along the lines of log(String text, bool debug) when the debug flag is on, only write to the log.txt when macro logging is turn on.
  3. add a feature to dump text to a file in the .maptool directory so GMs and write data from MapTool for use while offline (like character data and game stats).
@rkathey rkathey added bug feature Adding functionality that adds value labels Jul 14, 2016
@rkathey rkathey added this to the 1.4.3.x Dev Build milestone Jul 14, 2016
@Azhrei
Copy link
Member

Azhrei commented Aug 20, 2016

For (2) and (3), I'm not sure it's a great idea for macros to be able to write to files, even a log file, as it could easily degenerate into a denial of service if a macro filled up a partition with data. One solution would be to to use rotating files: when the current file hits a certain size, close it, rename it, create a new one; then, limit the number of rotated files to a half-dozen or so.

For (1), I think the issue will be mostly on Windows, since the UNIX shell is pretty consistent in how it handles spaces and quotes. (The rules Bash and Korn uses are arcane, but consistent. :))

@JamzTheMan
Copy link
Member

In regards to preventing macro overrun of the filesystem, why not just use a quota?

Before every write operation, just do a FileUtils.sizeOfDirectory() and return error if over ? Of course, give a macro to erase the file(s) as well... Could default the limit to 10 mb with a preference setting the user can change.

@Azhrei
Copy link
Member

Azhrei commented Nov 18, 2016

Well, sizeOfDirectory() is a bad choice as it recursively scans the given directory and all subdirectories. That's a bit of overkill. More likely would be single-file constraints since only a single call would be needed.

I don't think we want an error returned, though. The macro language doesn't have good error handling and it could be quite disruptive to error out because a log file filled up...

@JamzTheMan
Copy link
Member

Well my thought was that we would restrict where the files are saved, like, .maptool/extract or something. The subdir would only be for files MT creates via macro so size of directory should be ok.

That and, the one request is for "logging" but other output files would be useful. For instance, I output XP logs for my players every time the XP macro is ran. It's a csv file and written to my google drive and picked up in a Google Sheet for reference. So it's a "log" but I have other files and wouldn't want or need to parse them individually and they are not "rolling"...

@Azhrei
Copy link
Member

Azhrei commented Nov 18, 2016

I guess I'm a power user. :)

I create symbolic links so directories are where I want them, and I tuck directories and files under application directories to keep things organized (such as putting copies of config files under logs/.copies). I suppose most people don't do that, but if we make such a check on every write, it'll get slow...

@JamzTheMan
Copy link
Member

JamzTheMan commented Nov 18, 2016

Well, I would do the same (symbolic link from .maptool to google drive location but right now it's my own code/build so I just write to where ever i want :) (I've had this logic in my build for 2 years now lol)

The fileutils check is pretty fast, it's not like we're logging web server traffic. IF you are logging that much, then either you'll need to room to store the data (so no or high quota limits) or you'll roll off anything you may need...

Maybe it's two macro:
util.logging("all_my_dice_rolls_evar.log") which does a nice auto timestamp (in log and file) and handles rolling, 10 logs, 10ish mb limit, No checks, express checkout line, writes to .maptool/user/logging
util.write("campaign_state.csv") which is a "write anything you want file", writes to .maptool/user/files with a directory limit of x mb?

PS oh and a util.read because, yea, why not. only allow reads from files in /user/files

@Azhrei
Copy link
Member

Azhrei commented Nov 19, 2016

So who is doing the logging? Is it every client? Does that mean every client gets log files even if they don't want them? Is the configuration function (your util.logging() call, above) a trusted function? Does it allow those settings to be made (the sizes and such that you mention), or are they obtained from the Preferences, or both?

Overall, I think the idea of logging is good as it has plenty of uses, not the least of which is help in debugging MTscript. But since MTscript is going away, I'm not sure how useful this becomes when MTscript gets replaced. Will there be similar logging available in JS? Most likely, the JS code will use console.log() (or similar) and MT will create an appropriate console object so that logging goes wherever MT's logs are going.

Or maybe MT should have both a console object and a general MT log object, maybe something like mtlog? That way console log messages could actually go to a log window while macro logging for the benefit of the GM/programmer could go someplace else. Or maybe it would be better to add an extension to console so that it supports both console.log() and console.mtlog()? (Now I'm just spitballing, because I haven't thought this through at all.)

@Syndaryl
Copy link

We use the log files extensively as a record of what happened in the game session. It's our archive. For that, the background of the macros and the code behind them isn't important, but the HTML as rendered in the chat window is vital.

@Azhrei
Copy link
Member

Azhrei commented Nov 21, 2016

Yes, we all agree on that. What Jamz is referring to is a separate form of logging that script writers can add to their macro code for "behind the scenes"-type logging.

@JamzTheMan
Copy link
Member

@rkathey See #433 if it satisfies this issue? New functions added to log any text at standard log levels to the log file/console. Log files are also now rolling 14 day logs so plenty of time to capture/save archived logs as needed after games.

Console is no longer used so item 1 is not an issue now as well.

@rkathey
Copy link
Contributor Author

rkathey commented Apr 14, 2019

@JamzTheMan I'll try it after the RC build for 1.5.2

@Phergus
Copy link
Contributor

Phergus commented Apr 29, 2019

So I know that #2 is complete. As no example of #1 was given, I can't tell if it was fixed or not.

What's the status on #3?

@JamzTheMan
Copy link
Member

You can technically log to the maptool log via log functions so? Done for now? :)

@JamzTheMan
Copy link
Member

Oh and #1 was a launcher only issue...

I say this is done and #3 can be later expanded in 2.0 with more details and design.

@Phergus Phergus removed this from the 1.4.3.x Dev Build milestone Apr 30, 2019
@Phergus
Copy link
Contributor

Phergus commented Apr 30, 2019

Great. Considered Done and part of 1.5.2.

@Phergus Phergus closed this as completed Apr 30, 2019
@Phergus Phergus added the tested This issue has been QA tested by someone other than the developer. label Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug feature Adding functionality that adds value tested This issue has been QA tested by someone other than the developer.
Projects
None yet
Development

No branches or pull requests

5 participants