[WIP] Syncable objects NNG #447
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sput42
force-pushed
the
pr/syncobj-nng
branch
7 times, most recently
from
8ad70b7
to
0444923
Compare
Sput42
force-pushed
the
pr/syncobj-nng
branch
3 times, most recently
from
37f6f85
to
70f0ef9
Compare
Sput42
force-pushed
the
pr/syncobj-nng
branch
16 times, most recently
from
b520f13
to
73ca169
Compare
Init setters and getters are one of the more obscure features of SyncableObject. In addition to reading and syncing the state of all properties declared with the Q_PROPERTY macro when initializing a client-side SyncableObject, any slot with a name starting with "init" is be called, and its return value added to the property map in the InitData message. The client then calls the corresponding initSet slot with that value. This feature was intended to allow for initialization more complex than just setting property values. However, since the accessors for a property can be freely defined, and because a property does not actually need a corresponding data member (a fact which we probably didn't realize when we originally designed init setters/getters), this behavior can just as well be achieved by declaring a property and its potentially complex READ and WRITE functions accordingly. Using properties directly is also more efficient than going through the list of all slots, mangling method names, manually marshalling arguments, and invoking the slots by name; the moc generates more efficient code for calling property accessors. Because the format of the InitData message luckily does not actually distinguish between normal properties and init getter return values, we can easily and transparently replace init getters/setters by declaring a property named like the original method base name, without breaking protocol. The only caveat is that the method base name starts with a capital letter, slightly violating our naming conventions for property names, but that's a small price to pay for getting rid of the init setter/getter feature altogether. Do this for all init getters/setters, rename the methods as appropriate, un-slot them to avoid unnecessary moc-generated code, and clean up some things while we're at it.
Now that all init setters/getters have been replaced by properties, support for this feature can be removed from SyncableObject.
Qt uses string-based lookup whenever determining the index of a property. Since we do this for every property in every instance of every SyncableObject, so potentially thousands of times whenever a client syncs, let's cache the index in a hash.
Qt allows to declare a NOTIFY signal for properties. Use this feature to automatically sync a SyncableObject's properties that declare such a signal, avoiding the need to manually call SYNC() in the setter. NOTIFY signals need to adhere to the following naming scheme, so we can derive the slot name to be called on receiving a SyncMessage: - Signals "myFooChanged()" or "myFooSet()" will invoke "setMyFoo()" - Signal "syncWhateverSlot()" will invoke "whateverSlot()", allowing for (almost) arbitrary slot names (they need to start with a lower-case letter) NOTIFY signals can have arbitrary (or no) arguments, because the property's value is read explicitly when handling the signal. Modify and extend SignalProxyTest to cover this use case.
Make use of the newly introduced support for property NOTIFY signals to avoid manual calls of SYNC().
Add equality checks to all setters to avoid redundant emission of changed signals. This allows us to remove the conditions when importing a NetworkInfo, too.
Make use of the newly introduced support for property NOTIFY signals to avoid manual calls of SYNC(). Also avoid redundant emission of notify signals.
Make use of the newly introduced support for property NOTIFY signals to avoid manual calls of SYNC(). Also avoid redundant emission of notify signals.
This is the modern way to do things.
Make use of the newly introduced support for property NOTIFY signals to avoid manual calls of SYNC(). Also avoid redundant emission of notify signals.
Make use of the newly introduced support for property NOTIFY signals to avoid manual calls of SYNC().
Make use of the newly introduced support for property NOTIFY signals to avoid manual calls of SYNC(). Also fix signatures passing primitive types as const refs.
Make use of the newly introduced support for property NOTIFY signals to avoid manual calls of SYNC(). Also fix inconsistent signature types.
This class already declared NOTIFY signals, so SYNC() is redundant and would result in duplicate SyncMessages to be sent.
Following up to the automatic sending of property updates for properties with a supported NOTIFY signal, use the same heuristics to directly update the property from the SyncMessage on the client side. This means that instead of invoking the setter dynamically, QMetaProperty::write() is called, which is more efficient and pushes the unmarshalling of the value to Qt. The same heuristics are used for deriving the setter name from the NOTIFY signal as on the client side as on the sending side, so this works for all automatically synced properties as long as they are writable. If no matching property is found, or the value could not be set, the slot will be invoked the old-fashioned way.
Move function traits into a new header traits.h, with the intention to add more traits later. Add alias templates for the different function traits to avoid having to explicitly write 'typename' when using them. Let these alias templates follow STL naming conventions for a more consistent look and feel. Put the alias templates in namespace "traits" to avoid confusion with the std traits. Move invokeWithArgsList() into a new header invoke.h. With this, funchelpers.h can be removed. Adapt existing code accordingly.
Add a helper traits::decay_t<T> that, for a given tuple type T, provides a corresponding tuple type where all individual element types are decayed. This is most useful for normalizing argument tuples.
We only ever associate a SyncableObject with a single SignalProxy, so we don't need to support synchronizing with multiple ones. Simplify things by removing that support.
Add a way to access the proxy mode of the SignalProxy a SyncableObject instance is synchronized with.
Cover the case where a request method does not return a value, both with and without a "matching" (by name) receive method (which should never be called).
Since we can't use strcmp and friends in a constexpr context, provide a constexpr utility method to compare two C strings.
Since this method is being called with qobject_cast pointers, add a null pointer check to be safe even if the cast fails, and avoid the need to add safeguards at the call site.
Extend the SYNCABLE_OBJECT macro to allow for explicitly specifying syncable methods. Use this information, modern C++ features and some template meta programming magic to provide the means for handling SyncMessages and calling sync methods without having to rely on inefficent runtime look-up of methods, the deprecated Qt slot feature, or undocumented Qt APIs. For every sync method declared in the SYNCABLE_OBJECT macro, generate a call wrapper that can be directly invoked by name, and handles the forwarding of marshalled arguments as well as request/receive method functionality. Let SignalProxy make use of this feature for sync methods that have been declared accordingly, while falling back to the legacy way if necessary. Adapt SignalProxyTest so it uses and tests the new implementation. Adapt the existing SyncableObject specializations to using the new SYNCABLE_OBJECT macro syntax, albeit without declaring their sync methods for now; this will be added in follow-up commits. This brings us one step further in our quest to modernize SignalProxy and SyncableObject, and paves another part of the way for the removal of the legacy, inefficent, and hackish way we've been using so far for mapping protocol messages to function calls.
Sput42
force-pushed
the
pr/syncobj-nng
branch
from
73ca169
to
c8c7bc0
Compare
Since we always have the "update" and "requestUpdate" sync methods of SyncableObject itself, we don't need the complex macro machinery to account for the special case where the method list is empty. Simplify the macros accordingly. Unfortunately, MSVC still requires some indirection, so we can't completely get rid of SYNOBJ_APPLY_IMPL and have to use IDENTITY.
Sput42
force-pushed
the
pr/syncobj-nng
branch
from
c8c7bc0
to
64b8e70
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor SignalProxy and SyncableObject to make use of modern C++ features instead of MOC magic and undocumented Qt hacks, for fun, profit and performance!