Jake Potrebic
- Machine Maker
- JIRAUSER570737
- America/Los_Angeles
- Yes
- No
Low![[Mojang] Gegy](../../../data/avatars/users/10122.svg){kind=small}
MC-264081![[Mojang] Brandon Pearce](../../../data/avatars/users/40932.png){kind=small}
Normal
0752-Fix-entity-type-tags-suggestions-in-selectors.patch
By hide I mean obfuscate to the client user. The client has access to all the text in the component, but there are parts that aren't visible in the preview window without some extra action by the user.
A server may send a preview response that changes one `.` in the message to have a hover component that contains objectionable material by mojangs chat standards.
As a player, I have a few options to deal with this situation, both less than ideal.
- A player, before sending any message, could hover (and click) on every character in the preview response before sending a message. This is the only way (on the vanilla client) to ensure all the text contained in the preview is text that I, the player, agree to sign by sending the message.
- Just disable previewing altogether which takes a way an important feature servers have to modify messages while still allowing the player to sign them
- Just ignore the previews leaving me open to malicious servers trying to attribute text to me that I wasn't able to visually read
I think all these options are bad, and some change should be done to address this potential path a server has to falsely attributing content to a player in a signed message.
By hide I mean obfuscate to the client user. The client has access to all the text in the component, but there are parts that aren't visible in the preview window without some extra action by the user.
A server may send a preview response that changes one `.` in the message to have a hover component that contains objectionable material by mojangs chat standards.
As a player, I have a few options to deal with this situation, both less than ideal.
- A player, before sending any message, could hover (and click) on every character in the preview response before sending a message. This is the only way (on the vanilla client) to ensure all the text contained in the preview is text that I, the player, agree to sign by sending the message.
- Just disable previewing altogether which takes a way an important feature servers have to modify messages while still allowing the player to sign them
- Just ignore the previews leaving me open to malicious servers trying to attribute text to me that I wasn't able to visually read
I think all these options are bad, and some change should be done to address this potential path a server has to falsely attributing content to a player in a signed message.
By hide I mean obfuscate to the client user. The client has access to all the text in the component, but there are parts that aren't visible in the preview window without some extra action by the user.
A server may send a preview response that changes one `.` in the message to have a hover component that contains objectionable material by mojangs chat standards. By default, a client can't insert hover text, they only send a string to the server to then be run through the server's ChatDecorator. But if the server made use of a parser for syntax, such as MiniMessage which is a string format for minecraft components, then the previewed component might accurately represent what the user meant to type OR the server maliciously modified it. There is no way to know what happened.
As a player, I have a few options to deal with this situation, both less than ideal.
- A player, before sending any message, could hover (and click) on every character in the preview response before sending a message. This is the only way (on the vanilla client) to ensure all the text contained in the preview is text that I, the player, agree to sign by sending the message.
- Just disable previewing altogether which takes a way an important feature servers have to modify messages while still allowing the player to sign them
- Just ignore the previews leaving me open to malicious servers trying to attribute text to me that I wasn't able to visually read
I think all these options are bad, and some change should be done to address this potential path a server has to falsely attributing content to a player in a signed message.
(All code references are using mojang's mappings)
Each experience point should repair 2 durability according to "ExperienceOrb#xpToDurability" but the recursive "ExperienceOrb#repairPlayerItems"
doesn't use the correct value to calculate the xp amount left over for more repairs. It still uses the overall value of the orb for each call of "repairPlayerItems" instead of the "amount" parameter which is the correct value after the first recursive call.
You can see the incorrect results by doing the following:
- Give yourself 2 pieces of armor both with 20 damage with
/give @s <some_armor>{Damage:20}
and equip them.
- Apply the mending enchant to both of them with
/enchant @s mending- Spawn an experience orb with a value of 11 with
/summon experience_orb ~5 ~ ~5 {Value:11}- Collect the xp orb and observe that both armor pieces were fully repaired, repairing 40 damage while only 22 should've been possible.
For this issue to occur, its required that 1 item have less durability that the xp value * 2 so the function has a chance to engage in recursion.
The fix should be as simple as using the"amount" parameter instead of "this.value".(All code references are using mojang's mappings)
Each experience point should repair 2 durability according to "ExperienceOrb#xpToDurability" but the recursive "ExperienceOrb#repairPlayerItems"
doesn't use the correct value to calculate the xp amount left over for more repairs. It still uses the overall value of the orb for each call of "repairPlayerItems" instead of the "amount" parameter which is the correct value after the first recursive call.
You can see the incorrect results by doing the following:
- Give yourself 2 pieces of armor both with 20 damage with
/give @s <some_armor>{Damage:20}and equip them.
- Apply the mending enchant to both of them with
/enchant @s mending
- Spawn an experience orb with a value of 11 with
/summon experience_orb ~5 ~ ~5 {Value:11}
- Collect the xp orb and observe that both armor pieces were fully repaired, repairing 40 damage while only 22 should've been possible.
For this issue to occur, its required that 1 item have less durability that the xp value * 2 so the function has a chance to engage in recursion.
The fix should be as simple as using the "amount" parameter instead of "this.value".
EDIT #1: This has probably been a bug since 21w13a which included the fixes for https://bugs.mojang.com/browse/MC-194183 and https://bugs.mojang.com/browse/MC-205918
which I assume caused this, looking at the diff between ExperienceOrb.java for that version and 1.20-rc1.
(All code references are using mojang's mappings)
Each experience point should repair 2 durability according to "ExperienceOrb#xpToDurability" but the recursive "ExperienceOrb#repairPlayerItems"
doesn't use the correct value to calculate the xp amount left over for more repairs. It still uses the overall value of the orb for each call of "repairPlayerItems" instead of the "amount" parameter which is the correct value after the first recursive call.
You can see the incorrect results by doing the following:
- Give yourself 2 pieces of armor both with 20 damage with
/give @s <some_armor>{Damage:20}and equip them.
- Apply the mending enchant to both of them with
/enchant @s mending
- Spawn an experience orb with a value of 11 with
/summon experience_orb ~5 ~ ~5 {Value:11}
- Collect the xp orb and observe that both armor pieces were fully repaired, repairing 40 damage while only 22 should've been possible.
For this issue to occur, its required that 1 item have less durability that the xp value * 2 so the function has a chance to engage in recursion.
The fix should be as simple as using the "amount" parameter instead of "this.value".
EDIT #1: This has probably been a bug since 21w13a which included the fixes for https://bugs.mojang.com/browse/MC-194183 and https://bugs.mojang.com/browse/MC-205918
which I assume caused this, looking at the diff between ExperienceOrb.java for that version and 1.20-rc1.
(All code references are using mojang's mappings)
Each experience point should repair 2 durability according to "ExperienceOrb#xpToDurability" but the recursive "ExperienceOrb#repairPlayerItems"
doesn't use the correct value to calculate the xp amount left over for more repairs. It still uses the overall value of the orb for each call of "repairPlayerItems" instead of the "amount" parameter which is the correct value after the first recursive call.
You can see the incorrect results by doing the following:
- Give yourself 2 pieces of armor both with 20 damage with
/give @s <some_armor>{Damage:20}and equip them.
- Apply the mending enchant to both of them with
/enchant @s mending
- Spawn an experience orb with a value of 11 with
/summon experience_orb ~5 ~ ~5 {Value:11}
- Collect the xp orb and observe that both armor pieces were fully repaired, repairing 40 damage while only 22 should've been possible.
For this issue to occur, its required that 1 item have less durability that the xp value * 2 so the function has a chance to engage in recursion.
The fix should be as simple as using the "amount" parameter instead of "this.value".
EDIT #1: This has probably been a bug since 21w13a which included the fixes for https://bugs.mojang.com/browse/MC-194183 and https://bugs.mojang.com/browse/MC-205918 which I assume caused this, looking at the diff between ExperienceOrb.java for that version and 1.20-rc1.
The fix for
MC-257875which was to add a check to see if the itemstack had a damageable item failed to take into account the "Unbreakable" nbt tag. Currently, if you give yourself an unbreakable flint and steel via/give @s flint_and_steel{Unbreakable:1b}and ignite a creeper with it in survival mode, the itemstack is completely removed due to the stack size being shrunk by one in Creeper#mobInteract.
The proper fix I believe is shown here.
The fix for
MC-257875which was to add a check to see if the itemstack had a damageable item failed to take into account the "Unbreakable" nbt tag. Currently, if you give yourself an unbreakable flint and steel via/give @s flint_and_steel{Unbreakable:1b}and ignite a creeper with it in survival mode, the itemstack is completely removed due to the stack size being shrunk by one in Creeper#mobInteract.
The proper fix I believe is shown here.
The fix for
MC-257875which was to add a check to see if the itemstack had a damageable item failed to take into account the "Unbreakable" nbt tag. Currently, if you give yourself an unbreakable flint and steel via/give @s flint_and_steel{Unbreakable:1b}and ignite a creeper with it in survival mode, the itemstack is completely removed due to the stack size being shrunk by one in Creeper#mobInteract.
The proper fix I believe is shown here.
The fix for
MC-257875which was to add a check to see if the itemstack had a damageable item failed to take into account the "Unbreakable" nbt tag. Currently, if you give yourself an unbreakable flint and steel via/give @s flint_and_steel{Unbreakable:1b}and ignite a creeper with it in survival mode, the itemstack is completely removed due to the stack size being shrunk by one in Creeper#mobInteract.
The proper fix I believe is shown here.
The fix for
MC-257875which was to add a check to see if the itemstack had a damageable item failed to take into account the "Unbreakable" nbt tag. Currently, if you give yourself an unbreakable flint and steel via/give @s flint_and_steel{Unbreakable:1b}and ignite a creeper with it in survival mode, the itemstack is completely removed due to the stack size being shrunk by one in Creeper#mobInteract.
The proper fix I believe is shown here in the Paper repository.
This command outputs different text in singleplayer vs. multiplayer. On singleplayer, you get a "true hey" message in chat while on multiplayer you get a "1 hey" message in chat.
/tellraw @s {"translate":"%s %s","with":[true, "hey"]}This is caused by the serialization process for components using nbt now which does not preserve boolean information. When the component is serialized to NBT the arguments ("with") becomes
[{"":1b},{"":"hey"}]and when the client deserializes that, it reads the 1b as a numeric 1 byte which is then displayed as a 1.
This issue has existed for a while, but until 24w10a it
wasn'treally applicableto vanilla as there was no way to add recipes to a stonecutter for which the result itemstack had any custom data. With data components and the codec's for recipes being updated to allow custom data, this issue is nowpresentin vanilla.You cannot add 2 recipes with a common input whose outputs are the same item type. I have attached a datapack that reproduces the problem, adding 16 recipes, one for each light block level when given an oak plank as an input. You can see that by clicking on the buttons for each recipe, the output does not match the button you clicked.
The cause
of thisis a little complex and I first explained it here. The gistof itis,that thestonecutter recipe selection system relies on a list index for all applicable recipes being the same on the client and server. But the *ordering* of that list is set by RecipeManager#getRecipesFor(RecipeType, Container, Level). In getRecipesFor, the list is sorted using a Comparator that compares the description IDs for the result item which leads to ambiguous ordering when 2 identical result items are present. So the indices in this list get out of sync between the client and the server leading to the visual inconsistency between the button icons and the itemstack put into the result slot of the stonecutter inventory.This issue has existed for a while, but until 24w10a it didn't apply to vanilla as there was no way to add recipes to a stonecutter for which the result itemstack had any custom data. With data components and the codecs for recipes being updated to allow custom data, this issue is now reproducible in vanilla.
You cannot add 2 recipes with a common input whose outputs are the same item type. I have attached a datapack that reproduces the problem, adding 16 recipes, one for each light block level when given an oak plank as an input. You can see that by clicking on the buttons for each recipe, the output does not match the button you clicked.
The cause is a little complex and I first explained it here. The gist is that the Stonecutter recipe selection system relies on a list index for all applicable recipes being the same on the client and server. But the *ordering* of that list is set by RecipeManager#getRecipesFor(RecipeType, Container, Level). In getRecipesFor, the list is sorted using a Comparator that compares the description IDs for the result item which leads to ambiguous ordering when 2 identical result items are present. So the indices in this list get out of sync between the client and the server leading to the visual inconsistency between the button icons and the itemstack put into the result slot of the stonecutter inventory.
Steps to Reproduce
- Download and install the attached test_datapack.
- Place an oak_plank inside a Stonecutter and click around on the various recipes that show up, recipes that are added by the datapack
- Observe that the item created in the result slot does not match the recipe that you clicked on.
The minecraft:decorated_pot item type is missing the minecraft:container data component from it's "implicit" data components. This causes the pick block functionality to be inconsistent for decorated pots. Placing down a plain decorated pot with /give @s decorated_pot and then using (regular) middle-click to pick it back up and inspecting the two, we see
{"minecraft:container": []}
Original: {count: 1, id: "minecraft:decorated_pot"}
Picked:{{{components:, count: 1, id: "minecraft:decorated_pot"}}}
This is because the decorated_pot definition in Items.java doesn't have a default component value registered for the container data component type. Every other block entity that has a "container" has a default value set for the container component type which means that after picking the block for those, the 2 components are equal and it isn't included in the data component patch for the stack.
The minecraft:decorated_pot item type is missing the minecraft:container data component from it's "implicit" data components. This causes the pick block functionality to be inconsistent for decorated pots. Placing down a plain decorated pot with /give @s decorated_pot and then using (regular) middle-click to pick it back up and inspecting the two, we see
Original: {count: 1, id: "minecraft:decorated_pot"}
Picked:{components: {"minecraft:container": []}, count: 1, id: "minecraft:decorated_pot"}This is because the decorated_pot definition in Items.java doesn't have a default component value registered for the container data component type. Every other block entity that has a "container" has a default value set for the container component type which means that after picking the block for those, the 2 components are equal and it isn't included in the data component patch for the stack.
The minecraft:decorated_pot item type is missing the minecraft:container data component from it's "implicit" data components. This causes the pick block functionality to be inconsistent for decorated pots. Placing down a plain decorated pot with /give @s decorated_pot and then using (regular) middle-click to pick it back up and inspecting the two, we see
Original: {count: 1, id: "minecraft:decorated_pot"}
Picked: {components: {"minecraft:container": []}, count: 1, id: "minecraft:decorated_pot"}This is because the decorated_pot definition in Items.java doesn't have a default component value registered for the container data component type. Every other block entity that has a "container" has a default value set for the container component type which means that after picking the block for those, the 2 components are equal and it isn't included in the data component patch for the stack.
Problem
Before the change over to data components, the HideFlags nbt could be set without having to set any data that would be hidden by the flag. However, now that the show_in_tooltip is part of the attributes data component (and a bunch of others), you have to actually set some attributes in order to change it to false.
Incorrect Solutions
- You cannot just set an empty list of attributes because that overrides the implicit attributes (for non-armor items – see
MC-271826). Setting an empty list of attributes just removes the attributes- Copying over the default attributes makes the item not inherit changes to the default attributes made by the game in different versions. If all I want to do is hide the attributes and ensure the default attributes are still used, I can't copy them over
This should, therefore, be considered a regression in behavior as you could do this pre-data components.
Other data components suffer from the same issue, however none of the other data components that have included tooltip toggles have implicit values for an item type while the attributes one does.
Possible Solution
- Making the attribute modifier list an "optional" to signify that no modifiers have been set introduces a new state separate from an empty list. Setting an empty list still should mean you want no modifiers, but an empty optional means fallback to the possible implicit data components
- Could split out the hide tooltip into a separate data component.
Problem
Before the change over to data components, the HideFlags nbt could be set without having to set any data that would be hidden by the flag. However, now that the show_in_tooltip is part of the attributes data component (and a bunch of others), you have to actually set some attributes in order to change it to false.
Steps to Reproduce
/give @s wooden_pickaxe[minecraft:attribute_modifiers={show_in_tooltip:0b,modifiers:[]}]This command gives you a wooden pickaxe that hides the modifiers, but in order to do so, you have to actually set the modifiers list to an empty list, which now removes the default modifiers.
- Use the attached datapack with an item modifier and run that modifier on the item in your hand. The toggle_tooltips function does hide the tooltips while keeping the modifiers, but the implicit modifiers for that item type are copied over to the stack itself, so future changes to the default of the stack won't affect this stack.
Incorrect Solutions
- You cannot just set an empty list of attributes because that overrides the implicit attributes (for non-armor items – see
MC-271826). Setting an empty list of attributes just removes the attributes- Copying over the default attributes makes the item not inherit changes to the default attributes made by the game in different versions. If all I want to do is hide the attributes and ensure the default attributes are still used, I can't copy them over
This should, therefore, be considered a regression in behavior as you could do this pre-data components.
Other data components suffer from the same issue, however none of the other data components that have included tooltip toggles have implicit values for an item type while the attributes one does.
Possible Solution
- Making the attribute modifier list an "optional" to signify that no modifiers have been set introduces a new state separate from an empty list. Setting an empty list still should mean you want no modifiers, but an empty optional means fallback to the possible implicit data components
- Could split out the hide tooltip into a separate data component.
Problem
Before the change over to data components, the HideFlags nbt could be set without having to set any data that would be hidden by the flag. However, now that the show_in_tooltip is part of the attributes data component (and a bunch of others), you have to actually set some attributes in order to change it to false.
Steps to Reproduce
/give @s wooden_pickaxe[minecraft:attribute_modifiers={show_in_tooltip:0b,modifiers:[]}]This command gives you a wooden pickaxe that hides the modifiers, but in order to do so, you have to actually set the modifiers list to an empty list, which now removes the default modifiers.
- Use the attached datapack with an item modifier and run that modifier on the item in your hand. The toggle_tooltips function does hide the tooltips while keeping the modifiers, but the implicit modifiers for that item type are copied over to the stack itself, so future changes to the default of the stack won't affect this stack.
Incorrect Solutions
- You cannot just set an empty list of attributes because that overrides the implicit attributes (for non-armor items – see
MC-271826). Setting an empty list of attributes just removes the attributes- Copying over the default attributes makes the item not inherit changes to the default attributes made by the game in different versions. If all I want to do is hide the attributes and ensure the default attributes are still used, I can't copy them over
This should, therefore, be considered a regression in behavior as you could do this pre-data components.
Other data components suffer from the same issue, however none of the other data components that have included tooltip toggles have implicit values for an item type while the attributes one does.
Possible Solution
- Making the attribute modifier list an "optional" to signify that no modifiers have been set introduces a new state separate from an empty list. Setting an empty list still should mean you want no modifiers, but an empty optional means fallback to the possible implicit data components
- Could split out the hide tooltip into a separate data component.
Problem
Before the change over to data components, the HideFlags nbt could be set without having to set any data that would be hidden by the flag. However, now that the show_in_tooltip is part of the attributes data component (and a bunch of others), you have to actually set some attributes in order to change it to false.
Steps to Reproduce
/give @s wooden_pickaxe[minecraft:attribute_modifiers={show_in_tooltip:0b,modifiers:[]}]This command gives you a wooden pickaxe that hides the modifiers, but in order to do so, you have to actually set the modifiers list to an empty list, which now removes the default modifiers.
- Use the attached datapack with an item modifier and run that modifier on the item in your hand. The toggle_tooltips function does hide the tooltips while keeping the modifiers, but the implicit modifiers for that item type are copied over to the stack itself, so future changes to the default of the stack won't affect this stack.
Incorrect Solutions
- You cannot just set an empty list of attributes because that overrides the implicit attributes (for non-armor items – see
MC-271826). Setting an empty list of attributes just removes the attributes- Copying over the default attributes makes the item not inherit changes to the default attributes made by the game in different versions. If all I want to do is hide the attributes and ensure the default attributes are still used, I can't copy them over
This should, therefore, be considered a regression in behavior as you could do this pre-data components.
Other data components suffer from the same issue, however none of the other data components that have included tooltip toggles have implicit values for an item type while the attributes one does.
Possible Solution
- Making the attribute modifier list an "optional" to signify that no modifiers have been set introduces a new state separate from an empty list. Setting an empty list still should mean you want no modifiers, but an empty optional means fallback to the possible implicit data components
- Could split out the hide tooltip into a separate data component.
Expected Behavior
If I set the pickup status to DISALLOWED, it should stay that way despite hitting something which deflects the projectile. This was the behavior in <=1.20.4
Observed Behavior
If the summoned arrow, hits a player and deflects off, once it hits the ground, it's pickup status has been changed to ALLOWED
Steps to Reproduce
- Use the command and move out of the way of the arrow
/summon arrow ~ ~10 ~ {pickup:0b,Owner:[I; -2072785729, 1902920055, -1858696354, -1725573379]}- A player in survival cannot pick it up
- Run the command again (in creative mode) and let it hit you
rand then hit the ground- A player in survival can pick it up
Other Information
This is caused by the the logic which resets the owner of projectiles upon deflection. AbstractArrow overrides setOwner to change the pickup status, and Projectile#deflect calls setOwner.
Expected Behavior
If I set the pickup status to DISALLOWED, it should stay that way despite hitting something which deflects the projectile. This was the behavior in <=1.20.4
Observed Behavior
If the summoned arrow, hits a player and deflects off, once it hits the ground, it's pickup status has been changed to ALLOWED
Steps to Reproduce
- Use the command and move out of the way of the arrow
/summon arrow ~ ~10 ~ {pickup:0b,Owner:[I; -2072785729, 1902920055, -1858696354, -1725573379]}
- A player in survival cannot pick it up
- Run the command again (in creative mode) and let it hit you and then hit the ground
- A player in survival can pick it up
Other Information
This is caused by the the logic which resets the owner of projectiles upon deflection. AbstractArrow overrides setOwner to change the pickup status, and Projectile#deflect calls setOwner.
Can still reproduce this behavior on 1.18.1
Still an issue in 1.18.1
Drowned spawned just fine for me in water in a desert. They just require a water block below the block where they spawned. create a pool of water around the spawner and they'll spawn. I don't know if they should or not, there seems to be conflicting information on this tracker about that.
MC-175110suggests that its a bug if water mobs spawn outside of water, butMC-250054suggests its OK if water mobs spawn outside of water. I don't know what the correct answer is.@Adam, it looks like height requirements are WAI as suggested here in
MC-238526I think there are problems with always signing and including the original text in the message. What if a server wanted to hide information in that message? Like a bad word filter? The original message would still be sent to all the players. And what about the text-filtering-config system already in vanilla? That exists to filter words out of text, and so you wouldn’t want the exact original being sent to all clients, that completely defeats the purpose of it.
This issue also affects jukeboxes in 1.19.4.
Can confirm this is still an issue in 1.20-rc1
Still affects 1.20-rc1
This still happens in 1.20.1.
That is not a good fix. There are items that should be consumed and only have a stack size of 1, but are also not damageable. The correct fix is to change that conditional to instead check if them stack's item type has durability. If it does not have durability, consume the item, if it does, run the hurtAndBreak logic which has its own check for "Unbreakable". You can see how we fixed this issue on Paper.
To clarify (since the title could be misleading to anyone looking for the cause), the issue isn't caused by the "angle" argument being ignored, it's that in the constructor for the ServerPlayer object, the angle is overriden by the call to fudgeSpawnLocation which happens after the super constructor sets the spawn yaw to the yaw from the ServerLevel.
A fix could be to make the fudgeSpawnLocation method respect the level's yaw. Would solve the issue on initial spawn and respawn (without a "home" location).
I find it hard to believe its "works as intended" to have 2 starkly different behaviors for singleplayer and multiplayer. I don't see how that text bit in the minecraft changelog is at all indicative of this being WAI, just because they aren't converted to strings means that the behavior is different for singleplayer and multiplayer? How is that at all related.
Can confirm in 1.20.4.
This also affects Brewing Stands dropping items in the world. It seems to affect all uses of Containers#dropItemStack which happen while the block is still present in the world. All the other uses of it other than Campfire and Brewing Stand happen when the block is being broken.
Ok, I added clearer reproduction steps
Using the toggle_tooltips item modifier does succesfully hide the attribute modifiers (I've linked a datapack with such a function), but it still behaves as described above. It has to manually copy over the default values into the stack's data (use /data get entity @s SelectedItem to see the stack's components in NBT). This violates the 2nd of what I consider incorrect solutions because the itemstack no longer has the "default" attributes for a specific item type and any changes to those defaults would no longer be reflected in the itemstack.
You can just use /data get entity @s SelectedItem to view the data component structure serialized to NBT while holding each item and see the difference
Yes, that looks like my issue. Must've missed that when searching for an issue.
Still happens in 1.21.1