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

fix: allow missing COs on QML component #13011

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

acolombier
Copy link
Member

@acolombier acolombier commented Mar 27, 2024

This PR allows Mixxx.ControlProxy to reference CO that don't exist without crashing. This is to ensure backward compatibility the controller JS engine which allows to connect to in-existing COs and let the caller handle invalid (see src/controllers/scripting/legacy/controllerscriptinterfacelegacy.cpp/ControllerScriptInterfaceLegacy::makeConnectionInternal.

This is a dependency for #11407

@Holzhaus
Copy link
Member

Have you considered to you make this depend on a boolean Q_PROPERTY that is off by default? So that we need to be explicit about it that you 2ant that behavior?

Or, if that is too strict, maybe we could check if the --developer flag was passed on the cmdline and only use AllowInvalidOrMissing if it's not passed?

@Holzhaus
Copy link
Member

Holzhaus commented Mar 27, 2024

To elaborate, I'd just like to preserve the debug assertion when doing QML skin development if possible.🤔

@acolombier
Copy link
Member Author

Or, if that is too strict, maybe we could check if the --developer flag was passed on the cmdline and only use AllowInvalidOrMissing if it's not passed?

Yes, I was thinking the same if we wanted to keep the asserting. Will update that!

@acolombier acolombier force-pushed the fix/allow-mission-co branch from 83ee3ba to 86067f9 Compare March 27, 2024 23:31
@acolombier
Copy link
Member Author

@Holzhaus are you happy with the look of this fix now?

@Holzhaus Holzhaus merged commit 134bd6c into mixxxdj:main Apr 3, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants