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

Add support for partially synced client tags #3118

Merged
merged 15 commits into from
Jul 3, 2023

Conversation

dexman545
Copy link
Contributor

Allows for client tags to reference tags that may be synced from a server, allowing for them to be used.

An example of this being useful is in AutoSwitch. To address mc:sword_efficient not containing bamboo, a wrapper tag is needed as:sword_efficient. With partially synced tags it would allow for server-side changes to mc:sword_efficient to be seen in as:sword_efficient, which better reflects the intent of the tag.

This is a draft as implementation is worked out, some todos are left in with some questions.

@dexman545 dexman545 marked this pull request as ready for review June 20, 2023 18:24
@Technici4n Technici4n requested a review from a team June 21, 2023 09:14
@modmuss50 modmuss50 self-requested a review June 23, 2023 16:17
Copy link
Member

@modmuss50 modmuss50 left a comment

Choose a reason for hiding this comment

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

Looks great 👍 I just have a few questions/nit picks.

dexman545 and others added 4 commits June 23, 2023 13:42
Co-authored-by: modmuss <modmuss50@gmail.com>
Don't recurse through tag hierarchy
@Technici4n
Copy link
Member

Your change to the recursion handling means that nested tags won't be correctly overridden anymore, as far as I can tell.

@dexman545
Copy link
Contributor Author

Your change to the recursion handling means that nested tags won't be correctly overridden anymore, as far as I can tell.

Good catch. I've restored the recursive search, but added tracking of already tested tags so we should now always terminate.

@Technici4n Technici4n self-requested a review June 24, 2023 21:22
@modmuss50 modmuss50 added enhancement New feature or request last call If you care, make yourself heard right away! labels Jun 25, 2023
@modmuss50 modmuss50 added the merge me please Pull requests that are ready to merge label Jul 2, 2023
@modmuss50 modmuss50 merged commit 97bb207 into FabricMC:1.20.1 Jul 3, 2023
5 checks passed
modmuss50 added a commit that referenced this pull request Jul 3, 2023
* Draft v1.1.0

* Resolve some comments

* Add javadoc

* Remove old behavior

* Minor cleanup

* Add test for partially synced tags

* Address nitpick

* Fix checkstyle

* Hard fail when datapack fails to regsiter

Co-authored-by: modmuss <modmuss50@gmail.com>

* Fix missing import

* Refactor
Don't recurse through tag hierarchy

* Add note for test

* Adjustments to logic to handle server-missing nested tags

* Restore recursive search, add tracking of checked tags

* Cleanup

---------

Co-authored-by: modmuss <modmuss50@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request last call If you care, make yourself heard right away! merge me please Pull requests that are ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants