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

feat: add knockroom api #615

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

Conversation

Decodetalkers
Copy link

add a knockRoom to connection.h

Copy link
Member

@KitsuneRal KitsuneRal left a comment

Choose a reason for hiding this comment

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

I'm afraid it's just a tad harder than just that.

Comment on lines 1149 to 1154
// with a sync yet. If the room object is not there, provideRoom() will
// create it in Join state. finished() is used here instead of success()
// to overtake clients that may add their own slots to finished().
connect(job, &BaseJob::finished, this, [this, job] {
if (job->status().good())
provideRoom(job->roomId());
Copy link
Member

Choose a reason for hiding this comment

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

You really should have read this comment before copying it from joinRoom()... provideRoom() creates a room in Join state by default - this is not the desired behaviour here.

Suggested change
// with a sync yet. If the room object is not there, provideRoom() will
// create it in Join state. finished() is used here instead of success()
// to overtake clients that may add their own slots to finished().
connect(job, &BaseJob::finished, this, [this, job] {
if (job->status().good())
provideRoom(job->roomId());
// with a sync yet. If the room object is not there, provideRoom() will
// create it in Knock state. finished() is used here instead of success()
// to overtake clients that may add their own slots to finished().
connect(job, &BaseJob::finished, this, [this, job] {
if (job->status().good())
provideRoom(job->roomId(), JoinState::Knock);

There's yet another potential problem here: provideRoom() was written with just three join states in mind, and I cannot figure off the top of my head how it will behave if the room is found but is not in the Leave state. The code of provideRoom() should be reviewed and possibly updated to make sure Quotient clients don't accidentally turn, e.g., already joined rooms into knock rooms (the homeserver won't allow that but the local state may be screwed).

@KitsuneRal KitsuneRal added the enhancement A feature or change request for the library label Feb 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature or change request for the library
Projects
Status: 0.9 - To Do
Development

Successfully merging this pull request may close these issues.

2 participants