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

Core: smarter fill logging #3575

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

kedNalatacId
Copy link
Contributor

@kedNalatacId kedNalatacId commented Jun 21, 2024

Please format your title with what portion of the project this pull request is
targeting and what it's changing.

ex. "MyGame4: implement new game" or "Docs: add new guide for customizing MyGame3"

What is this fixing or adding?

Replace current Fill logging with a more dynamic logging mechanism. Scales appropriately between small, medium, and large seeds.

It doesn't log anything smaller than "min_size" (currently set to 1000 to match pre-existing logic).
It doesn't log any faster than "min_time" (currently 1/4 second).

Because of the introduction of carriage return, safeguards are put in to make sure any output heading to a non-tty location don't include said carriage return (it does nasty things to log files).

How was this tested?

Generating many, many seeds and watching them.
Genned:

  • On CLI (mac)
  • On CLI (Windows)
  • via Launcher (Windows)
  • via WebHost

All tests were done with python 3.11.

If this makes graphical changes, please attach screenshots.

Note status when done:
Screenshot 2024-06-20 at 17 44 20

Action shot:
smarter_fill_logging-20240620-short

[edit to add: log files are also changed; this can be rolled back easily if requested]
In the log file:
Screenshot 2024-07-04 at 15 55 49

@github-actions github-actions bot added affects: core Issues/PRs that touch core and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Jun 21, 2024
@Exempt-Medic Exempt-Medic added the is: enhancement Issues requesting new features or pull requests implementing new features. label Jun 21, 2024
@beauxq
Copy link
Collaborator

beauxq commented Jul 2, 2024

What does the log file look like with this? Does it add a line for every placed item? or every quarter second?

Fill.py Outdated Show resolved Hide resolved
@kedNalatacId
Copy link
Contributor Author

What does the log file look like with this? Does it add a line for every placed item? or every quarter second?

If you're talking about the screenshot, it's updating at most every quarter second (the "min_time" field specifies minimum time elapsed between logging updates). If quarter second is seen as too fast for logging updates it's pretty easy to slow it down by bumping the min_time field. The "min_time" field is only applicable to tty-based logging. The other field (min_size) is for non-tty based logging (to files or pipes), and specifies the smallest size increment there as 1000 items.

If you're talking about logs in the logs directory there's currently a bug such that those log files are receiving carriage returns and obviously shouldn't be (receiving tty logs when they should only get non-tty logs). I only discovered this issue recently and have an update I'll be pushing soon to address it (it can take a while to test logging for all sizes of seeds).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants