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

extension: Log used renderer. #10909

Merged
merged 1 commit into from
May 2, 2023

Conversation

iwannabethedev
Copy link
Contributor

Log which renderer-backend was actually used in the browser console.

This is basically the same as #10903 , but without the toggle option for whether to warn or not if the actually used renderer is not the same as the preferred renderer.

Related to: #10835 , #10831 , #10900 .

CC: @n0samu @relrelb .

@iwannabethedev iwannabethedev force-pushed the extension_log_renderer branch from 07786a2 to b339da4 Compare May 1, 2023 23:26
@iwannabethedev iwannabethedev marked this pull request as ready for review May 1, 2023 23:30
@relrelb
Copy link
Contributor

relrelb commented May 2, 2023

In fact this can be implemented completely on Rust side, and with greater simplicity, using just tracing::info!() to log the chosen backend (and I think this already the case for some backends). Am I missing a reason for it to pass through TypeScript?

@n0samu
Copy link
Member

n0samu commented May 2, 2023

I think we wanted to make the message appear regardless of the logLevel, like the "new Ruffle instance created" message. (My idea was to add the renderer name to that message specifically.)

@iwannabethedev
Copy link
Contributor Author

Yes, the concept was to make it a bit more convenient to view which renderer-backend was used, such that you would not have to switch the logging level to "Info" first. I spent some time seeing if I could do it just from Rust, but I did not figure out how to do printing to the browser console apart from using tracing::info or tracing::error, and using tracing::error would probably be confusing since the printing is not an error.

The printing is in many ways inspired by the image in #10900 (comment) , which is taken from a Phaser 3 application.

Additionally, another reason is that it would allow using an HTML-popup like with window.alert though that is only relevant if the changes in #10903 are considered.

web/src/lib.rs Outdated Show resolved Hide resolved
@iwannabethedev iwannabethedev force-pushed the extension_log_renderer branch from b339da4 to 7e11e45 Compare May 2, 2023 17:02
web/src/lib.rs Outdated Show resolved Hide resolved
@iwannabethedev iwannabethedev force-pushed the extension_log_renderer branch from 7e11e45 to a835d39 Compare May 2, 2023 17:28
@iwannabethedev iwannabethedev force-pushed the extension_log_renderer branch 3 times, most recently from 34e6efb to 1e1b750 Compare May 2, 2023 17:54
Log which renderer-backend was actually used in the
browser console.
@iwannabethedev iwannabethedev force-pushed the extension_log_renderer branch from 1e1b750 to e54ffb9 Compare May 2, 2023 18:10
Copy link
Contributor

@relrelb relrelb left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@relrelb relrelb merged commit 307f364 into ruffle-rs:master May 2, 2023
@iwannabethedev iwannabethedev deleted the extension_log_renderer branch May 6, 2023 12:32
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.

3 participants