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

Classify objects #59

Merged
merged 7 commits into from
Jan 1, 2025
Merged

Classify objects #59

merged 7 commits into from
Jan 1, 2025

Conversation

creesch
Copy link
Owner

@creesch creesch commented Dec 31, 2024

@danthedaniel back from the death(backlog)! ;)

  • All favicon related logic now is contained in a class in managers/favicon_manager.mjs
  • Server info is now a class in managers/server_info.mjs
  • Player list is now a class in... you guessed it managers/player_list.mjs

And a few other changes as well that made sense in this context:

  • The querySelectorWithAssertion function is now part of utils.mjs
  • Since this increased the amount of modules it became a bit messy so I decided to organize the modules in a bit of a structure. It is a bit arbitrary but the main point is to avoid one big list of modules.

*/
#hasPing = false;

constructor() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

🥹 it's so beautiful

Copy link
Collaborator

@danthedaniel danthedaniel left a comment

Choose a reason for hiding this comment

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

LGTM

* Clear the favicon state and update the display
*/
clear() {
this.#messageCount = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Private variables! I've never use those in JS.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah they are quite neat. Relatively recent but old enough that they are supported in any browser of the last few years.

src/client/resources/web/js/managers/player_list.mjs Outdated Show resolved Hide resolved
@creesch creesch merged commit 81a51eb into main Jan 1, 2025
4 checks passed
@creesch creesch deleted the classify-objects branch January 1, 2025 11:03
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