overte icon indicating copy to clipboard operation
overte copied to clipboard

Refactor Application.cpp

Open HifiExperiments opened this issue 1 year ago • 11 comments

Closes #931

(apologies to whoever reviews this)

Application.cpp has gotten totally out of control over the years (in many cases because of my dumb decisions). This is an attempt to corral it. I've organized the code as best I could into sections - Assets, Camera, Entities, Events, Graphics, Plugins, Setup, and UI - and divided the implementations into separate, smaller files. I completely rebuilt the includes from scratch so we only have what's necessary. I broke some of the classes into their own files. Overall, I changed very little functionally. I removed a few unused variables and functions but nearly everything is directly copy-pasted.

Testing-wise, things should just work as before. In particular, anything related to startup, login UI, and plugins.

Funding

This project is funded through NGI0 Entrust, a fund established by NLnet with financial support from the European Commission's Next Generation Internet program. Learn more at the NLnet project page.

NLnet foundation logo NGI Zero Logo

HifiExperiments avatar Apr 16 '24 21:04 HifiExperiments

hm

/__w/overte/overte/interface/src/scripting/PerformanceScriptingInterface.cpp:14:10: fatal error: QtQML: No such file or directory
   14 | #include <QtQML>
      |          ^~~~~~~

I'll look into that...

HifiExperiments avatar Apr 17 '24 19:04 HifiExperiments

I'm also having trouble building it on Linux using Overte-builder: image

ksuprynowicz avatar Jun 08 '24 07:06 ksuprynowicz

I tested it on Windows, and it asks to log in every time. A different than usual login screen is used for this: overte-snap-by--on-2024-06-15_15-10-47 Kraftwerk world becomes entirely white after skybox loads:

overte-snap-by--on-2024-06-15_15-02-21 No audio devices are detected: obraz

ksuprynowicz avatar Jun 15 '24 15:06 ksuprynowicz

It also seems that login screen that it uses in VR is intended for desktop?

ksuprynowicz avatar Jun 15 '24 15:06 ksuprynowicz

The good thing about that different login screen though is that it has option to continue anonymously. Ours is badly missing this.

ksuprynowicz avatar Jun 15 '24 15:06 ksuprynowicz

I believe I've fixed the audio device issue. for the login screen, it turns out I had accidentally deleted _disableLoginScreen which was always set to true. so whatever that fullscreen login page was for, it appears to have been disabled intentionally at some point. I've re-disabled it to just avoid messing with stuff, but maybe we should look into re-enabling it properly at some point. I'm not sure if this also fixed the VR login screen, if that's still a problem could you send me a screenshot?

I wasn't able to reproduce the issue in kraftwerk; I wonder if the download got corrupted or something? does it still happen if you clear your caches, including ktx cache?

HifiExperiments avatar Jun 20 '24 21:06 HifiExperiments

I gave this a test on Linux and so far, everything works fine. I tried clearing shader cache and ktx cache as well. In both scenarios, Kraftwerk renders properly.

JulianGro avatar Sep 02 '24 15:09 JulianGro

I tested it again and now everything works well :)

ksuprynowicz avatar Sep 12 '24 20:09 ksuprynowicz

Would someone like to test in VR on Windows? I don't have a VR setup at the moment. If all works well we could merge it.

ksuprynowicz avatar Sep 12 '24 20:09 ksuprynowicz

there are some conflicts now, I’ll probably wait until after the next release to rebase and resolve them just to avoid causing any problems

HifiExperiments avatar Sep 12 '24 20:09 HifiExperiments

Would someone like to test in VR on Windows? I don't have a VR setup at the moment. If all works well we could merge it.

I did some brief testing in VR and everything seems to work as expected.

there are some conflicts now, I’ll probably wait until after the next release to rebase and resolve them just to avoid causing any problems

I will also take a look at this again after the rebase!

Armored-Dragon avatar Sep 13 '24 04:09 Armored-Dragon

I've rebased this and it's all ready to be reviewed + tested again!

HifiExperiments avatar Oct 22 '24 04:10 HifiExperiments

I've rebased this again and it's still ready for final testing + review!

HifiExperiments avatar Jan 05 '25 06:01 HifiExperiments

I just tested it on Linux and on every start no microphone is selected. Default microphone needs to be selected every start. Output seems to be selected properly.

ksuprynowicz avatar Jan 05 '25 17:01 ksuprynowicz

It seems that microphone issue happens only on Linux. It happens on every start and no input is selected: image

ksuprynowicz avatar Jan 05 '25 18:01 ksuprynowicz

It looks like the microphone issue doesn't happen in Desktop mode on Windows, but does happen in VR mode on Windows, exactly like in Desktop mode on Linux.

ksuprynowicz avatar Jan 05 '25 18:01 ksuprynowicz

Teleport script in the tower near Overte_hub spawn point is broken, it fails with message:

[01/05 19:47:37] [CRITICAL] [overte.scriptengine] [about:Entities 1] Function call failed: "failed on line 26 column 8 with message: "Uncaught ReferenceError: Window is not defined" backtrace: ReferenceError: Window is not defined
[01/05 19:47:37] [CRITICAL] [overte.scriptengine]     at Object.enterEntity (https://aleziakurdis.github.io/inertia/teleporters/teleporter.js:26:9)
[01/05 19:47:37] [CRITICAL] [overte.scriptengine] [about:Entities 1] JS function call failed:

ksuprynowicz avatar Jan 05 '25 18:01 ksuprynowicz

After some more testing input selection issue seems to happen sometimes in Desktop mode on Windows too.

ksuprynowicz avatar Jan 06 '25 09:01 ksuprynowicz

darn! thank you for testing, I'll look into those

HifiExperiments avatar Jan 07 '25 05:01 HifiExperiments

there’s definitely some weirdness around audio devices on startup. the previous fix (to the much earlier issue of the audio devices not showing up) was to reorder some initialization: https://github.com/overte-org/overte/pull/939/commits/2345bcc2b58d2cb8560cbabcca65f191ee0e81de#diff-379416a9df19174e2bf0d9d644da8269931fbe2f7f0516719eedb75f8d022b34R810

so…perhaps there’s still something wrong about the order of that versus the loading from settings

HifiExperiments avatar Feb 19 '25 05:02 HifiExperiments