Skip to content

fix cover sync not marking null correctly#4939

Merged
jurrejelle merged 2 commits into
1.20.1from
gus/sync-nullability-issue
Jun 17, 2026
Merged

fix cover sync not marking null correctly#4939
jurrejelle merged 2 commits into
1.20.1from
gus/sync-nullability-issue

Conversation

@gustovafing

Copy link
Copy Markdown
Member

What

fix cover sync not marking null correctly.
based on #4937

@gustovafing gustovafing requested a review from a team as a code owner June 17, 2026 08:50
@gustovafing gustovafing added type: bugfix General bug fixes Release: Patch - 0.0.X Smaller changes that either are bug fixes or very minor tweaks. labels Jun 17, 2026
@github-actions github-actions Bot added the Tests: Passed Game Tests have passed on this PR label Jun 17, 2026
Comment on lines 22 to 26
if (value != null) {
return serialize(value, context.isClientSync(), context.isClientFullSyncUpdate());
}
return new CompoundTag();

var nullTag = new CompoundTag();
nullTag.putBoolean("null", true);
return nullTag;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you invert this?
if value == null: {return null}
return the normal path
reads more logical to me

Comment on lines +35 to +37
if (compoundTag.getBoolean("null")) {
return null;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

will this return false if the value isnt present?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah it will, getBoolean defaults to false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could be maybe cleaner to do sth like if(compoundTag.hasBoolean("null") && compoundTag.getBoolean("null") == true) but that's up to you

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think its fine as is

@jurrejelle jurrejelle added the ignore changelog PR should not be added to the changelog. label Jun 17, 2026
@jurrejelle jurrejelle enabled auto-merge (squash) June 17, 2026 11:15
@jurrejelle jurrejelle merged commit 79cee98 into 1.20.1 Jun 17, 2026
5 checks passed
@jurrejelle jurrejelle deleted the gus/sync-nullability-issue branch June 17, 2026 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.20.1 ignore changelog PR should not be added to the changelog. Release: Patch - 0.0.X Smaller changes that either are bug fixes or very minor tweaks. Tests: Passed Game Tests have passed on this PR type: bugfix General bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants