Skip to content

Conversation

@ahigerd
Copy link
Contributor

@ahigerd ahigerd commented Apr 30, 2025

Max wanted to see the scripting console output on stdout, but shimming the console object didn't work for capturing runtime errors. This will work for his use case, but maybe it's worth considering if we want to support this use case directly.

PR presented for discussion.

Comment on lines +45 to +46
out << message << "\n";
out.flush();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LogController.cpp uses a QT_VERSION check to make sure that endl is defined, but this equivalent implementation works in all Qt versions. Maybe we could apply the same change there to remove a conditional compilation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the very least, IMO the #define endl Qt::endl should be replaced with using Qt::endl;.

@ahigerd
Copy link
Contributor Author

ahigerd commented May 1, 2025

Oops, this PR depends on #3485.

@endrift
Copy link
Member

endrift commented May 3, 2025

This opens up a lot of architectural issues like "should logging from scripts go through the normal log controller flow", which this PR is kind of just sweeping under the rug. I don't think this is the right place to implement this and it needs more discussion.

@ahigerd
Copy link
Contributor Author

ahigerd commented May 3, 2025

I don't think this is the right place to implement this and it needs more discussion.

Discussion is the reason I opened the PR. I didn't actually expect it to be merged as-is.

@endrift endrift marked this pull request as draft May 4, 2025 01:03
@endrift
Copy link
Member

endrift commented May 4, 2025

So then let's ask the question: Should logging from the script go through the regular logging flow? If so, we'd need to re-plumb the console a bit, but it's probably a better approach, especially if we want to add options about the destination

@ahigerd
Copy link
Contributor Author

ahigerd commented May 4, 2025

It does sound like it might be a good idea, especially with autorun scripts in the picture now. I like the idea of being able to see script errors in with emulation errors in the general logs, and it means that the logs would get captured even if the scripting console isn't open.

I think we might need to have some kind of log forwarding in place, so that messages always go to the scripting console but honor the logging configuration for what goes into the general logs.

Some side questions:

  • Should we have separate log categories for script engine logs versus user script logs? Or is it enough to put everything into the SCRIPT category?
  • What should the default log levels be?
  • Is it meaningful to consider having log filtering in the scripting console? That would be enabled by such a re-plumbing. On the other hand, it introduces more questions about presentation and history tracking.

Also, pulling a comment from #3483 (comment) over into this discussion:

mLog itself isn't reentrant. Do we want to fix this now or later?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants