Can't Save Shortcut Data #2934

Closed
opened 2026-02-20 22:15:12 -05:00 by deekerman · 23 comments
Owner

Originally created by @MaxNarr on GitHub (Dec 11, 2024).

Description

The data field of a shortcut (for example Whisper/shout -> Subchannel #1) will not be saved after restarting. After restarting the data field says "empty" instead of Subchannel #1. The rest of the shortcut is stored correctly.

The command line prompts this when pressing apply :QVariant::save: unable to save type 'ShortcutTarget' (type id: 65538).
The command line prompts this on startup:
./mumble <X>2024-12-11 22:31:02.804 Loading settings from "/home/coms/.config/Mumble/Mumble/mumble_settings.json" <W>2024-12-11 22:31:02.804 QVariant::load: unable to load type 65538. <W>2024-12-11 22:31:02.804 QVariant::load: unable to load type 65538.

Steps to reproduce

  1. Open Settings Menu
  2. Go to Shortcuts
  3. Press add button
  4. choose whisper/ shout in the "Function" section
  5. in the "data" section choose a channel. For example "Subchannel #1"
  6. restart mumble

Mumble version

1.6.0

Mumble component

Client

OS

Linux

Reproducible?

Yes

Additional information

Mumble was compiled using the latest available code from git on a raspbian on RPI 5.
Using this manual https://wikiarchiv.natenom.de/mumble/entwicklung/mumble-selbst-bauen.html

Relevant log output

No response

Screenshots

No response

Originally created by @MaxNarr on GitHub (Dec 11, 2024). ### Description The data field of a shortcut (for example Whisper/shout -> Subchannel #1) will not be saved after restarting. After restarting the data field says "empty" instead of Subchannel #1. The rest of the shortcut is stored correctly. The command line prompts this when pressing apply :`QVariant::save: unable to save type 'ShortcutTarget' (type id: 65538).` The command line prompts this on startup: `./mumble <X>2024-12-11 22:31:02.804 Loading settings from "/home/coms/.config/Mumble/Mumble/mumble_settings.json" <W>2024-12-11 22:31:02.804 QVariant::load: unable to load type 65538. <W>2024-12-11 22:31:02.804 QVariant::load: unable to load type 65538.` ### Steps to reproduce 1. Open Settings Menu 2. Go to Shortcuts 3. Press add button 4. choose whisper/ shout in the "Function" section 5. in the "data" section choose a channel. For example "Subchannel #1" 6. restart mumble ### Mumble version 1.6.0 ### Mumble component Client ### OS Linux ### Reproducible? Yes ### Additional information Mumble was compiled using the latest available code from git on a raspbian on RPI 5. Using this manual https://wikiarchiv.natenom.de/mumble/entwicklung/mumble-selbst-bauen.html ### Relevant log output _No response_ ### Screenshots _No response_
Author
Owner

@Krzmbrzl commented on GitHub (Dec 11, 2024):

@davidebeatrici I strongly suspect this is due to some difference introduced with Qt 6 👀

@Krzmbrzl commented on GitHub (Dec 11, 2024): @davidebeatrici I strongly suspect this is due to some difference introduced with Qt 6 👀
Author
Owner

@davidebeatrici commented on GitHub (Dec 11, 2024):

The error is usually caused by the data type not being registered, but we're doing that:

github.com/mumble-voip/mumble@edd4588c8a/src/mumble/Settings.h (L71-L87)

github.com/mumble-voip/mumble@edd4588c8a/src/mumble/Settings.cpp (L513-L521)

One difference I'm aware of is the deprecation of qRegisterMetaTypeStreamOperators, in fact in #6516 all instances were surrounded by a Qt version check:

#if QT_VERSION < 0x060000
	qRegisterMetaTypeStreamOperators< ChannelTarget >("ChannelTarget");
	qRegisterMetaTypeStreamOperators< PluginSetting >("PluginSetting");
	qRegisterMetaTypeStreamOperators< ShortcutTarget >("ShortcutTarget");
#endif

From https://www.qt.io/blog/whats-new-in-qmetatype-qvariant:

With Qt 6, the first thing we did was to extend the information stored in QMetaType: Modern C++ is now almost 10 years old, so it was about time to store information about the move constructor in QMetaType. And to provide better support for overaligned types, we now also store the alignment requirements of your types. Moreover, we considered the registries to be a bit clunky. After all, why should we require you to call QMetaType::registerEqualsComparator() when we could already know this by simply looking at the type? So in Qt 6, QMetaType::registerEqualsComparator, QMetaType::registerComparators, qRegisterMetaTypeStreamOperators and QMetaType::registerDebugStreamOperator have been removed. The metatype system will instead know about those automatically.

I suspect we may have to implement the stream operators in the class ourselves.

And, if that's actually the case, I would expect the same issue with ChannelTarget and PluginSetting as well.

@davidebeatrici commented on GitHub (Dec 11, 2024): The error is usually caused by the data type not being registered, but we're doing that: https://github.com/mumble-voip/mumble/blob/edd4588c8ae03d785d59102e2435151a682ec51d/src/mumble/Settings.h#L71-L87 https://github.com/mumble-voip/mumble/blob/edd4588c8ae03d785d59102e2435151a682ec51d/src/mumble/Settings.cpp#L513-L521 One difference I'm aware of is the deprecation of `qRegisterMetaTypeStreamOperators`, in fact in #6516 all instances were surrounded by a Qt version check: ```c++ #if QT_VERSION < 0x060000 qRegisterMetaTypeStreamOperators< ChannelTarget >("ChannelTarget"); qRegisterMetaTypeStreamOperators< PluginSetting >("PluginSetting"); qRegisterMetaTypeStreamOperators< ShortcutTarget >("ShortcutTarget"); #endif ``` From https://www.qt.io/blog/whats-new-in-qmetatype-qvariant: > With Qt 6, the first thing we did was to extend the information stored in QMetaType: Modern C++ is now almost 10 years old, so it was about time to store information about the move constructor in QMetaType. And to provide better support for overaligned types, we now also store the alignment requirements of your types. Moreover, we considered the registries to be a bit clunky. After all, why should we require you to call `QMetaType::registerEqualsComparator()` when we could already know this by simply looking at the type? So in Qt 6, `QMetaType::registerEqualsComparator`, `QMetaType::registerComparators`, `qRegisterMetaTypeStreamOperators` and `QMetaType::registerDebugStreamOperator` **have been removed**. The metatype system will instead know about those automatically. I suspect we may have to implement the stream operators in the class ourselves. And, if that's actually the case, I would expect the same issue with `ChannelTarget` and `PluginSetting` as well.
Author
Owner

@MaxNarr commented on GitHub (Jan 8, 2025):

Can this warning while running make influence this behavior ?

In member function ‘__ct ’, inlined from ‘detached’ at /usr/include/aarch64-linux-gnu/qt6/QtCore/qhash.h:575:20, inlined from ‘detach’ at /usr/include/aarch64-linux-gnu/qt6/QtCore/qhash.h:939:75, inlined from ‘remove’ at /usr/include/aarch64-linux-gnu/qt6/QtCore/qhash.h:956:15, inlined from ‘remove’ at /home/coms2/mumble/mumble/src/mumble/GlobalShortcut.cpp:1061:28, inlined from ‘__dt_base ’ at /home/coms2/mumble/mumble/src/mumble/GlobalShortcut.cpp:1078:30: /usr/include/aarch64-linux-gnu/qt6/QtCore/qhash.h:559:17: warning: argument 1 value ‘18446744073709551615’ exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=] 559 | spans = new Span[nSpans]; | ^ /usr/include/c++/12/new: In member function ‘__dt_base ’: /usr/include/c++/12/new:128:26: note: in a call to allocation function ‘operator new []’ declared here 128 | _GLIBCXX_NODISCARD void* operator new[](std::size_t) _GLIBCXX_THROW (std::bad_alloc) | ^ [ 99%] Built target mumble [ 99%] Generating delayed configure script configure_files_1.cmake [100%] Executing delayed configure script configure_files_1.cmake [100%] Built target delayed_configure_files_target_1

