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

Implement spectator list display #31526

Merged
merged 8 commits into from
Jan 17, 2025
Merged

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Jan 15, 2025

2025-01-15.14-19-23.mp4

Roughly uses design shown in #22795 (comment) with some modifications to better fit everything else, and some customisation options so it can fit better on other skins.

- First step for ppy#22087
- Supersedes / closes ppy#22795

Roughly uses design shown in
ppy#22795 (comment) with some
modifications to better fit everything else, and some customisation
options so it can fit better on other skins.
@peppy
Copy link
Member

peppy commented Jan 16, 2025

I pushed test fixes and some visual changes. @bdach please confirm you're okay with the new commits.

peppy
peppy previously approved these changes Jan 16, 2025
bdach added 2 commits January 16, 2025 14:07
…ith spectators already present

Noticed by accident, but if the `BindCollectionChanged()` callback fires
immediately in `LoadComplete()` when set up and there are spectators
present already, then `NewStartingIndex` in the related event is -1:

	https://github.com/dotnet/runtime/blob/b03f83de362f7168c94daa2f4b192959abefe366/src/libraries/System.ObjectModel/src/System/Collections/Specialized/NotifyCollectionChangedEventArgs.cs#L84-L92

which kinda breaks the math introducing off-by-ones and in result causes
11 items to be displayed together rather than 10.
@bdach
Copy link
Collaborator Author

bdach commented Jan 16, 2025

New commits look okay 👍

In passing, thanks to the "last name gradiented" change, I noticed that I inadvertently did an off-by-one in the collection change callback; see 81f5450 for more info. Nothing to add other than that.

@peppy peppy merged commit 3272224 into ppy:master Jan 17, 2025
7 of 10 checks passed
@bdach bdach deleted the spectator-list-visuals branch January 17, 2025 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants