web: Automatically chosen preferred renderer

[iwannabethedev]

web: Add 'Automatic' option to 'preferredRenderer'.

The 'Automatic' option is with this PR the default. The option lets Ruffle select the renderer.

This option is more or less the same as similar options in related technologies have, with one example being Phaser 3:

https://newdocs.phaser.io/docs/3.55.2/Phaser.Types.Core.GameConfig

This PR came about through discussion and investigation between @n0samu and me and others.

The changes have been lightly tested manually.

[iwannabethedev]

This PR uses a commit from #10829 , so this PR will probably have to be rebased once that PR is merged or closed without merging.

[iwannabethedev]

n0samu and I have added an extra check.

[n0samu]

This is already explained by the existing comments, so not really needed IMO

[n0samu]

Let's hold off on merging this until we can decide if this is really the best approach

[relrelb]

As far as I understand, preferredRenderer: null and preferredRenderer: RenderBackend.Automatic are totally equivalent. Also, with forceRenderer in mind, I think forceRenderer: RenderBackend.Automatic doesn't make sense (and neither { preferredRenderer: RenderBackend.Automatic, forceRenderer: true }), so I'm not sure adding RenderBackend.Automatic is the way to go.

Ideally, we could remain with just preferredRenderer: null, but I'm not sure how it's best to implement in options.ts...

[iwannabethedev]

As far as I understand, preferredRenderer: null and preferredRenderer: RenderBackend.Automatic are totally equivalent.

Yes, 100% correct.

Also, with forceRenderer in mind, I think forceRenderer: RenderBackend.Automatic doesn't make sense (and neither { preferredRenderer: RenderBackend.Automatic, forceRenderer: true }), so I'm not sure adding RenderBackend.Automatic is the way to go.

In hindsight, this option might have been better of me to have implemented after the other parts of the options had been figured out. But it is not much work, and the work here can be used to help figure out what future work on this should be, so it is alright.

Ideally, we could remain with just preferredRenderer: null, but I'm not sure how it's best to implement in options.ts...

I actually implemented a version that supported null and force-pushed it (disappearing some non-null-using approach), but then I reversed course and decided against supporting null, and force-pushing these changes made the null-using changes disappear. I should probably have saved the changes in a side-branch 😅 . I think I did it by changing class SelectOption implements OptionElement<string> to class SelectOption implements OptionElement<string | null> and change the implementation accordingly.

The reason why I decided against it and reversed course was that all the other export const enum types in 'load-options.ts' do not allow null, and I tend to like "narrowing" types when possible. I also wrote some stuff in the Discord, I think in the #rendering channel (and maybe also some in #meta-discussion before we moved the discussion to #rendering ).

But it can be done with null, since I did test it and made it work and force-pushed it, from what I remember.

[iwannabethedev]

Wait, I think it was in #frontends-apps , not #rendering .

[relrelb]

I actually implemented a version that supported null and force-pushed it (disappearing some non-null-using approach), but then I reversed course and decided against supporting null, and force-pushing these changes made the null-using changes disappear. I should probably have saved the changes in a side-branch 😅 . I think I did it by changing class SelectOption implements OptionElement<string> to class SelectOption implements OptionElement<string | null> and change the implementation accordingly.

I changed SelectOption to imeplement OptionElement<string | null> in #10835, so it can be usable here once merged.

[n0samu]

Once #10835 is merged, we can change this PR to just be this single-line change, right?

If so, let's plan to do that. Thanks!

[iwannabethedev]

@n0samu I have read through both of the PRs again, and I agree, I believe that would work correctly, and I also believe it would be a very nice way of doing it. I will wait until the other PR is merged, and then update and test and rebase and then force-push a commit for this PR making it a one-line change in a single commit.

[relrelb]

@iwannabethedev Given that merging #10835 (if at all) will probably take some time, maybe the common.ts changes should be included in this PR, in order to unblock it. Then only the <option value="">Automatic</option> addition will be needed like you described.

[iwannabethedev]

@iwannabethedev Given that merging #10835 (if at all) will probably take some time, maybe the common.ts changes should be included in this PR, in order to unblock it. Then only the <option value="">Automatic</option> addition will be needed like you described.

I believe that the PR in #10900 already includes those changes to common.ts . I believe that it might make sense to get #10900 merged (maybe after extracting and removing the changes with enableFallbackRenderers like Dinnerbone proposed), since it also additionally has translations.

[iwannabethedev]

Superseded by #10900 .