@MaxNarr commented on GitHub (Jan 8, 2025): Can this warning while running make influence this behavior ? `In member function ‘__ct ’, inlined from ‘detached’ at /usr/include/aarch64-linux-gnu/qt6/QtCore/qhash.h:575:20, inlined from ‘detach’ at /usr/include/aarch64-linux-gnu/qt6/QtCore/qhash.h:939:75, inlined from ‘remove’ at /usr/include/aarch64-linux-gnu/qt6/QtCore/qhash.h:956:15, inlined from ‘remove’ at /home/coms2/mumble/mumble/src/mumble/GlobalShortcut.cpp:1061:28, inlined from ‘__dt_base ’ at /home/coms2/mumble/mumble/src/mumble/GlobalShortcut.cpp:1078:30: /usr/include/aarch64-linux-gnu/qt6/QtCore/qhash.h:559:17: warning: argument 1 value ‘18446744073709551615’ exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=] 559 | spans = new Span[nSpans]; | ^ /usr/include/c++/12/new: In member function ‘__dt_base ’: /usr/include/c++/12/new:128:26: note: in a call to allocation function ‘operator new []’ declared here 128 | _GLIBCXX_NODISCARD void* operator new[](std::size_t) _GLIBCXX_THROW (std::bad_alloc) | ^ [ 99%] Built target mumble [ 99%] Generating delayed configure script configure_files_1.cmake [100%] Executing delayed configure script configure_files_1.cmake [100%] Built target delayed_configure_files_target_1`
Author
Owner

