Skip to content

rewrite registry/registry list settings #5753

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: mc-update
Choose a base branch
from
Open

rewrite registry/registry list settings #5753

wants to merge 2 commits into from
+409 −608

Conversation

MukjepScarlet
Copy link
Contributor

@MukjepScarlet MukjepScarlet commented Oct 7, 2025 •
edited
Loading

Type of change

  • Performance improvement

Description

We have many registry-based values like Block/Item... But now they are of List. This causes issue if we:

  • Use .contains on them. Typical List.contains is of O(n) and Set.contains is of O(1)~O(log n).
  • Add same values, ArrayList doesn't prevent us to do that.

This PR is in order to:

  • Migrate same code of these settings
  • Replace List with Set of them

How Has This Been Tested?

Videos or screenshots of the changes if applicable.

Checklist:

  • My code follows the style guidelines of this project.
  • I have added comments to my code in more complex areas.
  • I have tested the code in both development and production environments.

@MukjepScarlet MukjepScarlet changed the title start rewrite registry/registry list settings Oct 7, 2025
@MukjepScarlet MukjepScarlet marked this pull request as ready for review October 7, 2025 12:37
RacoonDog
RacoonDog suggested changes Oct 7, 2025
View reviewed changes
Copy link
Collaborator

@RacoonDog RacoonDog left a comment

Choose a reason for hiding this comment

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

since switching the setting types is a very breaking change, pr should target the mc-update branch rather than master

@MukjepScarlet MukjepScarlet changed the base branch from master to mc-update October 7, 2025 19:22
@MukjepScarlet MukjepScarlet changed the base branch from mc-update to master October 7, 2025 19:36
@MukjepScarlet MukjepScarlet changed the base branch from master to mc-update October 7, 2025 19:36
@MukjepScarlet MukjepScarlet force-pushed the rewrite-registry-list branch 2 times, most recently from a755af4 to f9f604b Compare October 7, 2025 20:07
@MukjepScarlet
Copy link
Contributor Author

I think I messed up the commit history of this branch...

@Wide-Cat
Copy link
Collaborator

Wide-Cat commented Oct 7, 2025

you tried to merge this into the mc-update branch

@MukjepScarlet
Copy link
Contributor Author

I'll try to clean it later

@MukjepScarlet MukjepScarlet force-pushed the rewrite-registry-list branch from f9f604b to 4e7f399 Compare October 8, 2025 09:33
@MukjepScarlet
Copy link
Contributor Author

Okay now cleaned

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RacoonDog RacoonDog Awaiting requested review from RacoonDog

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MukjepScarlet @Wide-Cat @RacoonDog