PaperMC / Paper

The most widely used, high performance Minecraft server that aims to fix gameplay and mechanics inconsistencies
https://papermc.io/
Other
9.99k stars 2.32k forks source link

Stacktrace on generating villager trades #11615

Closed LeafyMitsuwa closed 5 hours ago

LeafyMitsuwa commented 6 hours ago

Stack trace

https://paste.denizenscript.com/View/128151

Plugin and Datapack List

Citizens: 2.0.35-SNAPSHOT (build 3598), Denizen: 1.3.1-SNAPSHOT (build 7081-DEV), LuckPerms: 5.4.145, Sentinel: 2.9.1-SNAPSHOT (build 525), VoidWorld: 1.0, Depenizen: 2.1.1 (build 865), dDiscordBot: 0.7 (build 303)

Actions to reproduce (if known)

Attempted to generate a trade window with a single trade, Price: 2x Emerald (single item stack) Output: Sponge

Initially posted to the Denizen development team, which referred the issue to the Paper team.

To my recollection, last known good was on 1.20.4.

Paper version

[20:02:16] [Render thread/INFO]: [System] [CHAT] Checking version, please wait... [20:02:17] [Render thread/INFO]: [System] [CHAT] This server is running Paper version 1.21.1-131-ver/1.21.1@84281ce (2024-10-31T17:43:44Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)\nYou are running the latest version\nPrevious version: 1.21.1-89-1ed64f8 (MC: 1.21.1)

Other

The following comments, though not exclusively from the dev team, were added in the bug report; they may, or may not make sense.

"it seems like it's an intended behavior from paper for info : https://github.com/PaperMC/Paper/issues/11030"

"or, well, bug in the minds of the paper developers who can't imagine not wanting to preset a specific result in a constructor but maybe setting that later"

"Well, Minecraft doesn't let you send air items there I don't think - I guess it's technically possible to have one with an air item server-side, but it would error out as soon as it's sent to the client"

"pretty sure the recipe resultant from a constructor is basically always invalid, you have to follow it up with setingredients n wotnot before it has any coherent value as a trade recipe so why is a result item mandatory at time of the constructor call, when nothing else is? according to the people who designed that API, it's not according to paper, what if heehoo actually that value in particular is mandatory now"

electronicboy commented 5 hours ago

There is no method to mutate the result, and so permitting an illegal result to be supplied which would only cause an error down the line makes 0 sense;

if a builder is wanted, then that would be a separate issue, but to allow illegal arguments into the constructor which the item cannot be mutated later generally makes little sense, especially as we'd then need to try to capture such broken state in other places, potentially with less context and more headaches