Use render method from OS instead of project settings in compositor RD#85387
Conversation
|
LGTM. There are other points in the codebase doing the same, though. I think all of them warrant revisiting. |
yes and I think I can do it in this pr to prevent further crashes.
That sounds like a usability improvement instead of fixing crash, and maybe can use a seperat pr. |
There was a problem hiding this comment.
Tested locally, it works as expected. This also fixes #85143 🙂
Forward+ project started with --rendering-method mobile:
| Before (is actually using Forward+) | After (correct appearance for Mobile) |
|---|---|
 |
 |
It also works the other way around (--rendering-method forward_plus on a Mobile project). --rendering-method gl_compatibility on a Forward+/Mobile project and --rendering-method <forward_plus|mobile> on a Comptaibility project also work.
PS: Should OS.get_current_rendering_method() also be used in the rendering dropdown in the top-right corner of the editor? Right now, the dropdown's current option will be incorrect if you use --rendering-method when opening the editor.
|
@Calinou, see my answer above. I think It'd be good to lock it if overridden to avoid confusion (if restarting the editor, is that one of the args that is carried over to new instances), or at least some other kind of special treatment. |
830f294 to
de0ad04
Compare
|
@RandomShaper I push a new commit to do what you said, if the rendering method is overriden then it will only show current one with a suffix |
RandomShaper
left a comment
There was a problem hiding this comment.
Good job. I've only added a comment about a refactor that would make the code more compact.
de0ad04 to
8c951bb
Compare
|
I have confirmed it works (and I'm using it in my game to allow for launching in compatibility through steam launch options with arguments) |
clayjohn
left a comment
There was a problem hiding this comment.
Everything looks good to me.
I personally don't like introducing more lambdas into the codebase, but this one comes with Pedro's seal of approval :)
8c951bb to
453c224
Compare
|
Updated the code to use member function instead of lambda, squashed the commit. |
YuriSizov
left a comment
There was a problem hiding this comment.
Editor parts look fine to me, but we'll probably clean them up in the future.
|
Thanks! |
|
Cherry-picked for 4.2.2. |
Fixes #85376
Not sure why Godot doesn't use the rendering method stored in OS in the first place, it's the correct one after all ( as it is computed in main.cpp ) . Feel free to correct me.
--rendering-methodCLI arguments are not effective on the actual scene rendering, only on warning messages #85143.