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

web: Various improvements to options #10900

Merged
merged 3 commits into from
May 3, 2023

Conversation

n0samu
Copy link
Member

@n0samu n0samu commented May 1, 2023

Intended to supersede #10831, this PR does the following:

  • Adds a collapsible "Advanced options" section to the extension settings page and moves less commonly used options there. (Thanks to @relrelb)
  • Adds an "Automatic" option for preferred renderer in the extension settings page (Thanks to @iwannabethedev)
  • Adds missing extension messages and improves wording for some existing ones. (Thanks to everyone who suggested these)
  • Adds an enableFallbackRenderers option that can be used to disable all fallback renderers. This is an attempt to address web: Add "preferred renderer" config option #9904 (comment) in a way that I find more palatable than web: Add forceRenderer config #10835. Even so, I would rather not add this option because I think its usefulness to developers is outweighed by the opportunities it creates for users and webmasters to cause problems for themselves. If you all agree, I would be very happy to remove this part of this PR! Removed!

@n0samu n0samu force-pushed the options-improvements branch 2 times, most recently from ab56c64 to 1c543d8 Compare May 1, 2023 08:24
@iwannabethedev
Copy link
Contributor

Even so, I would rather not add this option because I think its usefulness to developers is outweighed by the opportunities it creates for users and webmasters to cause problems for themselves. If you all agree, I would be very happy to remove this part of this PR!

Is there an easy and relatively convenient way for Ruffle-developers (and maybe also players, game-developers and webmasters, though these groups of users seem less important to me in regards to this part of the topic) to view or determine which render-backend ended up getting chosen? Because if so, I imagine that Ruffle-developers could depend on that to see which render-backend is being used, and I believe that would lessen a need for supporting a forced option in any case.

@n0samu
Copy link
Member Author

n0samu commented May 1, 2023

Yes, you can right-click => Copy Debug Info and paste the text somewhere to see. There will be a line that says (for example):
Renderer: wgpu
Personally I find that convenient enough.

Edit: Oh and another way is to set the log level to Info and look at the console messages. Here's an example of Ruffle falling back to WebGL:

INFO web/src/lib.rs:1325 Creating wgpu webgl renderer...
ERROR web/src/lib.rs:1336 Error creating wgpu webgl renderer: Creating a surface failed
INFO web/src/lib.rs:1345 Creating WebGL renderer...

@iwannabethedev
Copy link
Contributor

iwannabethedev commented May 1, 2023

I do not at all have a good feeling for whether that is convenient or not, I have never personally encountered it. I wonder if it might make sense to ask Dinnerbone if he thinks it would be convenient enough, since he described having multiple times encountered this issue: "I know I've been caught a few times by using one backend instead of another when the preference wasn't doing what I expected during dev. " .

For the log level method, I am not quite certain, since you would have to enable it before running or reload the page. Though, you could probably just set the log level to "Info", open a new tab with the application, see the first few lines which render-backend was chosen, and then revert.

For the context menu and copy-pasting debug information, is the right-click Flash context menu always available?

But, this is two different methods that one can use. And they seem decently straight-forward enough so that even users could also use them. They are not all that convenient, but not all that inconvenient either.

I am wondering if one could make the "forcedRenderer" or "enableFallbackRenderers" more user-friendly by, when the selected render-backend is not available, to give a sort-of pop-up (maybe just https://developer.mozilla.org/en-US/docs/Web/API/Window/alert ) that informs the user that the selected render-backend could not be chosen, and then still fall back. Such that it becomes "warn" instead of "force".

@iwannabethedev
Copy link
Contributor

Sorry, I accidentally posted the comment early, please see the edit.

@iwannabethedev
Copy link
Contributor

But this might be extra work and effort that is not worth what advantages it might give.

@iwannabethedev
Copy link
Contributor

I personally think there are reasonable arguments for and against keeping something like "forceRenderer"/"enableFallbackRenderers".

@iwannabethedev
Copy link
Contributor

iwannabethedev commented May 1, 2023

After some discussion and investigation elsewhere, one possible course of action might be to forgo the "forceRenderer" and "enableFallbackRenderers", and instead do something similar to Phaser 3, where which render-backend that has been chosen is always printed in the browser console:

Phaser 3 render-backend console printing

In the above image, you can see the "WebGL" string (as part of the nicely-colored and -formatted line), which indicates that "WebGL" (out of the two "WebGL" and "Canvas" render-backend options for Phaser 3) is used.

If that is always printed in the browser console, it would be even more convenient to figure out which render-backend was chosen instead of having to set the logging level to "Info".

I believe this could be a good substitute for "forceRenderer" and "enableFallbackRenderers", in particular since it would avoid any panic'ing as @n0samu describes, and it should make it relatively more convenient than setting the logging level to "info", since you would have access to this information no matter the logging level. And there is additionally also the possibility of getting debug info through the "Copy debug info" in the right-click context menu.

I will note that Phaser 3 additionally also has "forced" selection as well as "auto"/fallback selection, as the image in #10835 (comment) describes, but the situation for Phaser 3 is rather different from Ruffle in this regard, for in Ruffle, this forced selection setting is relevant only for game developers and while the game is being developed and maintained, which is very different from the situation and circumstances for Ruffle and applications run with Ruffle (Phaser 3 is also a game framework, while Ruffle is an emulator). Phaser 3 apparently also has no error handling if a forced selection is not supported, requiring the game developer to implement error handling. And Phaser 3 has WebGL-specific graphics features, while Flash and Ruffle's renderer-backends are meant to support all graphics features (I might be very wrong on this point, maybe it is more accurate to write that there are no renderer-backend-specific functionality?).

@iwannabethedev
Copy link
Contributor

I will open a PR that logs the currently used renderer-backend, since I think it would be a nice improvement in any case.

@Dinnerbone
Copy link
Contributor

@n0samu given the "enableFallbackRenderers" is debated topic right now, in idea and execution, can we remove it from this PR and pull it out into its own?

I'd like to get the rest of the fixes in, especially since you've touched translations.

@n0samu
Copy link
Member Author

n0samu commented May 1, 2023

Yep sounds good 👍

@n0samu
Copy link
Member Author

n0samu commented May 2, 2023

Alright I've rebased this and removed the enableFallbackRenderers change

@iwannabethedev
Copy link
Contributor

iwannabethedev commented May 3, 2023

I like the changes, I think it looks good, and I would believe that it is ready to be merged once the latest review comment has been looked at.

Edit: The latest review comment has been looked it and the changes look good to me 👍 .

relrelb and others added 3 commits May 3, 2023 09:31
To avoid visual clutter, move some less handy options under a
collapsible area:

* Log level
* Ignore website compatibility warnings
* Maximum allowed ActionScript execution duration (description was
shortened in this commit)
* Preferred renderer
* Player version number
@relrelb relrelb force-pushed the options-improvements branch from 871c27d to f33a579 Compare May 3, 2023 06:31
@relrelb relrelb enabled auto-merge (rebase) May 3, 2023 06:31
@relrelb relrelb merged commit 9b2dc26 into ruffle-rs:master May 3, 2023
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.

4 participants