@davidebeatrici commented on GitHub (Jan 8, 2025):

Unlikely, still a warning that shouldn't appear though.

@davidebeatrici commented on GitHub (Jan 8, 2025): Unlikely, still a warning that shouldn't appear though.
Author
Owner

@Krzmbrzl commented on GitHub (Jan 9, 2025):

This is a Qt issue (see https://bugreports.qt.io/browse/QTBUG-105388) and if I understand the bug report correctly, it's harmless 🤷

@Krzmbrzl commented on GitHub (Jan 9, 2025): This is a Qt issue (see https://bugreports.qt.io/browse/QTBUG-105388) and if I understand the bug report correctly, it's harmless :shrug:
Author
Owner

@MaxNarr commented on GitHub (Jan 9, 2025):

What confuses me is that the problem is not there on Mac. But it's there on Raspberry PI

@MaxNarr commented on GitHub (Jan 9, 2025): What confuses me is that the problem is not there on Mac. But it's there on Raspberry PI
Author
Owner

@Krzmbrzl commented on GitHub (Jan 10, 2025):

Most likely an issue due to different Qt versions that are installed 👀

@Krzmbrzl commented on GitHub (Jan 10, 2025): Most likely an issue due to different Qt versions that are installed 👀
Author
Owner

@davidebeatrici commented on GitHub (Jan 10, 2025):

Most likely. I assume the package(s) on Mac come from Homebrew, while the ones on Raspberry Pi come from Debian.

@davidebeatrici commented on GitHub (Jan 10, 2025): Most likely. I assume the package(s) on Mac come from Homebrew, while the ones on Raspberry Pi come from Debian.
Author
Owner

@Krzmbrzl commented on GitHub (Jan 11, 2025):

Hm, this is interesting. From the docs:

To use the type T in QMetaType, QVariant, or with the QObject::property() API, registration is not necessary.

@MaxNarr does the issue also occur when you start with a completely empty settings file? I.e. try to see what happens when you rename ~/.config/Mumble/Mumble/mumble_settings.json and ~/.config/Mumble/Mumble/mumble_settings.json.back to something else and then start Mumble to create the shortcut etc.
My suspicion kinda is that this might be due to the shortcut having been created before the move to Qt 6 and the type IDs and/or the QVariant serialization format has changed from Qt 5 to Qt 6. Hence, the old format can't be read/is misinterpreted 🤔

@Krzmbrzl commented on GitHub (Jan 11, 2025): Hm, this is interesting. From [the docs](https://doc.qt.io/qt-6/qmetatype.html#qRegisterMetaType): > To use the type T in QMetaType, **QVariant**, or with the QObject::property() API, registration is **not** necessary. @MaxNarr does the issue also occur when you start with a completely empty settings file? I.e. try to see what happens when you rename `~/.config/Mumble/Mumble/mumble_settings.json` and `~/.config/Mumble/Mumble/mumble_settings.json.back` to something else and then start Mumble to create the shortcut etc. My suspicion kinda is that this might be due to the shortcut having been created before the move to Qt 6 and the type IDs and/or the QVariant serialization format has changed from Qt 5 to Qt 6. Hence, the old format can't be read/is misinterpreted :thinking:
Author
Owner

@MaxNarr commented on GitHub (Jan 12, 2025):

@Krzmbrzl I renamed the two mentioned files. Unfortunately the behavior is still the same. After restarting mumble the shortcut is listed but the target field is empty

this is the section about shortcuts from mumble_settings.json
"shortcuts": { "defined": [ { "buttons": [ "AAAAAgAAAAAp" ], "data": "AAAEAAAAAAAPU2hvcnRjdXRUYXJnZXQA", "index": 12, "suppress": false } ], "enable_global_shortcuts": true },

this is from mumble_settings.json.back
"shortcuts": { "defined": [ { "buttons": [], "data": "AAAEAAAAAAAPU2hvcnRjdXRUYXJnZXQA", "index": 12, "suppress": false } ], "enable_global_shortcuts": true },

@MaxNarr commented on GitHub (Jan 12, 2025): @Krzmbrzl I renamed the two mentioned files. Unfortunately the behavior is still the same. After restarting mumble the shortcut is listed but the target field is empty this is the section about shortcuts from mumble_settings.json `"shortcuts": { "defined": [ { "buttons": [ "AAAAAgAAAAAp" ], "data": "AAAEAAAAAAAPU2hvcnRjdXRUYXJnZXQA", "index": 12, "suppress": false } ], "enable_global_shortcuts": true }, ` this is from mumble_settings.json.back `"shortcuts": { "defined": [ { "buttons": [], "data": "AAAEAAAAAAAPU2hvcnRjdXRUYXJnZXQA", "index": 12, "suppress": false } ], "enable_global_shortcuts": true }, `
Author
Owner

@MaxNarr commented on GitHub (Feb 5, 2025):

is there any way to make the shortcuts usable again ?

@MaxNarr commented on GitHub (Feb 5, 2025): is there any way to make the shortcuts usable again ?
Author
Owner

@Krzmbrzl commented on GitHub (Feb 6, 2025):

If you find out why they are broken in the first place, then most likely yes. But until then it is unclear what needs to be done about it.

@Krzmbrzl commented on GitHub (Feb 6, 2025): If you find out why they are broken in the first place, then most likely yes. But until then it is unclear what needs to be done about it.
Author
Owner

@MaxNarr commented on GitHub (Feb 19, 2025):

@davidebeatrici

I suspect we may have to implement the stream operators in the class ourselves.

And, if that's actually the case, I would expect the same issue with ChannelTarget and PluginSetting as well.

Aren't they already implemented in Settings.cpp:323?

On Raspberry Pi the latest version is Qt6.4.2
Is that a problem ?

QDataStream &operator<<(QDataStream &qds, const ShortcutTarget &st) {
	// Start by the version of this setting. This is needed to make sure we can stay compatible
	// with older versions (aka don't break existing shortcuts when updating the implementation)
	qds << QString::fromLatin1("v2");

	qds << st.bCurrentSelection << st.bUsers << st.bForceCenter;

	if (st.bCurrentSelection) {
		return qds << st.bLinks << st.bChildren;
	} else if (st.bUsers) {
		return qds << st.qlUsers;
	} else {
		return qds << st.iChannel << st.qsGroup << st.bLinks << st.bChildren;
	}
}

QDataStream &operator>>(QDataStream &qds, ShortcutTarget &st) {
	// Check for presence of a leading version string
	QString versionString;
	QIODevice *device = qds.device();

	if (device) {
		// Qt's way of serializing the stream requires us to read a few characters into
		// the stream in order to get across some leading zeros and other meta stuff.
		char buf[16];

		// Init buf
		for (unsigned int i = 0; i < sizeof(buf); i++) {
			buf[i] = 0;
		}

		qint64 read = device->peek(buf, sizeof(buf));

		for (unsigned int i = 0; i < read; i++) {
			if (buf[i] >= 31) {
				if (buf[i] == 'v') {
					qds >> versionString;
				} else {
					break;
				}
			}
		}
	} else {
		qCritical("Settings: Unable to determine version of setting for ShortcutTarget");
	}

	if (versionString == QLatin1String("v2")) {
		qds >> st.bCurrentSelection;
	}

	qds >> st.bUsers >> st.bForceCenter;

	if (st.bCurrentSelection) {
		return qds >> st.bLinks >> st.bChildren;
	} else if (st.bUsers) {
		return qds >> st.qlUsers;
	} else {
		return qds >> st.iChannel >> st.qsGroup >> st.bLinks >> st.bChildren;
	}
}
@MaxNarr commented on GitHub (Feb 19, 2025): @davidebeatrici > I suspect we may have to implement the stream operators in the class ourselves. > > And, if that's actually the case, I would expect the same issue with `ChannelTarget` and `PluginSetting` as well. Aren't they already implemented in Settings.cpp:323? On Raspberry Pi the latest version is Qt6.4.2 Is that a problem ? ``` QDataStream &operator<<(QDataStream &qds, const ShortcutTarget &st) { // Start by the version of this setting. This is needed to make sure we can stay compatible // with older versions (aka don't break existing shortcuts when updating the implementation) qds << QString::fromLatin1("v2"); qds << st.bCurrentSelection << st.bUsers << st.bForceCenter; if (st.bCurrentSelection) { return qds << st.bLinks << st.bChildren; } else if (st.bUsers) { return qds << st.qlUsers; } else { return qds << st.iChannel << st.qsGroup << st.bLinks << st.bChildren; } } QDataStream &operator>>(QDataStream &qds, ShortcutTarget &st) { // Check for presence of a leading version string QString versionString; QIODevice *device = qds.device(); if (device) { // Qt's way of serializing the stream requires us to read a few characters into // the stream in order to get across some leading zeros and other meta stuff. char buf[16]; // Init buf for (unsigned int i = 0; i < sizeof(buf); i++) { buf[i] = 0; } qint64 read = device->peek(buf, sizeof(buf)); for (unsigned int i = 0; i < read; i++) { if (buf[i] >= 31) { if (buf[i] == 'v') { qds >> versionString; } else { break; } } } } else { qCritical("Settings: Unable to determine version of setting for ShortcutTarget"); } if (versionString == QLatin1String("v2")) { qds >> st.bCurrentSelection; } qds >> st.bUsers >> st.bForceCenter; if (st.bCurrentSelection) { return qds >> st.bLinks >> st.bChildren; } else if (st.bUsers) { return qds >> st.qlUsers; } else { return qds >> st.iChannel >> st.qsGroup >> st.bLinks >> st.bChildren; } } ```
Author
Owner

@MaxNarr commented on GitHub (Feb 20, 2025):

@Krzmbrzl @davidebeatrici
After playing around for a night it appears there's a solution.

The following worked for me:
Move the stream operators from the .cpp file to the .h file (as inline)

are there any issues with doing that ?

@MaxNarr commented on GitHub (Feb 20, 2025): @Krzmbrzl @davidebeatrici After playing around for a night it appears there's a solution. The following worked for me: Move the stream operators from the .cpp file to the .h file (as inline) are there any issues with doing that ?
Author
Owner

@davidebeatrici commented on GitHub (Feb 21, 2025):

No, sounds good to me!

@davidebeatrici commented on GitHub (Feb 21, 2025): No, sounds good to me!
Author
Owner

@Krzmbrzl commented on GitHub (Feb 21, 2025):

I would like to understand why this fixes the issue though... is it enough to have declarations of these operators in the header file? 🤔

@Krzmbrzl commented on GitHub (Feb 21, 2025): I would like to understand why this fixes the issue though... is it enough to have declarations of these operators in the header file? 🤔
Author
Owner

@MaxNarr commented on GitHub (Feb 23, 2025):

@Krzmbrzl I checked. It is enough to have the additional declarations in the headers. Probably it's the same problem with all the operators. I checked ListenToChannel (ChannelTarget) as well, same issue. I have no overview how many things are affected by this.

@MaxNarr commented on GitHub (Feb 23, 2025): @Krzmbrzl I checked. It is enough to have the additional declarations in the headers. Probably it's the same problem with all the operators. I checked ListenToChannel (ChannelTarget) as well, same issue. I have no overview how many things are affected by this.
Author
Owner

@Dessix commented on GitHub (May 27, 2025):

It's been a few months, but this appears to still be occurring on Linux builds on the latest main branch. Do you have a patch with the workaround you mentioned @MaxNarr? Is there anything I can provide to help, since I have it reproing?

@Dessix commented on GitHub (May 27, 2025): It's been a few months, but this appears to still be occurring on Linux builds on the latest main branch. Do you have a patch with the workaround you mentioned @MaxNarr? Is there anything I can provide to help, since I have it reproing?
Author
Owner

@davidebeatrici commented on GitHub (May 29, 2025):

@Dessix Could you test #6832, please?

@davidebeatrici commented on GitHub (May 29, 2025): @Dessix Could you test #6832, please?
Author
Owner

@Dessix commented on GitHub (May 29, 2025):

That worked, but I'll note that it assumed the topmost channel as the default when it decoded what was otherwise blank, instead of nothing. It may be better to leave it blank on first-working-decode than to select the root channel? Otherwise- yeah, it worked.

@Dessix commented on GitHub (May 29, 2025): That worked, but I'll note that it assumed the topmost channel as the default when it decoded what was otherwise blank, instead of nothing. It may be better to leave it blank on first-working-decode than to select the root channel? Otherwise- yeah, it worked.
Author
Owner

@davidebeatrici commented on GitHub (May 29, 2025):

Thank you for testing so quickly!

The PR simply moves the operator definitions and doesn't change the settings behavior itself.

@Krzmbrzl

@davidebeatrici commented on GitHub (May 29, 2025): Thank you for testing so quickly! The PR simply moves the operator definitions and doesn't change the settings behavior itself. @Krzmbrzl
Author
Owner

@Krzmbrzl commented on GitHub (May 29, 2025):

I'll note that it assumed the topmost channel as the default when it decoded what was otherwise blank, instead of nothing. It may be better to leave it blank on first-working-decode than to select the root channel?

Yeah, idk what's up with that. Would have to get into the implementation to see whether I can find some sense in that ^^

@Krzmbrzl commented on GitHub (May 29, 2025): > I'll note that it assumed the topmost channel as the default when it decoded what was otherwise blank, instead of nothing. It may be better to leave it blank on first-working-decode than to select the root channel? Yeah, idk what's up with that. Would have to get into the implementation to see whether I can find some sense in that ^^
Author
Owner

@Hartmnt commented on GitHub (May 31, 2025):

@MaxNarr Thanks for poking at this so a solution could be found! 🎉

@Hartmnt commented on GitHub (May 31, 2025): @MaxNarr Thanks for poking at this so a solution could be found! :tada:
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/mumble-mumble-voip#2934
No description provided.