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

"username_display": "displayname" does not work #149

Closed
hrueschwein opened this issue Aug 20, 2023 · 7 comments · Fixed by #171
Closed

"username_display": "displayname" does not work #149

hrueschwein opened this issue Aug 20, 2023 · 7 comments · Fixed by #171

Comments

@hrueschwein
Copy link

OS: openSUSE Tumbleweed
Iamb version: 0.0.8 (installed from OBS repo home:smolsheep)
Logs (level: debug): https://paste.opensuse.org/493c0afc2a1e
Config:

{
	"profiles": {
		"kvasyan": {
			"user_id": "@kvasyan:sibnsk.net",
			"url": "https://matrix.sibnsk.net"
		}
	},
	"default_profile": "kvasyan",
	"settings": {
		"log_level": "debug",
		"username_display": "displayname"
	},
	"layout": {
		"style": "restore"
	}
}

Thank you very much for your work! Unfortunately, i've faced one issue.

Iamb just puts Matrix ID (@user:example.com) instead of display names. Tried in rooms with from 10 to 500 members and even in dms.
It's very uncomfortable in rooms with bridges to other messengers, because every username transforms into sth like @telegram_12345678910:t2bot.io.

Thanks for attention.

@u20n
Copy link
Contributor

u20n commented Sep 24, 2023

+1
OS is Arch (via AUR), iamb -V returns iamb 0.0.9-alpha.1 (c7864cb).
Confirmed relevant config was being loaded, but display style doesn't change.

@u20n
Copy link
Contributor

u20n commented Sep 24, 2023

Actually updating to 0.0.9 did solve it. (whoops)

@hrueschwein
Copy link
Author

Actually updating to 0.0.9 did solve it. (whoops)

But the latest release is 0.0.8...

@u20n
Copy link
Contributor

u20n commented Sep 24, 2023

v0.0.8 was released Jul 8, this issue was made Aug 20. Any progress would be on the v0.0.9 alpha, which is the git HEAD. There is some inconsistency whether or not displayname is shown, I've experienced both it working and not working with no change to the config file.

Possibly relevant logs:

Error("unknown variant `event_property_contains`, expected one of `event_match`, `contains_display_name`, `room_member_count`, `sender_notification_permission`", line: 1, column: 2552)
Error deserializing event Error("missing field `canonical`", line: 1, column: 22)

@u20n
Copy link
Contributor

u20n commented Sep 24, 2023

Removing the relevant session token fixes it for the rest of the session.

@photosheep
Copy link
Contributor

Maintainer of that repo. I updated the repo to git HEAD to solve this. It required removal of session token like u20n says, but it works on latest repo. Downgraded afterwards to confirm confirm 0.0.8's username_display wasn't working properly.
Doing a zypper dup and cleaning session should fix the issue.

@Benjamin-L
Copy link
Contributor

I believe I found the source of the display_name inconsistency in v0.0.9 alpha while working on #168. The UI uses the RoomInfo::display_name field to look up user display names. This field is only updated when syncing the m.room.member event for the first time. When iamb is restarted, the room members are already persisted to the database, and this event handler is not triggered a second time. The only situations where display names work is during the first run of iamb for a given session and when somebody joins a room during the current run.

This should be fixable by populating the RoomInfo::display_names field from room.members_no_sync when a room is loaded. Probably worth checking that this doesn't eat all the CPU when iterating over all the members of a large room though.

benjajaja added a commit to benjajaja/iamb that referenced this issue Oct 26, 2023
Fixes ulyssa#149

ClientWorker::get_room extends the returned FetchedRoom to include
RoomMembers from Room::members_no_sync. This allows RoomState to update
the RoomInfo's display_names, like it does with name and tags.
benjajaja added a commit to benjajaja/iamb that referenced this issue Oct 26, 2023
Fixes ulyssa#149

ClientWorker::get_room extends the returned FetchedRoom to include
RoomMembers from Room::members_no_sync. This allows RoomState to update
the RoomInfo's display_names, like it does with name and tags.

Thanks to <benjamin@computer.surgery> for finding the source of the bug
in that issue. It was mentioned that it might be problematic to iterate
over all members of large rooms. The largest room I had available to
test this was only about 200 members, but it seemed fine.
benjajaja added a commit to benjajaja/iamb that referenced this issue Oct 27, 2023
Fixes ulyssa#149

The display_names only got populated from the
OriginalSyncRoomMemberEvent. When opening an already synced room,
display_names is not populated.

ClientWorker::get_room extends the returned FetchedRoom to include
RoomMembers from Room::members_no_sync. This allows RoomState::new to
populate the RoomInfo's display_names.

Thanks to <benjamin@computer.surgery> for finding the source of the bug
in that issue. It was mentioned that it might be problematic to iterate
over all members of large rooms. The largest room I had available to
test this was only about 200 members, but it seemed fine.
benjajaja added a commit to benjajaja/iamb that referenced this issue Nov 25, 2023
Fixes ulyssa#149

The display_names were only populated from the
OriginalSyncRoomMemberEvent. When opening an already synced room,
display_names is not populated.

The ChatStore's `need_load` is changed to a hashmap with OwnedRoomIds as
keys and a bitmask of "needs" as value. The "need" thus can be `MESSAGES`
or `MEMBERS`. The task that loads messages now also loads members, if
that flag is set for the room, which is done when opening a window.

Thanks to <benjamin@computer.surgery> for finding the source of the bug
in that issue.
benjajaja added a commit to benjajaja/iamb that referenced this issue Nov 25, 2023
Fixes ulyssa#149

The display_names were only populated from the
OriginalSyncRoomMemberEvent. When opening an already synced room,
display_names is not populated.

The ChatStore's `need_load` is changed to a hashmap with OwnedRoomIds as
keys and a bitmask of "needs" as value. The "need" thus can be `MESSAGES`
or `MEMBERS`. The task that loads messages now also loads members, if
that flag is set for the room, which is done when opening a window.

Thanks to <benjamin@computer.surgery> for finding the source of the bug
in that issue.
benjajaja added a commit to benjajaja/iamb that referenced this issue Nov 25, 2023
Fixes ulyssa#149

The display_names were only populated from the
OriginalSyncRoomMemberEvent. When opening an already synced room,
display_names is not populated.

The ChatStore's `need_load` is changed to a hashmap with OwnedRoomIds as
keys and a bitmask of "needs" as value. The "need" thus can be `MESSAGES`
or `MEMBERS`. The task that loads messages now also loads members, if
that flag is set for the room, which is done when opening a window.

Thanks to <benjamin@computer.surgery> for finding the source of the bug
in that issue.
benjajaja added a commit to benjajaja/iamb that referenced this issue Nov 25, 2023
Fixes ulyssa#149

The display_names were only populated from the
OriginalSyncRoomMemberEvent. When opening an already synced room,
display_names is not populated.

The ChatStore's `need_load` is changed to a hashmap with OwnedRoomIds as
keys and a bitmask of "needs" as value. The "need" thus can be `MESSAGES`
or `MEMBERS`. The task that loads messages now also loads members, if
that flag is set for the room, which is done when opening a window.

Thanks to <benjamin@computer.surgery> for finding the source of the bug
in that issue.
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 a pull request may close this issue.

4 participants