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

Add IPC property consistency test #305

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jacky309
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@bitmouse bitmouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not quite sure what is the purpose of this test. Moreover the behavior seems to be wrong - when we receive boolProperty1Changed, we expect only this one to be changed. When we receive boolProperty2Changed, we expect it to change. We do not expect cross-changes. From the server POV it is the same property, but from the client POV these are completely separate things. If you need to make it consistent in the proposed way, you should pack it into a structure.

tests/propertyconsistency/client.cpp Outdated Show resolved Hide resolved
@jacky309
Copy link
Collaborator Author

I am not quite sure what is the purpose of this test. Moreover the behavior seems to be wrong - when we receive boolProperty1Changed, we expect only this one to be changed. When we receive boolProperty2Changed, we expect it to change. We do not expect cross-changes. From the server POV it is the same property, but from the client POV these are completely separate things. If you need to make it consistent in the proposed way, you should pack it into a structure.

The properties are guaranteed to be equal on the server side and the purpose of the IPC framework is to replicate the interface on the client side, so they have to be equal there as well. We do not want group multiple properties into one for the reasons which I already explained.
FYI, the test passes.

@bitmouse
Copy link
Collaborator

The properties are guaranteed to be equal on the server side and the purpose of the IPC framework is to replicate the interface on the client side, so they have to be equal there as well. We do not want group multiple properties into one for the reasons which I already explained.

This makes no sense. Properties are the same for the server, they should be replicated ASAP to the client. That part is OK. However the assumption that they are the same on the client side is wrong, client does not have this knowledge. Also we do not replicate the interface but its state.

What you have described is the problem with state of the interface that I mentioned in my other comments. If the state of the interface changes, then you have to update all properties, so the only correct way is to send whole state at once, even if properties are not changed. This is not an efficient way. Moving the state towards application rather than interface, and decoupling properties solves this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants