#paper

1 messages ยท Page 14 of 1

rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Already commented on Discord, but I am very much against deprecating this, especially for removal.

Yes, this API can cause issues in some cases with cursor items, and yes it does not have well defined behaviour. Frankly it sucks. But its also the only option we got for this feature. Just adding a note should be enough, no reason to deprecate it.

Being able to change an inventory title is a very important feature for making GUIs. And inventories are currently the only way of making clickable GUIs in this game.
This is especially an important feature when creating GUIs using resource pack fonts, which use the inventory title to display a background. Removing this method (which a deprecation for removal would imply happens at some point) would make creating GUIs needlessly complicated, error-prone and annoying as you would need to re-create the entire view just to change the title, which makes managing GUIs in a plugin a lot more complicated than being able to hold on the the sa...

rustic folioBOT
#

After more internal discussion we decided it's alright as long as it's not deprecated for removal - show that the method has a pretty bad implementation, but acknowledge it's useful/doesn't have a replacement

A good "replacement" IMO is just to open a new inventory (with the new name) without closing the previous one. I guess it has the same or similar caveats as the methods, but even if those will be removed, this might be used.

#

Already commented on Discord, but I am very much against deprecating this, especially for removal.

Yes, this API can cause issues in some cases with cursor items, and yes it does not have well defined behaviour. Frankly it sucks. But its also the only option we got for this feature. Just adding a note should be enough, no reason to deprecate it.

Being able to change an inventory title is a very important feature for making GUIs. And inventories are currently the only way of making clickable GUIs in this game. This is especially an important feature when creating GUIs using resource pack fonts, which use the inventory title to display a background. Removing this method (which a deprecation for removal would imply happens at some point) would make creating GUIs needlessly complicated, error-prone and annoying as you would need to re-create the entire view just to change the title, which makes managing GUIs in a plugin a lot more complicated than being able to hold on t...

rustic folioBOT
#

Is your feature request related to a problem?

Let's say you want to serialize a Minecraft class to your disk or database. Maybe you're making a custom way to store a small world! (This is something I'm struggling with actually)
You go to serialize a BlockState and you realize you have to make your own format. You code a system (or use a library), you make a class to serialize and deserialize BlockState. It works, but it took some extra effort to get working when you just want to save data if you coded it yourself, or you had to get another dependency if you used a library.

Another plugin or developer comes in and wants to read that format or edit your plugin - they now have to learn how you serialized BlockState. Why go through all this trouble when it can be standardized?

This issue isn't exclusive to BlockStates - maybe you want to serialize a Component (granted, MiniMessage exists but that's to String, not json/nbt/etc.), a Location, PotionEffect etc.

D...

rustic folioBOT
#

I know one reason to not expose dfu to the API, is that DFU API is not stable and changes pretty frequently. That doesn't fit well in an API where lots of stuff has compatibility guarentees. That is slightly changing with the Registry Modification API however. It means there isn't as much of a difference to just doing what you suggested.

You could also just call down to NMS to do that for you, but that is not supported and there could be an API for it.

Using NMS to access the codecs if you want them comes with the implicit understanding that things change between versions. If its part of the API, its less clear.

rustic folioBOT
#

Expected behavior

Timestamps in a region file's header are unix seconds, stored as ints:

private static int getTimestamp() {
        return (int) (Util.getEpochMillis() / 1000L);
}

In the header recalculation patch, timestamps can be accidentally set to (int)System.currentTimeMillis() without dividing by 1000 first
https://github.com/PaperMC/Paper/blob/32711191cdad920b4818915b21699f9433800bf2/patches/server/1038-Attempt-to-recalculate-regionfile-header-if-it-is-co.patch#L414

Very ironic comment :>
// write a valid timestamp for valid chunks, I do not want to find out whatever dumb program actually checks this

Observed/Actual behavior

N/A

Steps/models to reproduce

N/A

Plugin and Datapack List

N/A

Paper version

Paper 1.21.1 32711191cdad920b4818915b21699f9433800bf2

Other

No response

rustic folioBOT
rustic folioBOT
rustic folioBOT
#

I agree with machine that the javadocs are very open to interpretation in regards to the returned set being mutable.
However, this API has existed for so long and is an upstream API as well, that we obviously cannot just return an unmodifiable
set.

I think we'd indeed be best of to wrap the returned set in a form of delegating wrapper.
We can already start potentially printing nags that this behaviour is strongly discouraged and "deprecated for removal".
Not that we can remove it until hardfork, but a heads up warning via nag may be a useful addition to danix's suggestion.

rustic folioBOT
#

Follow-up pr to #10703. Fixes an issue i discovered while discussing the pr. Since craftbukkit manually calculates the InventoryAction for the click events it doesn't take bundles into account. I am not sure whether PICKUP_ONE_INTO_BUNDLE and PLACE_ONE_INTO_BUNDLE are needed since there isn't a way to explicitly get 1 item out of a bundle like with right clicking a normal stack (or whether the new enums are needed at all)

rustic folioBOT
#

Is your feature request related to a problem?

It'd be a good thing to know which Entity joined first and which Entity joined last to form some sort of hierarchy in a Team.

Describe the solution you'd like.

Return a LinkedHashSet<String> instead of Set<String> such as one in Team#getEntries.

Describe alternatives you've considered.

Persist the datetime which Entity joined a Team. Having this feature implemented would remove requirement to persist such detail.

Other

No response

rustic folioBOT
#

Stack trace

This error happened at an ocean ruin, probably during/just after world gen, seems like the structure placed a chest over a brushable block.

[10:25:34] [Server thread/ERROR]: Failed to create block entity minecraft:brushable_block
java.lang.IllegalStateException: Invalid block entity minecraft:brushable_block // net.minecraft.world.level.block.entity.BrushableBlockEntity state at BlockPos{x=26055, y=43, z=-6938}, got Block{minecraft:chest}[facing=north,type=single,waterlogged=false]
	at net.minecraft.world.level.block.entity.BlockEntity.validateBlockState(BlockEntity.java:70) ~[paper-1.21.1.jar:1.21.1-DEV-b4bc512]
	at net.minecraft.world.level.block.entity.BlockEntity.<init>(BlockEntity.java:61) ~[paper-1.21.1.jar:1.21.1-DEV-b4bc512]
	at net.minecraft.world.level.block.entity.BrushableBlockEntity.<init>(BrushableBlockEntity.java:61) ~[paper-1.21.1.jar:1.21.1-DEV-b4bc512]
	at net.minecraft.world.level.block.entity.BlockEntityType.create(BlockEntityType.j...
rustic folioBOT
rustic folioBOT
#

Expected behavior

The entire world should be all stone.

Observed/Actual behavior

Only the center chunk (0, 0) was all stone. The other chunks that were generated did not seem like anything was changed through the custom chunk generator.

This image shows the result of using TestGen on a newly created world. To be clear, the ores, dirt, gravel, and water were not added in via the custom TestGen generator. But I believe that is working as intended because shouldGenerateDecorations is set to true so it adds vanilla's decorations after all the main generation was complete.
image

This image shows the result when I edited the TestGen code to set diamond blocks instead of stone and all of the "should..." methods were set to false.
image

Steps/models to reproduce

I created the world like so:
`new Worl...

rustic folioBOT
rustic folioBOT
rustic folioBOT
#

I am also having this issue. Trying to reproduce by playing without plugins is almost infeasible since this has happened 10 times over a week with 10-20 players online (would likely have to play alone for 100+ hours to be sure).

My plugins:

[12:25:43 INFO]: [PluginInitializerManager] Paper plugins (2):
 - AdvancedServerList (4.12.1), EconomyShopGUI-Premium (5.14.2)
[12:25:43 INFO]: [PluginInitializerManager] Bukkit plugins (29):
 - AuraSkills (2.2.1), BetterRTP (3.6.13), BlueSlimeCore (2.9.6.431), CMILib (1.5.0.9), Chunky (1.4.10), CoinsEngine (2.3.4), CombatLogX (11.5.0.0.1242), DecentHolograms (2.8.9), Essentials (2.21.0-dev+106-8b08a8f), FarmControl (1.3.0), FastAsyncWorldEdit (2.11.1-SNAPSHOT-871;fcbd346), GravesX (4.9.3.9), GriefPrevention (16.18.4), LuckPerms (5.4.137), MobMoney (5.1.8), Multiverse-Core (4.3.12), Multiverse-Portals (4.2.3), PlaceholderAPI (2.11.6), PlayerBiomes (6.0.0), ProtocolLib (5.3.0-SNAPSHOT-726), SignShop (4.0.0), SimpleClans (2.19.2-05e7abc...
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

there is a small change in the experiment where right and left click were exchanged which here could just be fixed by making a change to an if statement. With my comment above i meant bigger changes for which we would need to delete an enum or move big portions of code around. We can ofc also wait for what mojang actually decides to do with bundles in 1.21.2 and decide what we do from there

rustic folioBOT
#

Expected behavior

When you change the seed for portals or villages in spigot.yml, you shouldn't be able to find villages with chunk base and the world seed

Observed/Actual behavior

Changing seed-village etc in spigot.yml has 0 effect

Steps/models to reproduce

make a world with a 32 bit seed (i dont think the issue is because the world has a 32 bit seed, but just to be sure)
set seed-village in spigot.yml to anything you like
load the world on chunkbase and search for villages, teleporting to them will always have a village.

Plugin and Datapack List

N/A

Paper version

This server is running Paper version 1.21.1-DEV-master@b4bc512 (2024-08-18T22:23:43Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are 8 version(s) behind
Download the new version at: https://papermc.io/downloads/paper
Previous version: 1.21.1-DEV-52ae4ad (MC: 1.21.1)

Other

No response

rustic folioBOT
#

Do we know if this issue is something that also affects builds prior to 1.21.X?
I'm hosting a v1.20.6 server for friends, and right now I don't want to tackle the process of updating many plugins and also looking for suitable replacements for a few of them.
So if this affects previous versions (well, mainly 1.20.6 is what I'm concerned of) too, I'll just fork and backport this change for the time being, until I have the time and mental capacity to spend hours updating the server xd

rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#
[PaperMC/Paper] New branch created: upstream-1.20.6
#
[PaperMC/Paper] branch deleted: upstream-1.20.6
rustic folioBOT
#

Is your feature request related to a problem?

currently this flag only support single file per instance
it would be a nice addition to be able to pass to this flag entire folder for easier managing for network of servers

Describe the solution you'd like.

add a seperate flag for providing entire folder of plugins to the server for example something like -add-plugin-folder="<path>"
server would filter for only jar files that would be loaded when booting up the server

Describe alternatives you've considered.

adding individually all plugins that I want server to have

Other

example setup
list of servers:

  • server1
  • server2
  • server3

have some kind of shared folder with subfolders

  • shared // main shared folder that user mounted to all servers
    • allservers // first sub folder that would be added to all servers with the flag
    • server1 // would only be added to server1 along side allservers
    • etc..
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Yes, I too saw that possible NPE when reviewing the code... I think, instead, this should be a reset method that resets both things. Technically it's actually incorrect for "tracked" to be a TriState, as the value is a primitive boolean. But there does need to be a 3rd state for "no lodestone component at all". I think a reset location and a clear lodestone (that clears both the tracked and position) is the correct solution.

rustic folioBOT
#

I think a reset location and a clear lodestone (that clears both the tracked and position) is the correct solution.

I mean, according the the Javadocs, the location can be cleared by passing null into the CompassMeta#setLodestone(Location) method.
As for the tri state, the tracked boolean is responsible for deciding whether the location is stored in the component. If we say we convert the tracked Boolean into a primitive, in the method I am adding here we could simply set the tracked to false and the location to null which is what you're saying I guess.

rustic folioBOT
rustic folioBOT
#

Is your feature request related to a problem?

The attack cooldown currently gets reset to 0.0 when a player attacks or switches held materials by scrolling/hotkey. I believe the latter is punishing the player for using items other than their weapon so I want to change this for my server.

Describe the solution you'd like.

I would like a cancellable event that gets fired every time a player's attack cooldown resets to 0.0. Preferably with the cause of the event.

Example use:

public void onAttackCooldown(PlayerAttackCooldownResetEvent event) {
Player player = event.getPlayer();
if (event.getCause() == PlayerAttackCooldownResetEvent.Cause.SWITCH_MATERIAL || player.isOp()) { 
event.setCancelled(true); 
   }
}```

### Describe alternatives you've considered.

I tried using Player#resetCooldown() but this doesn't actually reset the player's attack indicator, and even if it did it wouldn't help me because I want the cooldown to continue where it left so...
rustic folioBOT
rustic folioBOT
#

Stack trace

error log

[22:40:52 WARN]: **** FAILED TO BIND TO PORT!
[22:40:52 WARN]: The exception was: io.netty.channel.unix.Errors$NativeIoException: bind(..) failed: Address already in use
[22:40:52 WARN]: Perhaps a server is already running on that port?
[22:40:52 ERROR]: Encountered an unexpected exception
java.lang.IllegalStateException: Failed to bind to port
        at net.minecraft.server.dedicated.DedicatedServer.initServer(DedicatedServer.java:286) ~[paper-1.21.1.jar:1.21.1-41-fcedb49]
        at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1215) ~[paper-1.21.1.jar:1.21.1-41-fcedb49]
        at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:330) ~[paper-1.21.1.jar:1.21.1-41-fcedb49]
        at java.base/java.lang.Thread.run(Thread.java:1583) ~[?:?]
Caused by: io.netty.channel.unix.Errors$NativeIoException: bind(..) failed: Address already in use
[22:40:52 ERROR]: This crash report has been saved to: /wor...
rustic folioBOT
#

Passing a non-item Material to a MaterialChoice results in an obscure and hard-to-debug error in the depths of the Minecraft codecs.

<details>
<summary>Example of error that doesn't offer any help with identifiying what recipe has a non-item</summary>



        [20:17:16 ERROR]: Error sending packet clientbound/minecraft:update_recipes (skippable? false)
        io.netty.handler.codec.EncoderException: Failed to encode packet 'clientbound/minecraft:update_recipes'
                at net.minecraft.network.codec.IdDispatchCodec.encode(IdDispatchCodec.java:53) ~[folia-1.20.6.jar:1.20.6-DEV-d797082]
                at net.minecraft.network.codec.IdDispatchCodec.encode(IdDispatchCodec.java:20) ~[folia-1.20.6.jar:1.20.6-DEV-d797082]
                at net.minecraft.network.PacketEncoder.encode(PacketEncoder.java:26) ~[folia-1.20.6.jar:1.20.6-DEV-d797082]
                at net.minecraft.network.PacketEncoder.encode(PacketEncoder.java:12) ~[folia-1.20.6.jar:1.20.6-DEV-d7...
rustic folioBOT
rustic folioBOT
#

Is your feature request related to a problem?

I'm thinking it might be nice idea to add PreInventoryMoveItemEvent, which would be fired before source and target inventories are checked, so at this point server doesn't even know if it should move item, but it does know it will have to try to.

This would allow protection plugins to cancel such events before expensive itemstack checks are done, and it would also allow plugins that modify say hopper behavior to do the same and skip whole builtin logic.

Furthermore, source inventory passed in InventoryMoveItemEvent has first amount set to 1 instead of actual value, which further limits plugins.

Describe the solution you'd like.

Add PreInventoryMoveItemEvent which is fired before any checks are done on from/to inventories.

Describe alternatives you've considered.

Running delayed task on certain InventoryMoveItemEvent events, but I feel like that's not too great solution as processing is actually delayed by 1 tick, a...

#

From my personal observation, it was never the checks being expensive, 90% of the time it is the protection plugins being extremely bad which causes lags, because there are often hundreds or thousands of hoppers transferring items each tick, and protection plugins just can't properly check all of those that fast so after 1000 checks in a tick they would still be in reasonable <1 ms

#

Depends on amount of hoppers?

First, first slot is pulled, that item stack would have to be matched against every itemstack in target? Then same for 2nd slot, 3rd slot, etc... having say, 100 players, 50k hoppers across a few loaded chunks, think it would add quickly, but you're right I should check spark reports.

rustic folioBOT
#

the source stack being sized 1 is due to the patch attempting to avoid cloning due to how expensive it is, for the root tree this is now much cheaper, the only real issue is the handling of the PDC is potentially going to cause issues here if we wanted to restore some of the cloning back now that it's supposed to be cheaper.

Adding more events inside of this tightly defined loop is fairly meh, however.

rustic folioBOT
rustic folioBOT
#

This is a distance check relative to the edge of border.
Without that spider cannot climb any block outside the border when the config is disabled. I think the config should only apply to the edge. isCollidingWithBorder check if the mob is colliding with the border voxel shape so not only the edge. You can see similar things in CollisionUtil#getCollisionsForBlocksOrWorldBorder L1586.

#

Is your feature request related to a problem?

I want to be able to update spark as needed instead of relying on a paper update.

Describe the solution you'd like.

A way to disable the spark integration and use a standard spark jar file.

Describe alternatives you've considered.

None because paper ignores the spark file in /plugins/

Other

No response

rustic folioBOT
#

Expected behavior

Edit config\paper-world-defaults.yml
Change flush-regions-on-save: from "false" to "true", restart server.
I expect to see regions with changed blocks to be written to disk when the autosave (default every 5 mins) happens.

Observed/Actual behavior

Region files are not updated on the hard drive. The timestamp doesn't alter. When using Bluemap it never decides to trigger an update. Extra info: A server command "save-all" does not save regions. "save-all flush" does save to disk (and then Bluemap sees the change).

Steps/models to reproduce

I'm using Windows 10 Pro 22H2, and Amazon Coretto - "java -version" gives "openjdk version "21.0.4" 2024-07-16 LTS".

  1. Start a brand new, default paper server, no flags, no plugins, accept EULA:
    java.exe -jar paper-1.21.1-45.jar

  2. Connect a client, break some blocks, disconnect.

  3. Stop the server to force regions to be written to disk.

  4. Edit config\paper-world-defaults.yml: Change flush-regions-on...

#

I have same issue,my world seed is Seed: [4582130043017388758]
and here is my logs

[00:32:37] [Server thread/ERROR]: Failed to create block entity minecraft:sculk_sensor
java.lang.IllegalStateException: Invalid block entity minecraft:sculk_sensor // net.minecraft.world.level.block.entity.SculkSensorBlockEntity state at BlockPos{x=1645, y=-47, z=-5927}, got Block{minecraft:sculk_catalyst}[bloom=false]
	at net.minecraft.world.level.block.entity.BlockEntity.validateBlockState(BlockEntity.java:70) ~[paper-1.21.1.jar:1.21.1-40-2fdb2e9]
	at net.minecraft.world.level.block.entity.BlockEntity.<init>(BlockEntity.java:61) ~[paper-1.21.1.jar:1.21.1-40-2fdb2e9]
	at net.minecraft.world.level.block.entity.SculkSensorBlockEntity.<init>(SculkSensorBlockEntity.java:32) ~[paper-1.21.1.jar:1.21.1-40-2fdb2e9]
	at net.minecraft.world.level.block.entity.SculkSensorBlockEntity.<init>(SculkSensorBlockEntity.java:38) ~[paper-1.21.1.jar:1.21.1-40-2fdb2e9]
	at net.minecraft.world.level.bloc...
rustic folioBOT
#

So I checked the default installation of Lootin, as that was the plugin that exposed a similar bug in the past, but that didn't cause this error when I generated the world and teleported to that location. Can you perform a binary search of your plugins, on a complete copy of your server/world? Essentially copy your server, delete the world (cause it has to "generate" to cause this issue). Then remove half your plugins, generate world, see if that exception happens. If it does, you know the issue is in that half of the plugins, otherwise its in the other half. adjust your installed plugins, delete the world and check again.

#

Thanks @electronicboy Are you suggesting I can work around the issue by adding a -DPaper.enable-sync-chunk-writes=true flag to my batch file (I tried it, didn't work)? Also, as this is a documented config file entry that doesn't work (and you closed this as 'not planned' to fix), could it be mentioned somewhere in the docs, or just removed from the config file altogether?

Is there another method to force regions to be regularly written to disk, for those worried about server crashes for whatever reason? This (not working) config item is the only one I could find. Appreciate your time.

rustic folioBOT
#

Expected behavior

I expect that when a Player triggers EntityDamageEvent & EntityDamageByEntityEvent through a melee attack that player.getEquipment().getItemInMainHand() or player.getInventory().getItemInMainHand() will return the weapon used to damage the entity.

Observed/Actual behavior

When attacking immediately after switching slots the new slot will be the mainhand, while the entity was hit with item in the previous slot. This behaviour can be verified by comparing the damage values with the weapon type in the video below.

With some practise this can be easily abused by switching to a slow weapon that triggers an ability within this event. Basically ignoring the slower attackspeed and still triggering the ability.

Steps/models to reproduce

https://github.com/user-attachments/assets/70f90bdc-f093-47dc-b1dd-e719b02f8294

Plugin and Datapack List

image
!...

rustic folioBOT
rustic folioBOT
#

I guess you're right on performance end of things, as well as firing too many events in such a short loop.

But, in terms of plugins/API handling hoppers it would still be an issue? And looking at the code there's not many we could change InventoryMoveItemEvent to mitigate issue of setting slot amount to 1? Not without breaking other stuff and existing plugins?

Maybe the solution would be to extend Hopper api instead? And make it so that each hopper has own transfer amount that could be modified? Not quite sure how doable that would be tho. It also might break existing plugins that just assume amount would always be 1? Then again amount wouldn't be 1 it would always be whatever is set in spigot.yml config file.

I actually went explore this trying to implement own Hopper plugin but ran into issues were amount in source inventory not being the actual amount.

rustic folioBOT
#

The amount thing was down to aikar attempting to avoid cloning due to highly nested NBT structures, I don't think that this is a real issue anymore given mojang rewriting itemstacks, the only caveat would be the compound tag data component, but, that is generally not going to be populated to the degree that was causing issues in the past.

rustic folioBOT
rustic folioBOT
#

So I checked the default installation of Lootin, as that was the plugin that exposed a similar bug in the past, but that didn't cause this error when I generated the world and teleported to that location. Can you perform a binary search of your plugins, on a complete copy of your server/world? Essentially copy your server, delete the world (cause it has to "generate" to cause this issue). Then remove half your plugins, generate world, see if that exception happens. If it does, you know the issue is in that half of the plugins, otherwise its in the other half. adjust your installed plugins, delete the world and check again.

This I don't think is plugin-related, as when it happened to me I didn't have any plugins

rustic folioBOT
rustic folioBOT
#

If a plugin sets the health of a living entity above 0 after it has already died, the entity will be "revived".
It will behave the exact same as before, except with the internal "dead" flag set, resulting in 2 behavior changes,
A: it's completely invulnerable to all damage
B: it's unable to pickup items

isValid() for these bugged entities will return true, isDead() will return false, despite the dead flag.
This patch checks that the mob isn't dead before saying its alive.

Also, even if the plugin is responsibly checking !isDead() before modifying health, on very rare circumstances
I am currently unable to replicate, these "revived" entities can still appear

rustic folioBOT
rustic folioBOT
#

In the case where multiple messages from different players are being processed in parallel, there was a potential race condition where the messages would be sent to the client in a different order than the message signature cache was updated. However, the cache relies on the fact that the client and server get the exact same updates in the same order. This race condition would cause the caches to become corrupted, and any future message received by the client would fail to validate.

This also applies to the last seen state of the server, which becomes inconsistent in the same way as the message signature cache and would cause any messages sent to be rejected by the server too.

This can be easily reproduced by having two players very rapidly send chat messages to the server at the same time.

rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#
[PaperMC/Paper] branch deleted: policy
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Welcome to the rabbit hole that is capturing every single case where client
predictability causes a desync between the client and the servers state.
The general understanding that was made when we accepted Thais as a feature
is that we would not be chasing all of these.

On Tue, 27 Aug 2024 at 19:12, KillerCreeper @.***>
wrote:

There's currently no way for a plugin to reliably fix this though. The
point of the disable-noteblock-updates setting is so plugins don't need to
listen to physics events, right? Yet, to implement a decent fix for this, a
plugin would need to use listen to physic events anyway.

โ€”
Reply to this email directly, view it on GitHub
https://github.com/PaperMC/Paper/issues/9496#issuecomment-2313214172,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAJMAZEBBPFN3FOB5MHTLO3ZTS6RTAVCNFSM6AAAAAA2LHCPDGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMJTGIYTIMJXGI
.
You are receiving this because you comme...

rustic folioBOT
#

I think we probably could add an interface with both serializeToBytes and serializeToJson methods. Then that interface would mark types that would have static deserializeFromBytes deserializeFromJson methods. The implementations of such methods would use the codecs and wrap each type to include the data version to upgrade data on deserialization. I know the main issue people have with human-readable serialization formats w/the data version is: Why is it human readable if I can't just make changes to it? The changes I make have to line up with the data version which might be years out of date.

#

I would just "wrap" a bukkit team in your own type that tracked that information. The order of entries is not defined. So what I'd do is save your "teams" somewhere, load them and create a bukkit team from it, but then wrap it in your own type that tracks that information and only make changes through that wrapper type. Then you can save that back to whatever storage you want.

rustic folioBOT
#

@masmc05 I think that the "human-readable"-ness of this is really not that useful, especially when this would be a data-versioned format. Say you serialize an ItemStack on this version and it's got whatever version, say 1000. Then, in 3 years you go to deserialize it, but you want to change it first. 3 years from now, the format for itemstacks may have changed drastically, but you have to go back and know exactly what the format is for your specific data version. That information isn't readily available. I think it being human readable is really not worth much. Do you have a specific reason why human readable would be good?

rustic folioBOT
#

Well, the possibility to change itemstacks in configs is often useful, starting with the possibility to be able to manually write them without the usage of a command, GUI etc so to save an item from the game. This helps in developing small plugins where those items won't be changed often. As I said here the IDE's help with the syntax will be very useful for writing those. And this usefulness goes up to the fact that it's easier to batch change the items in configs, so you could make a change up to a hundred of items at the same time, which wouldn't be possible with byte format

Also thought about supporting items without data version, assuming the newest version, but that could lead to not the best practices. Since it's just json, a data version can be added by the plugin, but then it could understand that the item will need to be resaved so it future it could be converted, while otherwise this may have been ignored very often, resulting in future mess

rustic folioBOT
#

I do wish that there was a "human readable" standard agreed upon, but, json tangengially datapack related stuff is IMHO, not it. I do understand the arguments for having it in favour of people who want to bite that bullet to deal with, but, as said, it's a raw low level type of deal which can/will change often without warning, datapacks themselves already break once or twice a year as-is.

snbt based serialisation in configs is one of those areas of, yea, sure, you can tweak SNBT, but it's still a fairly low-level representation of the data stored.

There is never going to be a real good, long term solution for dealing with representing consistently evolving data. you could agree on the forefront, but, that would be library material, not built-in API type of stuff.

rustic folioBOT
#

I do understand the arguments for having it in favour of people who want to bite that bullet to deal with

Yeah, exactly, that's why the scope of this pr is not to change something existing, people using this will have to understand that it's version dependent format, not a great solution for a big plugin like DeluxeMenu, but still great for some plugins where it's not worth having a whole library to deal with this. This is not a universal solution for each use case

datapacks themselves already break once or twice a year as-is.

This format is based more on codecs than datapacks, just mojang expanded the capabilities of that codec for datapacks, which this format uses. Since how universally codecs are used for both json and nbt in Minecraft, I don't think it's going to be breaking soon. And usually when Mojang makes breaking changes for datapacks, it's rare for it to be more restricting without a new alternative, just may be something different, meaning that the possibili...

rustic folioBOT
rustic folioBOT
#

Stack trace

---- Minecraft Crash Report ----
// Would you like a cupcake?

Time: 2024-08-28 11:17:02
Description: Exception in server tick loop

org.spongepowered.configurate.loader.ParsingException: []: Unknown error occurred while loading
	at org.spongepowered.configurate.loader.AbstractConfigurationLoader.load(AbstractConfigurationLoader.java:165)
	at org.spongepowered.configurate.loader.AbstractConfigurationLoader.load(AbstractConfigurationLoader.java:63)
	at org.spongepowered.configurate.loader.ConfigurationLoader.load(ConfigurationLoader.java:56)
	at io.papermc.paper.configuration.Configurations.initializeWorldDefaultsConfiguration(Configurations.java:171)
	at net.minecraft.server.dedicated.DedicatedServer.initServer(DedicatedServer.java:227)
	at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1215)
	at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:330)
	at java.base/java.lang.Thread.run(Thread.java:1583)
C...
rustic folioBOT
#

I am also sadly not that big of a fan of JSON for this.
The two arguments your initial comment pointed out were that
a) it makes it easy to convert the output into other formats, like yaml
b) it has better editor support

Argument b imo is rather irrelevant for a PR like this, especially when editing the output is something, as we got to above, is only for those that want to bite the bullet anyway. If the "normal" usecase for this is read-only, SNBT should be as easily "read" by users as json to understand if the item stored somewhere is the right one.

Argument a makes a bit more sense to me, json is obviously a lot more wide spread than SNBT and hence tools like jq or whatever else can parse the output, whereas snbt presumably lacks these tools. How much this helps anyone tho is kinda up in the air because, again, the internal json structure may change from one version to the next with 0 warning and 0 ability of ours to preserve anything. Which then means that e...

rustic folioBOT
#

From searching code across GitHub, the primary use case for ChunkUnloadEvent.setSaveChunk(false) seems to be preventing certain worlds, like minigame worlds, from being saved.

In most cases, if you're not saving a chunk, it also makes sense not to save the entities within that chunk.
Before version 1.17, this was the default behavior, as entities were saved within chunks.

However, in the current system, even when chunk saving is canceled, entities like shot arrows or unexploded TNT are still saved, which can cause leftover items in minigame worlds.

rustic folioBOT
rustic folioBOT
#

I don't have a strong opinion on JSON or SNBT, both seem fine to me. I generally believe such a system would be very useful.

But I don't really think the format changing between versions is as much of an issue. One of the main use cases of such a PR, at least for me, would be purely reading items from configs. Being able to write an item definition inside a config is preferable over long and unwieldy commands to create such items in-game in my opinion, those get really confusing and too long to enter in the chat box very fast. It also allows for easier version control than base64 encoded strings from serialized in-game items.
The Bukkit format sucks, and doesn't nicely represent the game anymore. So having some replacement, be it SNBT or JSON, for this usecase would be nice.

And versioning really doesn't matter here that much - it is kinda expected that if Minecraft changes something my items use, I will have to update my config file with those items. That's the same thing th...

rustic folioBOT
#

a lot more stable than passing a whole json string of undefined format

I think we went into a bit of miscommunication here, since I didn't provide any example usage of this, using this api to create json strings to write those to the config is indeed dumb and a lot worse than the snbt strings, so I think first of all I should provide the example before saying something else, ofc this api still can be used in many other (and most probably better) ways

This is a quickly written small plugin fragment which just adds an extremely simple menu item, it takes advantage of the "which also may be transformed to many other formats like yaml", here is the full class

    private final Gson gson = new GsonBuilder()
        .disableHtmlEscaping()
        .setObjectToNumberStrategy(ToNumberPolicy.LONG_OR_DOUBLE) //Not necessary, but more pretty output
        .registerTypeHierarchyAdapter(ItemStack.class, ItemStackAdapter.itemStackAdapter()...
rustic folioBOT
#

Argument a makes a bit more sense to me, json is obviously a lot more wide spread than SNBT and hence tools like jq or whatever else can parse the output

The message of the argument wasn't this, I didn't mean it can be interacted with more easily by external tool, as you mentioned, it's unstable data, this will for sure become a nightmare. I indeed meant that the relaxed data types of json (only string, number, object, array, boolean, null) means that no matter to which format it will be transformed (yaml, toml etc) no data loss will occur, so this can be easily used in any kind of config, using the proper syntax and IDE support, this misunderstanding may have lead to considering argument b useless, because yes, a simple json string in a random config yaml config wouldn't bring better IDE support

rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Expected behavior

When obfuscation is on, it should block cheats to be able to look at other player items meta, durability etc.

Observed/Actual behavior

Currently with items obfuscation set to true, cheats do allow to see items durability and itemmeta in other players inventories.

Steps/models to reproduce

  1. In _paper-world-defaults.yml set:
    obfuscation: items: hide-durability: true hide-itemmeta: true hide-itemmeta-with-visual-effects: true
  2. Startup server
  3. Using cheats see other players inventories. You'd be avaliable

Plugin and Datapack List

No plugins

Paper version

This server is running Paper version 1.21.1-52-master@e08e667 (2024-08-26T18:02:06Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are running the latest version

Other

When testing on 1.20.4 this issue wasn't present. I wasn't able to see players inventory meta on paper 1.20.4

rustic folioBOT
#

Expected behavior

When using a grindstone to remove enchantments from an item, it should fire BlockExpEvent as the grindstone drops experience orbs.

Observed/Actual behavior

When using a grindstone to remove enchantments from an item, the server does not fire BlockExpEvent.

Steps/models to reproduce

  1. Code a test plugin that listens for BlockExpEvent
@EventHandler
public void onBlockExp(BlockExpEvent event) {
    BlockPosition pos = event.getBlock().getLocation().toBlock();
    System.out.println("Event at " + pos);
}
  1. Join the server and use a grindstone
  2. See that the test code is not triggered

Plugin and Datapack List

  • (Test plugin to reproduce the bug)

Paper version

This server is running Paper version 1.21.1-52-master@e08e667 (2024-08-26T18:02:06Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are running the latest version

Other

No response

rustic folioBOT
rustic folioBOT
#

Expected behavior

Breaking a single crop will make the statistic for global block increment by one.

Observed/Actual behavior

When breaking crops, the global value statistic increments by 2-4 times every time they're broken but the material-unique statistic increments only once.

Netherwart = 3
Carrot = 2
Wheat = 3
Beet = 2
Potato = 2

VIsual Example

Steps/models to reproduce

Break crops in survival mode, watch the global statistic increase several times at once

Plugin and Datapack List

PlaceholderAPI, ViaBackwards, ViaVersion

PlaceholderAPI is used to display the statistic easily and viaversion/viabackwards because i was on 1.20.4 client, although still it still persists without them.

Paper version

version
[16:27:49 INFO]: Checking version, please wait...
[16:27:50 INFO]: This server is running Paper version 1.21.1-52-master@e08e667 (2024-08-26T18:02:06Z) (Implementing API versi...

rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

It's specifically using Player#getStatistic(Statistic.MINE_BLOCK), Which should result in the total blocks mined by the player or atleast thats my understanding of it.

public class BlockListener implements Listener {

    @EventHandler(ignoreCancelled = true)
    public void onBreak(BlockBreakEvent event) {
        Player player = event.getPlayer();
        int totalBroken = player.getStatistic(Statistic.MINE_BLOCK);
        int brokenSpecific = player.getStatistic(Statistic.MINE_BLOCK, event.getBlock().getType());

        player.sendMessage(Component.text("Total Blocks Broken: x" + totalBroken));
        player.sendMessage(Component.text("Broken " + event.getBlock().getType().name() + ": x" + brokenSpecific));
    }

}

rustic folioBOT
#

If setting the delay to 0 leads to an exception somewhere (idk if this happens for sure, just assuming), then either that exception should be fixed, or if that isn't feasible, 0 just shouldn't be allowed as an argument value.

It is not allowed precisely. But on the entity scheduler, we can set it to 0 BUT it is corrected by a Math.max so that the lowest value is 1 (see here: https://github.com/PaperMC/Paper/blob/25621248d3f30f35c3458b4b13a2474ffd0bee0e/patches/server/0848-Folia-scheduler-and-owned-region-API.patch#L178 )

rustic folioBOT
#

Stack trace

[paste your stack trace or a paste.gg link here!](https://paste.gg/p/anonymous/0f3dfd9ca8ab46a4b9f4adce840847b5)

Plugin and Datapack List

X

Actions to reproduce (if known)

No response

Paper version

[04:19:34 INFO]: Checking version, please wait...
[04:19:35 INFO]: Current: git-Purpur-1985 (MC: 1.19.4)*
Previous: git-Paper-550 (MC: 1.19.4)

  • You are running the latest version

Other

No response

rustic folioBOT
rustic folioBOT
#

Stack trace

<html>
<body>
<div class="wrapper" style="box-sizing: inherit; flex: 1 1 0%; color: rgb(255, 255, 255); font-family: sans-serif; font-size: 16px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: normal; background-color: rgb(31, 36, 36); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;"><nav class="navbar" style="box-sizing: inherit; background-color: rgb(55, 90, 127); min-height: 4rem; position: relative; z-index: 30; align-items: stretch; display: flex; border-radius: unset;"><div class="navbar-brand" style="box-sizing: inherit; align-items: stretch; display: flex; flex-shrink: 0; min-height: 4rem;"><a class="navbar-item" href="https://paste.gg/" style="box-sizing: inherit; color: rgb(255, 255...
rustic folioBOT
rustic folioBOT
#

Is your feature request related to a problem?

CommandSourceStack has CommandSender and Executor but when you want to get the last sender you have to get it yourself

Describe the solution you'd like.

I would like to have a method that returns the last sender

Describe alternatives you've considered.

Nothing

Other

At the moment I'm using this

public void execute(@NotNull CommandSourceStack commandSourceStack, @NotNull String[] args) {
    CommandSender sender = commandSourceStack.getSender();
    if (commandSourceStack.getExecutor()!=null) sender = commandSourceStack.getExecutor();
    /* code */
}
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Expected behavior

When changing the value of "perform-username-validation" from true to false, I expected to see that when I try to log in with a Cyrillic nickname, I will successfully log in to the server and be able to play.

Observed/Actual behavior

When I tried to log in, I saw the message "Failed to verify username!". I was not allowed to access the server, and I saw warnings in the console.

`[13:38:07 INFO]: Disconnecting ะœะธั€ัะตะฒ (/95.131.78.56:55886): Failed to verify username!
[13:38:07 WARN]: Exception verifying ะœะธั€ัะตะฒ
java.lang.IllegalArgumentException: The name of the profile contains invalid characters: ะœะธั€ัะตะฒ
at com.google.common.base.Preconditions.checkArgument(Preconditions.java:218) ~[guava-32.1.2-jre.jar:?]
at com.destroystokyo.paper.profile.CraftPlayerProfile.createAuthLibProfile(CraftPlayerProfile.java:274) ~[paper-1.20.6.jar:1.20.6-149-6e71f41]
at com.destroystokyo.paper.profile.CraftPlayerProfile.<init>(CraftPlayerProfile...

rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Expected behavior

When iterating ofer the inventory you get from the InventoryMoveItemEvent, should get the correct stack sizes from the source inventory, e.g. the hopper inventory.

Observed/Actual behavior

The first slot of the the hopper (source inventory) shows always the stack size of 1, even though it is not 1.

Steps/models to reproduce

Place a hopper and fill it with 5*64 Items. Place a chest below. The source-Inventory always has the stack size 1 in Slot 0.

Plugin and Datapack List

Paper version

version
[21:37:44 INFO]: Checking version, please wait...
[21:37:44 INFO]: This server is running Paper version 1.21-130-master@b1b5d4c (2024-08-10T10:02:42Z) (Implementing API version 1.21-R0.1-SNAPSHOT)
You are running the latest version
Previous version: 1.21-119-100d75a (MC: 1.21)

Other

No response

rustic folioBOT
rustic folioBOT
#

Maybe the getExecutor method could be renamed to getProxyEntity/getTargetEntity, with javadocs that say something like, The entity targeted by this command, which can be changed using "/execute as <entity> run <command>". If execute as was not used to target an entity, getProxyEntity would default to the Player who sent the command, or null if the console or a command block ran the command. There could also be a boolean isProxySender() method like Ender-Fake mentioned to check if the target entity was changed using execute as.

That might help make it clear that getSender should be used for checking permissions, while getProxyEntity should be used for performing actions if that is what the plugin wants to do.

rustic folioBOT
#

Is your feature request related to a problem?

I want to make a mob follow a custom path instead of having minecraft calculate one for me.

Describe the solution you'd like.

I would like Paper API to create my own PathResult object.

public void followPath(Pathfinder pathfinder, List<Location> points) {
        Pathfinder.PathResult pathResult = new Pathfinder.PathResult(points);
        pathfinder.moveTo(pathResult);
    }

### Describe alternatives you've considered.

I have tried this, but this calls the findPath method for every iteration which I assume is quite expensive. And this also requires me to basically rewrite the entire system to make sure the mob only moves to the next point when it reaches the previous point

```java
public void followPath(Pathfinder pathfinder, List<Location> points) {
        for (Location location : points) {
            pathfinder.moveTo(location, 1.0);
        }
    }
    
    default boolean moveTo(@NotNull Locat...
rustic folioBOT
#

Expected behavior

I expected using the player.spigot().respawn() method to not throw a NPE regardless, and respawn the player regardless of their state. It seems that the DimensionTransition stuff is relatively new and probably not handled correctly somehow.

Observed/Actual behavior

Instead, the server threw a NPE (https://paste.helpch.at/iritoqudur.css), complaining the following:
java.lang.NullPointerException: Cannot invoke "net.minecraft.world.level.portal.DimensionTransition.pos()" because "dimensiontransition" is null

Steps/models to reproduce

  1. Register an event listener for PlayerQuitEvent. In that event, kill the player by setting their health to 0f or player.setHealth(0f)
  2. Register an event listener for EntityDeathEvent. In that event, check that the Entity is a Player and then use player.spigot().respawn() to attempt to respawn the player.
  3. In-game, log out and check the error in console.

Plugin and Datapack List

WorldEdit,...

rustic folioBOT
#

But if the player "quit" you can respawn? i mean the respawn can handle a check for online for avoid the NPE but not sure if use respawn in the context of player die in quit is valid too

I'm not sure, that's subjective. But imo it shouldn't throw an exception either way. I fixed it easily by marking the player when they log out and making sure to not call player.spigot.respawn() as the log out, but if another plugin (let's say some combat plugin) doesn't check this, it could run into this NPE. Also, this NPE is related to DimensionTransition, which is relatively new and I don't think this bug was there in previous versions. Thus, that's why I think it should be fixed (maybe like a simple null check for ex).

rustic folioBOT
rustic folioBOT
#

Expected behavior

This command in a command block in 1.20.4:
`/setblock -39 30 707 minecraft:hopper[enabled=true,facing=east]{Items:[{Count:41b,Slot:0b,id:"minecraft:blue_dye",tag:{display:{Lore:['[{"text":"- gษพษ‘ะฟษซs ษซhษ› gษชฦ’ษซ oฦ’ แดœะฟีชษ›ษพษฏษ‘ษซษ›ษพ","italic":true,"color":"gray"}]','[{"text":"ษพษ›sฯษชษพษ‘ษซษชoะฟ oะฟcษ› gษชฮฝษ›ะฟ ษซo ษซhษ› shษพษชะฟษ›","italic":true,"color":"gray"}]','[{"text":"ฦ…ษ›hษชะฟีช ษซhษ› ษ›ะฟีชlษ›ss ษซแดœะฟะฟษ›l.","italic":true,"color":"gray"}]'],Name:'[{"text":"๐ŸŒŠ","italic":false,"color":"aqua"},{"text":" "},{"text":"ฮ›ีฆ","bold":true},{"text":"แดœษ‘","bold":true,"color":"dark_aqua"},{"text":" ","bold":true},{"text":"ฮ”แดœ","bold":true,"color":"blue"},{"text":"sษซ","bold":true,"color":"dark_blue"},{"text":" ","bold":false,"color":"dark_blue"},{"text":"๐ŸŒŠ","bold":false,"color":"dark_blue"}]'}}},{Count:1b,Slot:1b,id:"minecraft:blue_dye",tag:{display:{Lore:['[{"text":"- gษพษ‘ะฟษซs ษซhษ› gษชฦ’ษซ oฦ’ แดœะฟีชษ›ษพษฏษ‘ษซษ›ษพ","italic":true,"color":"gray"}]','[{"text":"ษพษ›sฯษชษพษ‘ษซษชoะฟ oะฟcษ› gษชฮฝษ›ะฟ ษซo ษซhษ› shษพษชะฟษ›","italic":true,"color":"gray"}]','...

rustic folioBOT
#

Fixes the issue reported in https://github.com/lucko/spark/issues/444

A brief summary is that currently, the bundled spark command is not registered unless the sender has the "spark" permission, whereas it should be registered if the sender has the base "spark" wildcard permission or permission for any of the spark sub-commands.

The fix uses the same approach as the built-in /paper command, using ; as delimiter:

https://github.com/PaperMC/Paper/blob/b483da4e026ad078c9b1dd6e1e5ec25ac450df69/patches/server/0017-Paper-command.patch#L193

Related commit in spark https://github.com/lucko/spark/commit/55b38397296813b66082ad935f773357c8ad5282

rustic folioBOT
#

Expected behavior

https://cdn.discordapp.com/attachments/1244502175573348372/1274646324708642859/2024-08-18_03-22-20.mp4?ex=66d771a5&is=66d62025&hm=f93fde8ae554821e0ba77744afa58e2cc60e2160cc5d50156bf0f29115344c74&

Timestamp: 0:27

Observed/Actual behavior

The redstone build breaks and does not open or close properly as shown in the video.

Steps/models to reproduce

  1. Start a server using the latest paper build (paper-1.21-57)
    hidden.zip
  2. Load into world and paste in the schem file
  3. Flick the lever that's on one of the side and you should see it doesn't work

Plugin and Datapack List

Plugins:
PaperTweaks, TreasureMapsPlus, AntiRaidFarm, AntiVillagerLag, *BannerBoard, Chunky, ClaimChunk, ClearLag, ClearLagTimer, DecentHolograms, dynmap, EntityDetection, Essentials, EssentialsChat, EssentialsSpawn, floodgate, FreedomChat, Geyser-Spigot, Insights, LightAPI, LPC, LuckPerms, Magic, Maintenan...

rustic folioBOT
rustic folioBOT
#

The issue is not just monster_spawn_light_level - I tested by changing all the values to max (monster_spawn_block_light_limit to 15, monster_spawn_light_level to 0-15) and no monster spawns.

I checked the code, and it seems like the problem is (NMS) Mob#checkSpawnRules - for all "sentient" mobs it checks if PathfinderMob#getWalkTargetValue is bigger or equal to 0, but for monsters (except: Giant, Guardian, Pillager, Silverfish, Warden) the value is -(LevelReader#getPathfindingCostFromLightLevels(pos)), which equals LevelReader#getLightLevelDependentMagicValue - 0.5, and as far as I can tell, the value is "proper" ONLY when the light level (in the Overworld) is 7.5 or lower.

The order of calling:

  1. MinecraftServer#tick
  2. ServerLevel#tick
  3. ServerChunkCache#tick
  4. ServerChunkCache#tickChunks
  5. NaturalSpawner#spawnForChunk
  6. NaturalSpawner#spawnCategoryForChunk
  7. NaturalSpawner#spawnCategoryForPosition
  8. `NaturalSpawner#isValidPos...
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Expected behavior

When player hides from the other and starts to ride camel (as rider) the other player which don't see the hidden one can enter the camel and become second passenger (even if he does not see the hidden player which started riding it first)

Observed/Actual behavior

Possibly making camel be on two different locations with the first one that started riding it actually moving with it.

Steps/models to reproduce

  1. Make a way to hide one of your accounts (you'll need two)
  2. Hide one account from the other
  3. Start riding the camel as the hidden account
  4. Step in with the second account
    Result: You both can ride camel but the hidden one actually moves it

Plugin and Datapack List

image
Only mine which contains the command with hiding the other player.

Paper version

image
L...

#

The server just sends a list of passengers to a client, I'd imagine that the client is sent a packet with the passengers but the fact that the client doesn't know about said passenger, it's probably just going to discard it. Best you can probably do is just not allow vanished players to ride entities

rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

I feel like the "isOnline" method should already be checking this. What about changing the implementation of isOnline to also check this?

i mean isOnline docs already mention the player can have instances where is online but is disconnected and recommed check other method.. make this change require change the behaviour of that method and can cause "side" effects...

and yeah exists isDisconnected but that check another place and not the method used in respawn then for this case i think is better just check the same thing...

rustic folioBOT
#

This fixes the natural spawning problem of monsters due to light, which causes any custom natural spawning light levels (from a datapack altering the dimension type or by changing paper-world-defaults#monster-spawn-max-light-level) to be completely ignored.

The natural spawning code (in NaturalSpawner) checks the mob's custom spawn rules (Mob#checkSpawnRules), which, for sentient creatures always checks if PathfinderMob#getWalkTargetValue is 0 or positive. This logic, specifically for monsters, is flawed (except for: Giant, Guardian, Pillager, Silverfish, and Warden), because the value of this function for monsters depends on the light level - in the overworld this means that light levels 7 or below are ok and above 7 are not (which is the default behavior).

This PR changes checkSpawnRules (Monster#checkSpawnRules) to ignore natural spawning (including chunk population) because all monsters already have a custom check for spawning rules in SpawnPlacements. Also f...

rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Expected behavior

When running the following code on a Player p:

private static void stopSwarming(final Player player) {
    for (final World world : player.getServer().getWorlds()) {
        for (final Mob mob : world.getEntitiesByClass(Mob.class)) {
            if (!player.equals(mob.getTarget())) {
                continue;
            }

            mob.setTarget(null);
            mob.setAggressive(false);
        }
    }
}

I expect the anger/pendingTarget Goals of that Mob entity to be changed internally to also remove the target and their aggression from their memories. Below is an example of what I would expect the "anger" Goal of an Enderman to look like after this method is invoked:
image
Notice that the "pendingTarget" field is null.

This stops the Mob from immediately becoming aggressive again for any anger based mob(Enderman, Pigman, Wolf, etc)

###...

rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Expected behavior

When setting send-namespaced: true, Brigadier commands that have its .requires() return false won't appear for auto-completion (which i think is logical and expected).

Same thing should be expected when setting send-namespaced: false, Brigadier commands that have its .requires() return false will still not appear to Player, right? However, this is not the case, setting send-namespaced: false will result in Brigadier command that isn't satisfied by its .requires() to be shown and be available for autocomplete.

Observed/Actual behavior

Setting send-namespaced: false results in Brigadier commands that have its .requires() return false to be shown and be available for autocomplete.

Steps/models to reproduce

Don't forget to set send-namespaced: false.

public class SamplePlugin extends JavaPlugin {
  @Override
  public void onEnable() {
    final LifecycleEventManager<Plugin> manager = this.getLifecycleManager();

  ...
rustic folioBOT
#

Is your feature request related to a problem?

image
Paper 1.21.1 57 build
Liquid spread performance is terrible, always has been. Is there nothing that can be done? That spark screenshot was captured with 200+ ticks over but I am sure you are already familiar with the performance of liquid spread.

Describe the solution you'd like.

I dont know how liquid slope distance is calculated but it doesnt seem like its very efficient, also I am pretty sure it cannot be moved away from the main thread?

Describe alternatives you've considered.

I tried ticking liquids less often, it works but it looks weird as it feels like it lags.

Other

No response

rustic folioBOT
rustic folioBOT
rustic folioBOT
#
[PaperMC/Paper] New branch created: tagset
rustic folioBOT
#

Would something like this help or cause more issues?

protected int getSlopeDistance(LevelReader world, BlockPos pos, int initialDistance, Direction excludedDirection, BlockState state, BlockPos fromPos) {
    Queue<BlockPos> queue = new LinkedList<>();
    Map<BlockPos, Integer> distanceMap = new HashMap<>();
    queue.add(pos);
    distanceMap.put(pos, 0);

    while (!queue.isEmpty()) {
        BlockPos currentPos = queue.poll();
        int currentDistance = distanceMap.get(currentPos);

        if (currentDistance >= this.getSlopeFindDistance(world)) {
            continue;
        }

        for (Direction direction : Plane.HORIZONTAL) {
            if (direction == excludedDirection) continue;

            BlockPos nextPos = currentPos.relative(direction);
            BlockState nextState = world.getBlockState(nextPos);
            FluidState nextFluidState = nextState.getFluidState();

            if (this.canPassThrough(world, this.getFlowing(), ...
rustic folioBOT
#

Expected behavior

I should be able to close any container implementing Lidded interface using method close().

Observed/Actual behavior

Every lidded container remained opened when there were viewers (even though it should be closed because the method for closing was called).

Steps/models to reproduce

Call Lidded#close(); on PlayerInteractEvent

Plugin and Datapack List

Mine and luckperms.

Paper version

This server is running Paper version 1.21-130-master@b1b5d4c (2024-08-10T10:02:42Z) (Implementing API version 1.21-R0.1-SNAPSHOT)
You are running the latest version
Previous version: 1.21-129-4d7cef3 (MC: 1.21)

Other

No response

rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Maybe I could add Experimental annotation, and in case this will be in danger of breaking soon or people will use this wrong etc, this could be quickly deprecated and removed? Seems like this just got stuck on different opinions if it's ok to add

The main points against that were:

Say you serialize an ItemStack on this version and it's got whatever version, say 1000. Then, in 3 years you go to deserialize it, but you want to change it first. 3 years from now, the format for itemstacks may have changed drastically, but you have to go back and know exactly what the format is for your specific data version. That information isn't readily available.

What if you have paper config in a format. After 3 years the format was changed, on docs you of course updated them, and you want to change the config before the server start? Same situation, no info available on that old format, because such situation was never supported before. Same as the plugin having the item in config changed ...

#

Nope, bukkit shutdown also seems to work fine, completely cannot reproduce

shutdown-message: |-
    ยง2ยงlยงnShutdown Bold underline
    ยงf
    ยง6ยงlBold only
    ยงf
    ยง6ยงnUnderline only

(cause bukkit.yml's shutdown message does use legacy section according to the docs)

image

#

While this PR fixes the issue, I think it is best for us to wait with merging this until we have some mojang input on the linked ticket.
Messing with spawn logic like this could break contraptions and until we know why mojang is even checking this in the first place when they are already checking brighness values in SpawnPlacements, this may cause more harm to normal players than good.

rustic folioBOT
#

I'm going to close this issue, because the fix for this is not clear. It's not clear what exactly the universalAnger gamerule is controlling or what is a neutral mob. Should spiders that are hit during the day be angry at all players, not just the one that hit them? They don't implement the NeutralMob interface. I don't think we can fix this without changing behavior in a way mojang might not.

#

Thanks for your first contribution, however when going over this an the linked issue, it's clear that we cannot really fix this without changing some other behavior (which then departs from vanilla functionality). You can see a fuller explanation of it in the message that closed the linked issue. Sorry for not realizing this earlier and closing that issue before you spent time trying to fix it.

rustic folioBOT
#

Mojang disconnected chunk and entity saving a while ago.
Hence why spigot introduced the EntitiesUnloadEvent.

The fact that they save in the same logic again is entire an implementation detail of the chunk rewrite. Having the chunk event be responsible for disabling the entity saving hence becomes rough if this impl ever changes.

I think it would be better to have such a setting on the respective EntitiesUnloadEvent.

#

While this PR fixes the issue, I think it is best for us to wait with merging this until we have some mojang input on the linked ticket.
Messing with spawn logic like this could break contraptions and until we know why mojang is even checking this in the first place when they are already checking brighness values in SpawnPlacements, this may cause more harm to normal players than good.

I opened that ticket today, so doubt it'll be soon...
As I see it, they did the check to add some extra checks for each entity type, which I believe is just a leftover (why not just add that check to the default checker in SpawnPlacements?...).
Anyway, my solution is just one of the many possible solutions - maybe Mojang will say it's an intended behavior (even though that makes no sense - why add custom light levels for monster spawning if you then ignore it?...), or will fix it in another way.

I, for one, will just compile this patch into my own server.

rustic folioBOT
rustic folioBOT
#

Stack trace

#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x0000000000002170, pid=3098, tid=3149
#
# JRE version: OpenJDK Runtime Environment (21.0.4+7) (build 21.0.4+7-alpine-r0)
# Java VM: OpenJDK 64-Bit Server VM (21.0.4+7-alpine-r0, mixed mode, sharing, tiered, compressed oops, compressed class ptrs, g1 gc, linux-amd64)
# Problematic frame:
# C  [native-13390704918971276105.so+0x2a0c]  sort_symbols+0x4c
#
# No core dump will be written. Core dumps have been disabled. To enable core dumping, try "ulimit -c unlimited" before starting Java again
#
# An error report file with more information is saved as:
# /opt/clod-mc/minecraft/hs_err_pid3098.log
[45.296s][warning][os] Loading hsdis library failed
#
# If you would like to submit a bug report, please visit:
#   https://gitlab.alpinelinux.org/alpine/aports/issues
# The crash happened outside the Java Virtual Machine in native code.
# See problematic frame f...
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Hey, currently on latest version of paper, I'm having issues with actually using interface Lidded#close to actually close the container (barrel, shulkerbox or etc...), so I tried fixing it and well it works, I still have to fix some things there as it does not look open but still sends sound (which it shouldn't if it already closed), but I also wanted to hear a feedback before continuing.

Issue (maybe more detailed why): https://github.com/PaperMC/Paper/issues/11364

#

Hey, currently on latest version of paper, I'm having issues with actually using interface Lidded#close to actually close the container (barrel, shulkerbox or etc...), so I tried fixing it and well it works, I still have to fix some things there as it does not look open but still sends sound (which it shouldn't if it already closed), but I also wanted to hear a feedback before continuing.

Issue (maybe more detailed why): https://github.com/PaperMC/Paper/issues/11364

(sorry for previous invalid PR)

#

Yea, I agree that it seems to be a leftover.
But on the other hand, we really have 0 idea why that entire method even exists and diff around there was moved back in 22w12a, so not that long ago.

We'll try to get some info on the ticket, your patch certainly works yea, and for most users this change is probably 100% fine, but messing with spawning like this without a way to disable the early out on the Mob instance spawn check might break contraptions we would not want to break.

Thank you anyway for the PR, we'll poke around with the linked mojira :+1:

rustic folioBOT
rustic folioBOT
rustic folioBOT
#

I'm not sure this has ever worked in the way you are imagining... It looks like every 5 ticks, the game re-checks the open status and will re-send the block event for opening/closing to clients in range. Additionally, the close method will only have an effect if the open method was called first. You can see this if you just call open right before close, you can see the chest start to close, but then open back again (cause 5 ticks have passed).

I'm not sure what the correct fix is here. It seems this API is fundamentally broken. We could add something like a force-closed boolean that prevents any opening from being sent to any clients. Plugins could toggle that on/off (or a force open). But tieing that into the existing Lidded API doesn't seem feasible.

rustic folioBOT
rustic folioBOT
#

From the documentation, I thought close() should always force-close the chest no matter what so I would count with it being closed and not open again. Although I could research more, sorry for that, the PR I wanted to make does not make that sense after this being told.

So from what you've said, maybe adding a new setter & getter with enum LiddedStatus would be good approach by me. Lidded status would have 3 values, FORCE_OPEN, FORCE_CLOSED, DEFAULT. FORCE_OPEN would force Lidded to be opened no matter what, FORCE_CLOSE would force Lidded to be closed and DEFAULT would make it behave normally (for example if someone wants to reset it custom behavior). With that Lidded#isOpen could be overriden to work normally again and it would return its current status (even if it is forced). Should I do that?

rustic folioBOT
#

I've also had a look at this, and I think I've got a patch exposing the ability to force open/force close containers lids in the nms container count manager (so chests, barrels, and enderchests, but not shulker boxes yet).

I also got a new feature working, a flag to automatically remove the force flag (once the first real viewer opens it for for forced open, or once the last real viewer has closed it for forced closed)

The enum based API sounds sensible though, since all the states are mutually exclusive

rustic folioBOT
#

Expected behavior

The relevant packet for PlayerPickItemEvent (ServerboundPickItemPacket) is used by some mods to quick move items through the inventory to the player's hand. This event should extend InventoryEvent - it has all relevant Inventory context to provide the necessary information for an InventoryEvent.

Observed/Actual behavior

When sending a ServerboundPickItemPacket on slot 9, this event will fire and cause inventory manipulation without triggering any InventoryEvent (such as an InventoryClickEvent). This event also does not extend InventoryEvent, making it semi-invisible to Javadocs browsing for trying to handle Inventory-related exploits and bugs.

Steps/models to reproduce

  1. Implement a system utilizing InventoryEvents to prevent players from moving items
  2. Trigger a ServerboundPickItemPacket on a relevant slot
  3. PlayerPickItemEvent will fire, but no InventoryEvents will fire. The inventory will, however, still be mutated.

Plugin and Datapack...

rustic folioBOT
#

Stack trace

[00:42:02 ERROR]: --- DO NOT REPORT THIS TO PAPER - THIS IS NOT A BUG OR A CRASH  - 1.21.1-69-925c3b9 (MC: 1.21.1) ---
[00:42:02 ERROR]: The server has not responded for 10 seconds! Creating thread dump
[00:42:02 ERROR]: ------------------------------
[00:42:02 ERROR]: Server thread dump (Look for plugins here before reporting to Paper!):
[00:42:02 ERROR]: ------------------------------
[00:42:02 ERROR]: Current Thread: Server thread
[00:42:02 ERROR]:       PID: 64 | Suspended: false | Native: true | State: RUNNABLE
[00:42:02 ERROR]:       Stack:
[00:42:02 ERROR]:               java.base@21.0.4/sun.nio.ch.UnixFileDispatcherImpl.write0(Native Method)
[00:42:02 ERROR]:               java.base@21.0.4/sun.nio.ch.UnixFileDispatcherImpl.write(UnixFileDispatcherImpl.java:65)
[00:42:02 ERROR]:               java.base@21.0.4/sun.nio.ch.IOUtil.writeFromNativeBuffer(IOUtil.java:137)
[00:42:02 ERROR]:               java.base@21.0.4/sun.nio.ch.IOUtil.write(IOUtil...
rustic folioBOT
#

Expected behavior

Blocks that placed in End Platform drop items when End Platform generated

Observed/Actual behavior

Blocks don't drop items

Steps/models to reproduce

https://www.minecraft.net/en-us/article/minecraft-1-21-pre-release-1
https://minecraft.wiki/w/End_platform

place block in obsidian platform and re-enter end portal

Plugin and Datapack List

Nothing

Paper version

[18:21:51 INFO]: Checking version, please wait...
[18:21:52 INFO]: This server is running Paper version 1.21.1-69-master@925c3b9 (2024-09-07T20:43:21Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are running the latest version

Other

No response

#

Expected behavior

Well, command suggestions, specifically async.

Observed/Actual behavior

The event gets called but it seems to never use the completions provided.

Steps/models to reproduce

https://github.com/Manered/Commands
Use this library, create a test command:

@Override
public void onEnable() {
  CommandsAPI.api(this)
    .manager()
    .register(CommandNode.literal("test")
      // OfflinePlayerArgument implements AsyncSuggestionHandler which means it has a default.
      .argument(CommandArgument.argument(OfflinePlayerArgument::new, "player"))
      .handler(ctx -> {
        // ...
      })
    );
}

Plugin and Datapack List

Ran on a test server with one plugin - shades in my Command API and creates a simple test command.
TestPlugin

Paper version

[11:30:04 INFO]: Checking version, please wait...
[11:30:04 INFO]: This server is running Paper version 1.21.1-13-master@7c9240f (2024-08-12T16:54:16Z) (Implementing AP...
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Expected behavior

When I execute my /test command as a player with no permissions, it runs successfully.

Observed/Actual behavior

When I execute my /test command using Bukkit#dispatchCommand, the command fails with the message I'm sorry, but you do not have permission to ...

image

Steps/models to reproduce

I created this plugin that registers two commands:

public final class BukkitTestPlugin extends JavaPlugin {
    @Override
    public void onEnable() {
        getLifecycleManager().registerEventHandler(LifecycleEvents.COMMANDS, event -> {
            Commands commands = event.registrar();
            CommandDispatcher<CommandSourceStack> dispatcher = commands.getDispatcher();

            dispatcher.register(Commands.literal("test")
                .executes(context -> {
                    context.getSource().getSender().sendMessage("You ran the t...
#
[PaperMC/Paper] branch deleted: feature/tag-lifecycle-events
rustic folioBOT
rustic folioBOT
#

Featuring

  • 5 LidModes, 3 of which were not possible before
  • The ability to get if the lid actually should be open, if a player is in the container
  • The ability to get if the is open, from api or the player itself

This also deprecates Bukkits api because of the confusing and outright incorrect javadocs (so those now accurately represent the behaviour)

Testable with the included testplugin commands.

Im not 100% sure if the PaperLidded interface is the best way of doing this, but it is working
Also, let me know if the naming needs to change.

#

I'm wondering if an event is better suited for more control over what happens. I feel like some event in the correct spot would enable even more than just these 5 options. I'm not sure how that would look just now, I just had this idea. Maybe some event for when the open count changes? You could then query if it was open or not before, and decide whether to allow it to open, or to close or something.

rustic folioBOT
rustic folioBOT
rustic folioBOT
#

I could imagine an event here (PlayerContainerOpenEvent - maybe someone would come up with a better name ideally not confusable with InventoryOpenEvent) that would allow to prevent opening the lid for a specific player, container and/or situation. I would add that along with the api you already implemented. I think that would handle most possible use cases...

rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

There is this in NbtIo:

private static final OpenOption[] SYNC_OUTPUT_OPTIONS = new OpenOption[]{StandardOpenOption.SYNC, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING};

and I think the StandardOpenOption.SYNC might cause it. I don't know why it is there at all, as is is mostly for transactions to ensure data integrity even if the system crashes by forcing disc writes to be in order. I don't think it is of any use in minecraft (because minecraft does not even use anything like transactions) and will just unnecessarily delay io operations

#

Expected behavior

Should Work in 1.21+ as it was working in 1.20

Observed/Actual behavior

does not work

Steps/models to reproduce

check for player item break event

Plugin and Datapack List

none

Paper version

1.21-2256-de2e7a7

Other

recently spigot issued a fix
https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/commits/ba21e94725b89670f6565070492c61c75fa02f33#nms-patches%2Fnet%2Fminecraft%2Fworld%2Fitem%2FItemStack.patch

rustic folioBOT
rustic folioBOT
#

Expected behavior

I expect this code to work and to give the "config" argument

public final class BrigadierTest extends JavaPlugin {

    @Override
    public void onEnable() {
        // Plugin startup logic
        LifecycleEventManager<Plugin> manager = this.getLifecycleManager();
        manager.registerEventHandler(LifecycleEvents.COMMANDS, event -> {
            final Commands commands = event.registrar();

            SuggestionProvider<CommandSourceStack> configSuggestionProvider = (context, builder) -> {
                builder.suggest("config1");
                builder.suggest("config2");

                return builder.buildFuture();
            };

            SuggestionProvider<CommandSourceStack> optionSuggestionProvider = (context, builder) -> {
                var configName = context.getArgument("config", String.class);

                builder.suggest("option");

                return builder.buildFuture();
            };

 ...
#
[PaperMC/Paper] New branch created: fix/update-data-convertor
rustic folioBOT
rustic folioBOT
#

Yeah, this is a combination of how Brigadier behaves (Mojang/brigadier#142) and how Paper registers commands.

When you register a command through Paper's Commands#register, the main label ("testcommand" here) gets registered as a redirect to the namespaced node ("brigadiertest:testcommand" for example). When a command redirects, Brigadier puts future arguments into a child CommandContext. So, to access the previous arguments in the SuggestionProvider, you need to call CommandContext.getLastChild()#getArgument.

On the other hand, using Commands.getDispatcher()#register does not create a redirected node, which is why the command you gave under Other works. Note that this command can still generate suggestions in a redirected situation using /execute run testcommand..., so it can help to call `CommandContext#g...

rustic folioBOT
#

Resolves https://github.com/PaperMC/Paper/issues/11378 by "restoring" the Spigot behavior where VanillaCommandNodes are only created once. This allows command frameworks that insert CommandNodes directly into the Brigadier dispatcher to change the permission String of the VanillaCommandNodes created for their commands, rather than it always being the default "minecraft.commands.<name>".

For example, the test plugin described in https://github.com/PaperMC/Paper/issues/11378 can do something like this now:

public final class BukkitTestPlugin extends JavaPlugin {
    @Override
    public void onEnable() {
        getLifecycleManager().registerEventHandler(LifecycleEvents.COMMANDS, event -> {
            Commands commands = event.registrar();
            CommandDispatcher<CommandSourceStack> dispatcher = commands.getDispatcher();

            dispatcher.register(Commands.literal("test")...);

            dispatcher.register(Commands.literal("run")...);

     ...
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

I could imagine an event here (PlayerContainerOpenEvent - maybe someone would come up with a better name ideally not confusable with InventoryOpenEvent) that would allow to prevent opening the lid for a specific player, container and/or situation. I would add that along with the api you already implemented. I think that would handle most possible use cases...

Agreed

rustic folioBOT
#

To solve the linked issue properly we need to reimplement dispatchCommand without using the legacy command map.

Fair point. I imagine Paper will eventually remove the CommandMap after hard forking from Spigot, so it would be useful to get started on that by changing the command dispatch and suggestions logic available in the API to use Brigadier directly.

We could also set the permission to null for plugin commands and override hasPermission(Silent) to improve the legacy representation.

Note that this is indeed the case for commands registered using Commands#register. For those Brigadier commands which are associated with a specific PluginMeta, Paper uses the PluginCommandNode extends LiteralCommandNode class as the root node for the command. When PaperBrigadier#wrapNode is given one of these CommandNode instances, it returns a PluginVanillaCommandWrapper extends VanillaCommandWrapper, which by default has its permission set to null. That means `Bukkit#dispa...

rustic folioBOT
rustic folioBOT
#

Expected behavior

Using Entity#setGravity should only change the gravity value of the entity.

Observed/Actual behavior

Setting the gravity of an armor stand to false also changes noPhysics to true. This allows the armor stand to be pushed through blocks.

Steps/models to reproduce

  1. Use the method Entity#hasNoPhysics() on an armor stand. It should return false.
  2. Use the method Entity#setGravity(false) on the armor stand.
  3. Check the noPhysics value again, it should now be true.
  4. You can now push the armor stand with a piston through blocks

Plugin and Datapack List

Only a test plugin to set the gravity and get the physics value. No datapacks

Paper version

This server is running Paper version 1.21.1-76-master@e945cfe (2024-09-10T17:56:53Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are running the latest version
Previous version: 1.21.1-69-925c3b9 (MC: 1.21.1)

Other

No response

rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Expected behavior

No sound

Observed/Actual behavior

When multiple players (in the video, 2 or 3) are at the same location without moving, there is the annoying moving sound.

Sound ON: https://www.youtube.com/watch?v=fZLvVHeJDxQ

Steps/models to reproduce

Join multiple account on same server, spawned at same location (and keep them on same block)

Plugin and Datapack List

Can be done without plugin

Paper version

This server is running Paper version 1.21.1-76-master@e945cfe (2024-09-10T17:56:53Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are running the latest version

Other

No response

rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Expected behavior

playerInventory.setItem(46, someItemStack); should throw error as described in the javadoc as the slot is outside of this inventory. But the server accept it.

Observed/Actual behavior

Actually, the server side allow to do this, and just send it through the packet. But, we are disconnected for "Network protocol error" as the client cannot handle it. Also, it means the javadoc has different content that the real effect

Steps/models to reproduce

  • Login on server
  • Use a plugin that will do:
player.getInventory().setItem(46, new ItemStack(Material.STICK));

Plugin and Datapack List

The test plugin

Paper version

This server is running Paper version 1.21.1-76-master@e945cfe (2024-09-10T17:56:53Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are running the latest version

Other

No response

rustic folioBOT
rustic folioBOT
#

Expected behavior

In pre 1.21 if you do item.setAmount(0) then it simply removes the item. But in 1.21 it throws an error. Is this ability to remove an item something that can return or is it gone for good? I use this method of setting the amount to 0 a lot for my "limited use" items that deplete after each use. So it's easy for me to just set the amount to currentUses - 1. Here's a screenshot of the error https://gyazo.com/19a00bb855851bcb4aeb99a951ff93b8

Observed/Actual behavior

It should remove the item (like it used to) and not throw an error

Steps/models to reproduce

Set an item's amount to 0

Plugin and Datapack List

N/A

Paper version

Any 1.21 ver

Other

No response

rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Expected behavior

This issue applies to all damage types, but is specifically more noticeable in damage types that attempt to deal damage each tick (lava, cactus), so i will use lava as an example.

When standing in lava, I expect armor to take damage every 10 ticks as it does in vanilla, regardless of the amount of damage the lava deals.

Observed/Actual behavior

Armor takes damage and lava burn sounds are played every tick.

Steps/models to reproduce

  1. Create a plugin with the following handler in a Listener:
    @EventHandler
    public void onLava(EntityDamageEvent event) {
    if (event.getCause() == EntityDamageEvent.DamageCause.LAVA) {
    event.setDamage(event.getDamage() * .99);
    }
    }
  2. Run the server and equip armor.
  3. Stand in lava.
  4. Observe your armor taking damage each tick, instead of every 10th tick.

Plugin and Datapack List

Nothing except this listener.

Paper version

This server is running Silv...

rustic folioBOT
rustic folioBOT
#

Stack trace

[15:54:07 ERROR]: Command exception: /fill -38 71 -43 -32 71 -36 minecraft:redstone_wire[power=14, east=up]
java.lang.NullPointerException: Cannot read field "currentState" because "center_up" is null
        at com.destroystokyo.paper.util.RedstoneWireTurbo.calculateCurrentChanges(RedstoneWireTurbo.java:861) ~[paper-1.21.1.jar:1.21.1-57-b483da4]
        at com.destroystokyo.paper.util.RedstoneWireTurbo.updateNode(RedstoneWireTurbo.java:444) ~[paper-1.21.1.jar:1.21.1-57-b483da4]
        at com.destroystokyo.paper.util.RedstoneWireTurbo.breadthFirstWalk(RedstoneWireTurbo.java:638) ~[paper-1.21.1.jar:1.21.1-57-b483da4]
        at com.destroystokyo.paper.util.RedstoneWireTurbo.updateSurroundingRedstone(RedstoneWireTurbo.java:807) ~[paper-1.21.1.jar:1.21.1-57-b483da4]
        at net.minecraft.world.level.block.RedStoneWireBlock.updateSurroundingRedstone(RedStoneWireBlock.java:272) ~[paper-1.21.1.jar:1.21.1-57-b483da4]
        at net.minecraft.world.level.blo...
rustic folioBOT
rustic folioBOT
#

Expected behavior

In the brig api there is no wrapped argument for RotationArgument
net.minecraft.commands.arguments.coordinates.RotationArgument

Observed/Actual behavior

missing api

Steps/models to reproduce

well it is just missing

Plugin and Datapack List

irrelevant

Paper version

This server is running Paper version 1.21.1-76-master@e945cfe (2024-09-10T17:56:53Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are 1 version(s) behind
Download the new version at: https://papermc.io/downloads/paper
Previous version: 1.21.1-65-21f125f (MC: 1.21.1)

Other

No response

rustic folioBOT
#

I dont understand how this could cause a dupe it just looks like a desync
The event itself does not trigger a dupe of any kind. This is a developer UX issue on one hand, and an oversight in the API on the other. PlayerPickItemEvent is not built with the consideration that it can modify non-creative inventories in mind, so it does not extend InventoryEvent or provide any important context. This makes it somewhat invisible to folks making custom GUIs.

also could you link the modpack that causes the issue?
This is the modpack. It's ~175mb. I believe the specific mod here may be MouseTweaks or a different inventory QOL mod - it's the behavior where it swaps an item into your hotbar when it's consumed

rustic folioBOT
rustic folioBOT
#
[PaperMC/Paper] New branch created: player-join-ticket
#

Adding the entity will add and then immediately remove an entity load ticket, which would result in the chunk loading and then unloading before being loaded again once the player chunk loader reacts (delay can vary based on rate limit configs)

By adding a ticket with a short removal delay we attempt to keep the chunk loaded until the player chunk loader reacts, but this is not a guarantee due to the aforementioned rate limit configs. Plugins should still handle load/unload events as normal, however this will reduce redundant calls.

The delay is currently set to 2 seconds, however, we may want to adjust this before merging

#
[PaperMC/Paper] branch deleted: fix-delay-chunk-unloads
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Expected behavior

Chatting should just work, as my friends are all on vanilla clients without mods. I'm using Paper on the server for optimizations (vanilla kept giving me "can't keep up" messages despite having 1 or 2 players on at the time of testing), but I haven't installed any plugins or datapacks, and the vanilla clients have seemed to connect without any issues until yesterday.

Observed/Actual behavior

The first time I observed this behavior was yesterday, and I saw this in the logs:

[18:16:24 INFO]: UUID of player nachocheesefries is 52682139-39f6-4909-9143-75b1640baaa8
[18:16:25 INFO]: nachocheesefries joined the game
[18:16:25 INFO]: nachocheesefries[/172.19.0.3:49072] logged in with entity id 140749 at ([world]572.3000000119209, 70.0, 1305.531163827157)
[18:16:29 WARN]: Failed to update secure chat state for nachocheesefries: 'Chat disabled due to missing profile public key. Please try reconnecting.'

However, at the time, the player had immediate...

rustic folioBOT
#

Profile link

https://github.com/juwonjun

Description of issue

it's syaing

12:50:07 | [DirectoryProviderSource] Error loading plugin: Directory 'plugins\team-1.jar' failed to load!java.lang.RuntimeException: Directory 'plugins\team-1.jar' failed to load!
at io.papermc.paper.plugin.provider.source.FileProviderSource.registerProviders(FileProviderSource.java:59) ~[paper-1.20.1.jar:git-Paper-196]
at io.papermc.paper.plugin.provider.source.DirectoryProviderSource.lambda$registerProviders$1(DirectoryProviderSource.java:34) ~[paper-1.20.1.jar:git-Paper-196]
at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184) ~[?:?]
at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:194) ~[?:?]
at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:212) ~[?:?]
at java.util.Iterator.forEachRemaining(Iterator.java:133) ~[?:?]
at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1939) ~[?:?]
...

#

Profile link

https://github.com/juwonjun

Description of issue

it's saying like so

12:59:40 | [ModernPluginLoadingStrategy] Could not load plugin 'team-1.jar' in folder 'plugins'java.lang.ClassNotFoundException: com.jungj.clan.TeamPlugin
at io.papermc.paper.plugin.entrypoint.classloader.PaperPluginClassLoader.loadClass(PaperPluginClassLoader.java:142) ~[paper-1.20.1.jar:git-Paper-196]
at io.papermc.paper.plugin.entrypoint.classloader.PaperPluginClassLoader.loadClass(PaperPluginClassLoader.java:103) ~[paper-1.20.1.jar:git-Paper-196]
at java.lang.ClassLoader.loadClass(ClassLoader.java:525) ~[?:?]
at java.lang.Class.forName0(Native Method) ~[?:?]
at java.lang.Class.forName(Class.java:529) ~[?:?]
at java.lang.Class.forName(Class.java:508) ~[?:?]
at io.papermc.paper.plugin.provider.util.ProviderUtil.loadClass(ProviderUtil.java:51) ~[paper-api-1.20.1-R0.1-SNAPSHOT.jar:?]
at io.papermc.paper.plugin.provider.util.ProviderUtil.loadClass(ProviderUtil.java:29) ~[p...

rustic folioBOT
#

Is your feature request related to a problem?

Yes, Currently of course most people should not be noticing this as much, but a lot of packets such as block updates, ping/pongs, entity movement, particle packets, level packets, such packets tend to be smaller than most of the network-compression-threshold, I have noticed this after implementing metrics on pps on DonutSMP, and due to our gamemode players have huge farms which do thousands of block updates a second, including particles etc due to water streams and more, this used to be a huge issue for us, since when implementing the metrics I came to realize most of our backend -> proxy bandwidth was actually not being compressed. and a minority of the packets were, for this example here, 98% of our packet rate was smaller than 256 bytes. This would generate also millions of writes as well due to the high amount of packet rate with small byte size.

Describe the solution you'd like.

A "sort of" pack of a buffer, this buffer w...

rustic folioBOT
rustic folioBOT
#
[PaperMC/Paper] New branch created: upstream-update
rustic folioBOT
#

Well this is a bit rough. The behaviour is what you'd expect from vanilla when the damage event is interpreted as "This entity is about to take n points of damage".

Idk if there is a point in potentially exposing a "pls re-check if the damage should still be dealt after damage modification" option to the entity damage event, but the alternative would be to either call entity damage event before that check, which obviously leads to completely wrong calls for the event, or to set the lastDamageAmount to the wrong value, which is also incorrect.

Such a "pls recheck" option cannot be true by default either, because it would break a plugin just setting the value to something less but still expecting the damage to be dealt.

rustic folioBOT
#

Expected behavior

Items in lazy chunks should not age

Observed/Actual behavior

The item age stays consistent until the chunks are fully loaded, then increases by how many ticks passed since the item last ticked

Steps/models to reproduce

  • Go into a non-spawn chunk
  • /forceload add ~ ~
  • throw an item 2 chunks away
    • If you would like to track the item do /data modify entity @n[type=minecraft:item] Health set value 10s
    • And to see the age /data get entity @n[type=minecraft:item,nbt={Health:10s}] Age
  • go in the nether to unload the chunks (or far away)
  • wait (or tick warp) 2000 ticks
  • return to see the age update to be 2000 higher than it was 1 tick ago (if over 6000 the item will despawn as expected)

Plugin and Datapack List

No plugins

No datapacks

Paper version

This server is running Paper version 1.21.1-77-master@4ff58c4 (2024-09-12T18:05:09Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are running the latest version

Othe...

rustic folioBOT
#

How does turning off TCP_NODELAY compare to your implementation? In PPS, Avg/Median packet size and CPU time (probably sys time). Only on Paper's end in the scheme (Paper - localhost Velocity - Player)

To counteract the issue with latency I assume the send buffer of the socket must be reduced.

If the traffic to Velocity proxy is local, is uncompressed traffic really an issue? At what level do you measure the bandwidth? Does it include Ethernet encapsulation overhead or not?

I have no idea what the networking architecture here looks like, but some packets would benefit from immediate sends (at the expense of another syscall) like entity movement (the reason NODELAY was enabled after all) while others could be kept in a per type buffer (i.e. all particles together) purely for improved compression ratios and sent whenever, without an explicit flush or NODELAY. I think it's too optimistic to think such an overhaul will happen for a niche case of a busy server network. Maybe yo...

rustic folioBOT
#

Upstream has released updates that appear to apply and compile correctly.
This update has not been tested by PaperMC and as with ANY update, please do your own testing

Bukkit Changes:
1fc1020a PR-1049: Add MenuType API
8ae2e3be PR-1055: Expand riptiding API
cac68bfb SPIGOT-7890: AttributeModifier#getUniqueId() doesn't match the UUID passed to its constructor
7004fcf2 SPIGOT-7886: Fix mistake in AttributeModifier UUID shim
1ac7f950 PR-1054: Add FireworkMeta#hasPower
4cfb565f SPIGOT-7873: Add powered state for skulls

CraftBukkit Changes:
bbb30e7a8 SPIGOT-7894: NPE when sending tile entity update
ba21e9472 SPIGOT-7895: PlayerItemBreakEvent not firing
0fb24bbe0 SPIGOT-7875: Fix PlayerItemConsumeEvent cancellation causing client-side desync
815066449 SPIGOT-7891: Can't remove second ingredient of MerchantRecipe
45c206f2c PR-1458: Add MenuType API
19c8ef9ae SPIGOT-7867: Merchant instanceof AbstractVillager always returns false
4e006d28f PR-1468: Expand riptiding API
bd8aded7d Ignore che...

rustic folioBOT
rustic folioBOT
#

IF the proxy is local, there's no benefit mostly really due to the kernel handling it all, it doesn't even go through public on the first place between both. but on the scenairo of a lot of Minecraft Servers this might not be the case. No it does not include ethernet encapsulation. It's all just from the JVM and that's it, just CPU time mostly. I don't think sending packets like entity movement instantly. it doens't really optimize or benefit on anything from what I know.

#

IF the proxy is local, there's no benefit mostly really due to the kernel handling it all,

Not for the sending side, but the receiving side should be hit with fewer packets. It's probably not noticable if you have idle CPU cores and memory bandwidth. Otherwise the Unix sockets for local connections were mentioned here before for efficiency and performance.

#
[PaperMC/Paper] branch deleted: upstream-update
rustic folioBOT
#

In the recent upstream update, the paper player profile was updated to
correctly return null for both name and id if constructed as such. This
change however broke the serialisation logic, as it depended on the name
never being null.

The commit moves the checks over to the newly introduced emptyName/UUID
fields that track if the profile was constructed with a null name or
uuid to differentiate it against an empty string or the NIL_UUID.

rustic folioBOT
#

Profile link

not necessary

Description of issue

Instead of calling a sync blocking call from the connection, instead maybe implement a "state" where until the packet listener was a succes, to not flush the queues ("write to the connection"), this way there is not a full risk of an mspt spike due to this call, in which in some cases, it'll happen very often.

The method is #syncAfterConfigurationChange
I'm mainly only opening the issue to report the case, unfortunately I don't think I can open a PR, maybe someone can if paper team thinks this is a valuable add.

Plugin and Datapack List

code

Server config files

Paste configs or paste.gg link here!

Paper version

code

Other

code

rustic folioBOT
#

Expected behavior

ItemStack.getEnchantments().toString() returns {CraftEnchantment[minecraft:silk_touch]=1} for a silk touch enchanted book taken from creative inventory.

Observed/Actual behavior

ItemStack.getEnchantments().toString() returns {} for a silk touch enchanted book taken from creative inventory.

Steps/models to reproduce

  • Get an enchanted book from creative inventory and try to call getEnchantments for this ItemStack. The function returns an empty set.

  • Get an enchanted tool and try to call getEnchantments for this ItemStack. The function returns the enchantments as expected.

Example video of bug reproduction :
https://github.com/user-attachments/assets/9884b9ad-41c7-479f-a648-2500cd1bb3df

Plugin and Datapack List

> plugins
[19:52:17 INFO]: Server Plugins (1):
[19:52:17 INFO]: Bukkit Plugins:
[19:52:17 INFO]:  - AdvancedSilkTouch
> datapack list
[19:53:25 INFO]: There are 4 data pack(s) enabled: [vanil...
rustic folioBOT
rustic folioBOT
#

Expected behavior

Fix bug that doesn't kick players just for typing in chat

Observed/Actual behavior

that problem occurs and kicks players out for writing any text, letter

Steps/models to reproduce

.l

Plugin and Datapack List

antilag, chunky, clearlag, combatplus, essentials, foxaddition,graves,grimac,itemedit,itemtag,luckperms,nametagedit,placeholderapi, skinsrestorer,staffplus,userlogin,viabackwards,viarewind,viaversion, worldedit,worldguard

Paper version

This server is running Paper version git-Paper-9 (MC: 1.20) (Implementing API version 1.20-R0.1-SNAPSHOT) (Git: 3722877)
You are 8 version(s) behind
Download the new version at: https://papermc.io/downloads/paper
Previous version: git-Paper-81 (MC: 1.19)

Other

I have no more information

rustic folioBOT
#

Expected behavior

I have a self-written plugin and it worked on Purpur 2306 version

Observed/Actual behavior

image

Steps/models to reproduce

Install the plugin and start the server

Plugin and Datapack List

image

Paper version

Current Purpur Version: 1.21.1-2306-bc7bcbb (MC: 1.21.1)*

Other

I had to go back to the previous version again for the plugin to work, this is not a PurPur mistake, they just made the last 2 versions upstream
image

rustic folioBOT
#

No, not unless you're performing such digging into internals all over the place. The API already covers the ability to mess with game profiles and has for years.

I know that one of those lines causing error:
Field profileField = skullMeta.getClass().getDeclaredField("profile");
profileField.setAccessible(true);
profileField.set(skullMeta,profile);
How to get the same result using API, without getting into skillMeta's class which (probably) causes error.

rustic folioBOT
#

Yeah, in terms of plugins I have no idea exactly what would be fully affecting this, I can't see why a plugin would be touching something after the compression handler. My current patch seems to work within protocollib and packet events both in as well, so I don't know what kind of dogdy thing the plugin would be doing, but I bet if there's any breakage with this it would be such a small use-case

rustic folioBOT
#

Spigot still maintains some partial implementation of "tick skipping", a
practice in which the MinecraftServer.currentTick field is updated not
by an increment of one per actual tick, but instead of the
System.currentTimeMillis() / 50. This behaviour means that the tracked
tick may "skip" a tick value in case a previous tick took more than the
expected 50ms.

To compensate for this in important paths, spigot/craftbukkit
implements "wall-time". Instead of incrementing/decrementing ticks on
block entities/entities by one for each call to their tick() method,
they instead decrement important values, like an ItemEntity's age or
pickupDelay, by the difference of currentTick - lastTick, where
lastTick is the value of currentTick during the last tick() call.

These "fixes" however do not play nicely with minecraft's simulation
distance as entities/block entities implementing the above behaviour
would "catch up" their values when moving from a non-ticking chunk to a
ticking one as the...

rustic folioBOT
rustic folioBOT
#

ultimatly, yes. a single small file being written to the disk is not going to block for 50 seconds, not unless your kernel is broken. Something is likely causing issues here elsewhere, but, that would generally involve pulling up tools to watch what files are being modified and tracking IO, etc. Not sure what tool I'd recommend for that kinda thing these days.

#
[PaperMC/Paper] New branch created: only-mark-maps-dirty-if-needed
rustic folioBOT
#

Correct me if I am wrong but i think you should deprecate the old methods and create entirely new ones that take Components :)

I cannot deprecate the old methods due to the method signatures. The signature of the old methods and the new methods are the same but the return type is different and because Java doesn't consider the return type in the method signature, I simply need to replace them entirely.

#

Correct me if I am wrong but i think you should deprecate the old methods and create entirely new ones that take Components :)

I cannot deprecate the old methods due to the method signatures. The signature of the old methods and the new methods are the same but the return type is different and because Java doesn't consider the return type in the method signature, I simply need to replace them entirely.

Usually you just use the record style getter/setter methods then

  • getInputNotNumericText -> inputNotNumericText
  • getPrefix -> prefix
  • getPromptText -> promptText

But how you do it right now it breaks backwards compatibility with bukkit and all plugins that used those methods before.

#

Correct me if I am wrong but i think you should deprecate the old methods and create entirely new ones that take Components :)

I cannot deprecate the old methods due to the method signatures. The signature of the old methods and the new methods are the same but the return type is different and because Java doesn't consider the return type in the method signature, I simply need to replace them entirely.

Usually you just use the record style getter/setter methods then

* getInputNotNumericText -> inputNotNumericText

* getPrefix -> prefix

* getPromptText -> promptText

But how you do it right now it breaks backwards compatibility with bukkit and all plugins that used those methods before.

You make an excellent point about the record-style formatting for Component-based API changes, this is an oversight on my part and will be resolved ASAP. Thanks!

rustic folioBOT
#

Profile link

Description of issue

image
The villager check for bed is sort of causing a chunk load, it'd be nice if there is a check for the chunk present/not so that this does not happen. This seems to be mostly affected when there's a huge number of chunk generations or so, cannot confirm with 100% sureness, but I am seeing this call on donut

Plugin and Datapack List

.

Server config files

.

Paper version

.

Other

.

rustic folioBOT
#

Hey @PedroMPagani,
I'm closing this due to you not providing actual logs (a screenshot isn't helpful), and the report being incomplete which makes it more difficult for the people that spend their free time working on Paper.

Please feel free to create another issue with actual logs, code examples to reach your issue, and a more complete report.
Cheers!

rustic folioBOT
rustic folioBOT
#

Expected behavior

Plugins should be loaded after spark is loaded.

Observed/Actual behavior

I tried using spark singleton, but the server told me that spark was not loaded yet.

java.lang.IllegalStateException: spark has not loaded yet!
    at me.lucko.spark.api.SparkProvider.get(SparkProvider.java:45) ~[spark-api-0.1-20240720.200737-2.jar:?]

If I add a dependency on spark in plugin.yml, the plugin cannot be loaded.

[20:04:04] [Server thread/ERROR]: [ModernPluginLoadingStrategy] Could not load 'plugins/.paper-remapped/myplugin-1.4.1.jar' in 'plugins/.paper-remapped'
org.bukkit.plugin.UnknownDependencyException: Unknown/missing dependency plugins: [spark]. Please download and install these plugins to run 

Steps/models to reproduce

no.

Plugin and Datapack List

no.

Paper version

ver
[21:18:03 INFO]: Checking version, please wait...
[21:18:06 INFO]: This server is running Paper version 1.21.1-83-master@2aaf436 (2024-09-17T15:10:4...

rustic folioBOT
#

Expected behavior

shoot a breeze wind charge

Observed/Actual behavior

shoot a fireball

Steps/models to reproduce

try launchProjectile(WindBreezeCharge.class)

Plugin and Datapack List

not plugin related

Paper version

[18:16:32 INFO]: Checking version, please wait...
[18:16:36 INFO]: This server is running Paper version 1.21.1-85-master@c5a1066 (2024-09-19T14:47:47Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are running the latest version
Previous version: 1.21.1-77-4ff58c4 (MC: 1.21.1)

Other

No response

rustic folioBOT
#

The launch API on LivingEntity only respected the WindCharge type, not
its near-clone BreezeWindCharge.

This commit correctly accounts for BreezeWindCharge in
CraftLivingEntity.
Additionally, this also ports over the entire wind charge launch logic
to BlockProjectileSource as dispensers are capable of launching wind
charges.

Resolves: #11417

rustic folioBOT
#

The more I look at the diff the less happy I am with it.
The explosion invocation code is already ugly in vanilla with the large amounts of overload.
Having to add a boolean param to that many method seems annoying.

Not that we can really get around overloading internal methods here, but maybe a Consumer<Explosion> would be more suited to avoid further diff if we expand the API even more?

rustic folioBOT
rustic folioBOT
#

I didn't put it on the data access as we would be sending that extra data to the client (i believe there are some size checks in there so that might break something).

Second point: Do you just want to make the value we calculate when sending the data to the client an extra variable? I don't see how that would be beneficial. Could you please elaborate on that?

rustic folioBOT
#

Thank you for the PR but sadly Owen is right.
We already declined further maintenance in an earlier issue.

In general the API is rather old and has become rather undesirable to maintain given its low adoption in the community.
Upstream also does not seem interested in this API being further developed.
Between the small userbase, a lack of interest in our and apparently upstream's team and its interesting interactions with chat signatures, the conversation API is, at least for this project, sunset. While it "works", we do not want to invest effort into maintaining patches for the conversation API.

I am sorry you put in all this work into a PR towards a sunset system, but I hope my above points at least make our reasoning clear as to why we are considering it sunset.

Thanks again for the effort!

rustic folioBOT
#

I do not have any chat spam/filtering system

I have multiple services that I have dockerized, including the vanilla Minecraft server that I've built off a very basic OpenJDK image from dockerhub, I'd post a link to the base image, but I don't know your rules on external links. I use a dockerized nginx as a reverse proxy to access them all, and handle SSL, though TCP streams don't use SSL certs. my reverse proxy also handles connections to the minecraft container. It's very basic:

# /etc/nginx/stream.d/default.conf
server {
    listen 25565;
    proxy_pass minecraft:25565;
}

Further up the chain, if you want to see where streams are declared on the same level as http, I have this:

# #/etc/nginx/nginx.conf
stream {
    include /etc/nginx/stream.d/*.conf;
}

You can actually see in the logs that everyone has the same IP, I was guessing that might be due to the reverse proxy, and it hadn't caused problems before, and it's not like I need to IP ba...

rustic folioBOT
#

I just had this too. Although I am running plain old Spigot on my test server, I hope the information is still useful.

Seed: 4113167221189323283

[02:09:30] [Server thread/ERROR]: Failed to create block entity minecraft:brushable_block
java.lang.IllegalStateException: Invalid block entity minecraft:brushable_block // net.minecraft.world.level.block.entity.BrushableBlockEntity state at BlockPosition{x=3570, y=51, z=-271}, got Block{minecraft:chest}[facing=north,type=single,waterlogged=false]
	at net.minecraft.world.level.block.entity.TileEntity.a(TileEntity.java:66) ~[spigot-1.21.1-R0.1-SNAPSHOT.jar:4318-Spigot-a759b62-98c57cb]
	at net.minecraft.world.level.block.entity.TileEntity.<init>(TileEntity.java:58) ~[spigot-1.21.1-R0.1-SNAPSHOT.jar:4318-Spigot-a759b62-98c57cb]
	at net.minecraft.world.level.block.entity.BrushableBlockEntity.<init>(BrushableBlockEntity.java:61) ~[spigot-1.21.1-R0.1-SNAPSHOT.jar:4318-Spigot-a759b62-98c57cb]
	at net.minecraft.world.level.block.ent...
rustic folioBOT
#

Expected behavior

Per Bukkit Javadocs, the Server#getRegistry(Class) method returns null when a registry is not available for the given Keyed class.

Observed/Actual behavior

Paper asserts that a registry must exists and throws a NullPointerException:

[10:40:39 ERROR]: Exception when Jannyboy11 attempted to tab complete Jannyboy11
org.bukkit.command.CommandException: Unhandled exception during tab completion for command '/igive Jannyboy11 ' in plugin InvSeePlusPlus_Give v0.29.6-SNAPSHOT
        at org.bukkit.command.PluginCommand.tabComplete(PluginCommand.java:150) ~[paper-mojangapi-1.21.1-R0.1-SNAPSHOT.jar:?]
        at org.bukkit.command.Command.tabComplete(Command.java:101) ~[paper-mojangapi-1.21.1-R0.1-SNAPSHOT.jar:?]
        at io.papermc.paper.command.brigadier.bukkit.BukkitCommandNode$BukkitBrigSuggestionProvider.getSuggestions(BukkitCommandNode.java:118) ~[...
#

Yes, as Materials are not a registry concept, that's a bukkit/spigot concept, mojang doesn't have a registry of all Materials

So [this announcement](#dev-announcements message) on the PaperMC Discord is fake news then? I quote:

Going beyond this, please especially make sure you move away from Material.values() calls and instead get them [via the Registry API](https://jd.papermc.io/paper/1.21/io/papermc/paper/registry/RegistryAccess.html#getRegistry(io.papermc.paper.registry.RegistryKey)) (or the [upstream compatible one](https://jd.papermc.io/paper/1.21/org/bukkit/Server.html#getRegistry(java.lang.Class)))
rustic folioBOT
#

The Bukkit#getRegistry(Class) method contract specifies that it returns
null for unknown registry types. The current implementation however
requires the passed class to be mappable to a known registry key.

For types like Material, which have a SimpleRegistry in bukkit's
Registry interface, no server side registry exists and such the type
cannot be mapped to a registry key.

The commit correctly returns null for types that are not mappable to a
registry key instead of throwing a NullPointerException.

Resolves: #11421

rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Looking at this further, hijacking the slot setup is not gonna work for most implementations sadly, e.g. opening a custom inventory of that type currently just has hardcoded values for this. So I guess ctor overload it is or some fuckery with instanceOf in the ctor for ContainerData.

So to that degree I guess we need to force it being passed as an actual variable to a constructor/overload both.

Is starting really a good description for what this value does?
I mean, from a technical perspective, yea, but maybe something along the lines of "total" or "required"?

rustic folioBOT
#

I can confirm this happening on my server too but I'm definitely using a chat filtering system based on Skript.

I think it used to work fine but I can reproduce this and will try to find more information and share here as soon as I can.

The problem for me seems to be that the Skript is listening to two events: on chat AND on command. For the on chat event I can replace parts of the message while for the on command that does not work and thus the event is cancelled instead. The problem is that some commands such as /msg are really a chat message and it being cancelled results in the chain breaking as @electronicboy pointed out.

I think it used to be the case that for commands such as /msg no chat event was fired and thus it worked fine but potentially something has changed and now the chat event does fire and thus this issue arises.

As I said, I'll dig a little deeper and troubleshoot this some more to see if I can find a solution to my problem at least.

rustic folioBOT
#

Well this even happens on 1.21 build 1 of paper so it's potentially always been this way.

I was also able to verify that this happens in any command that's a message in reality so you definitely can't cancel a command such as /minecraft:msg Folas1337 Hi. Does paper provide any way to tell apart "non-message commands" from commands which are actually messages such as /minecraft:msg and then also a way to replace parts of that message/command? Or alternatively any way to cancel the command while not breaking the message chain?

This is all pretty confusing and the fact that I also partially don't know what I'm talking about does not help the fact either ๐Ÿ˜•

Alternative solutions to this are definitely also welcome ๐Ÿ˜…

rustic folioBOT
#

Expected behavior

The "internal" tag will continue to work...

Observed/Actual behavior

The tag is lost upon loading and storing the ItemStack, as it now seems to be renamed to "custom" without automatic migration

Steps/models to reproduce

Given the below yaml output, load it as itemstack and save it again on 1.21.1

        spawns:
        - custom_egg:
            ==: org.bukkit.inventory.ItemStack
            v: 3700
            type: MAGMA_CUBE_SPAWN_EGG
            meta:
              ==: ItemMeta
              meta-type: SPAWN_EGG
              display-name: '{"extra":[{"italic":false,"color":"white","text":"Spawn LavaLurker"}],"text":""}'
              lore:
              - '{"extra":[{"bold":false,"italic":false,"underlined":false,"strikethrough":false,"obfuscated":false,"color":"gray","text":""}],"text":""}'
              - '{"extra":[{"bold":true,"italic":false,"color":"dark_green","text":"< "},{"bold":false,"italic":false,"color":"gray","...
#

There is no structure here to run through DFU. We could maybe create a means for handling known keys, but for anything else, all we could maybe try to do is transpose them over to the custom data component. However, I'm not sure that there is much desire to invest much effort into this broken form of Item persistence.

https://github.com/PaperMC/Paper/pull/10609 - will likely be the solution here, but that will not deal with situations upstream has not handled on upgrades. ItemMeta is highly broken, especially when it comes to serialisation; this is why I've been warning against it for eons. The best you can probably do is release an update for older versions to migrate the format to something sane and then get people to upgrade.

#

The PlayerCommandPreprocessEvent is fairly long known to be broken and will break the chain state if it's being used to cancel stuff, that is probably what is causing your issue

That's good to know but then I wonder what a good solution to the problem would be? :thinking: Is there any other way to cancel a command that I should use instead or should the chat plugin I'm using instead see if the command was canceled and then stop even trying to send a chat message so it wouldn't break the chain?

rustic folioBOT
#

Gotcha. I got it fixed in my own library nice and dirty way, but I agree with what you said, unfortunately keeping up 1.8.8-1.21 support is not straightforward. I propose you add a dirty fix for this for now, and in the future move towards a hard paper fork, get rid of all the deprecations etc., essentially force plugin authors to develop against paper, which would solve a lot of issues.

Here is my code for the record, it actually implements a whole custom yaml parser inspired by bukkit:

private class ConstructCustomObject extends ConstructYamlMap {

		private final boolean atLeast1_21 = MinecraftVersion.atLeast(V.v1_21);

		@Override
		public Object construct(Node node) {
			if (node.isRecursive())
				throw new YamlEngineException("Unexpected referential mapping structure. Node: " + node);

			final Map raw = (Map) super.construct(node);

			if (raw.containsKey(ConfigurationSerialization.SERIALIZED_TYPE_KEY)) {
				final String classPath = (String) raw.get(C...
#

Personal note keeping after talking it over with MM.
Create new BrewingSimpleContainerData subtype that extends SimpleContainerData but inits itself with value 2 set to 400.
Pass that in simple ctro and CraftContainer. Set both BrewingBLockEntityData and BrewingSimpleContainerData to count 3, keep the current "new containerData() { }` in setDataSlots to "translate" back to count 2 and do the conversion,

rustic folioBOT
#

@jpenilla Suggested that paper could add the ability to cancel signed commands (which until now I didn't know they existed or that that's what they're called).

It sounds like a really good idea to me however as it would avoid these situations and allow more freedom as you wouldn't have to use all kinds of plugin APIs just to cancel their specific event cause they're using signed commands and when you switch plugins you have to support yet another one.

As a simple owner who doesn't do any coding this is basically an impossible task and even to a plugin developer this would mean having to support dozens if not hundreds of plugins' APIs if you want to somehow censor commands like I just explained.

I think this would really be beneficial.

rustic folioBOT
rustic folioBOT
#

After further consideration, we thinking throwing is the more correct behavior. It's odd to clamp values to 1 instead of specifying what values are valid in the first place and disallowing other values. A PR adding a thrown exception for the EntityScheduler#execute method which I think is the only method that doesn't throw out of all the scheduler methods.

rustic folioBOT
#
[PaperMC/Paper] New branch created: command-preprocess
rustic folioBOT
#

I moved this PR over to an environment variable, which should be a lot easier to use than a volume mount from a secret.
The PR will still respect a potentially configured value to enable people on restrictive hosts etc to continue to define their secrets via a config, but once the configured env var is set, it'll take precedent.

The PR is gonna be up for more in-depth review tomorrow, if you have time till then to give feedback on my changes, that would be great so we can merge (would not wanna merge without an okay from you @oliverjanka, given the drastic changes).

If we wanna revert to your previous state, 4d85ab46001bded20077c2b7bcb8763783211588 was the previous HEAD.

rustic folioBOT
rustic folioBOT
#

Profile link

.

Description of issue

  1. As you can see in the video i attached below, it took me 4 attempts to whitelist the username "1000". Every time the command failed with the result "That player does not exist", even though on the 4th try it whitelisted the player. This probably means that the whitelist command failed to fetch the UUID of the username for some reason (API ratelimit possibly), but the error message isn't accurate.
  2. Almost every time I run the command, you can see in my video example that the server freezes until the command finishes. This affects the whole server, I've had another player tell me they almost died due to the lag when I tried adding someone to the whitelist. This issue is present in both the add and remove arguments of the command, as shown in the video

Plugin and Datapack List

Bukkit Plugins:

  • FreedomChat, GrimAC, InvSeePlusPlus, LuckPerms, OgPlace, ProtocolLib, SuperVanish, ViaBackwards, ViaRewind, ViaRewind-Legacy-Support, ...
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

A) the way the code is structured would make a more meaningful error message along the lines of "rated limited" incredibly difficult to near impossible for us to implement without just fully re-implementing the whitelist command and player lookup logic.

B) The command doing its work on the main thread is something that recently was also discussed on #11155.

I think that there was generally some long term concerns over toying with some of this stuff due to stuff potentially expecting mutation to occur during execution
Moving the execution of the whitelist command off the main thread may simply break existing things that expect the command to finish its work immediately. The avenue of potentially exposing off-thread capabilities for this on an API level, allowing plugins to easily create "async whitelist" commands is something doable, but at least for me, modifying the vanilla command behaviour in that way is non desirable.

Will keep issue open for some other peoples feedba...

rustic folioBOT
rustic folioBOT
#

the only real "solution" for this is essentially inserting a hack into when item stacks are serialized for inventories. that's the most "compatible" since it only works around the client, though it would be a pain to maintain. one way or another, Mojang needs to be pushed to properly fix the issue since this feature is an intentional part of the client. it's not impossible to be worked around, there's just going to be a trade off somewhere.

rustic folioBOT
#

Actually I'm not in favour of providing secret data through environment variables due to security concerns. This is one of the reasons I opted for this approach (along with the fact that Velocity works similarly).
However, I'm currently very busy and do not have time to work on my previous or an alternative approach and I'd like to see this feature implemented soon.

Therefore, I wonโ€™t stand in the way of you proceeding with this method ๐Ÿ™‚.

#

Thanks for your quick feedback.
While yea, an environment variable is accessible by anything in the pod, so is the velocity secret file given the user running the server process does need read permissions, and the server internal field storing said secret.

I'll proceed with the environment variable just to get some way of doing it in, and if we care enough, the above ref can always be used to potentially revisit a file based approach as well as the env variable.

rustic folioBOT
#
[PaperMC/Paper] branch deleted: weee
#
[PaperMC/Paper] branch deleted: command-preprocess
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Expected behavior

Works fine on 1.20.4 below
it would be great the API can add an additional function on this to set whether arrows are consumed or not.

Observed/Actual behavior

Arrows are being consumed on event cancel

Steps/models to reproduce

  1. shoot an arrow
  2. event.setCancelled(true)
  3. arrows are consumed

Plugin and Datapack List

[11:45:19 INFO]: Server Plugins (5):
[11:45:19 INFO]: Bukkit Plugins:
[11:45:19 INFO]:  - Essentials, FNAmplifications, RelicsOfCthonia, SfChunkInfo, Slimefun

Paper version

This server is running Paper version 1.21.1-96-master@4514c71 (2024-09-22T21:50:05Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are running the latest version
Previous version: 1.21.1-85-c5a1066 (MC: 1.21.1)

Other

No response

rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Wait, sorry a lot happened in a very quick amount of time, and I didn't have any time to process and test the commands @Folas1337 was referring to in order to verify if we had the same issue.

I apologize, but I'm also not familiar with the minecraft ecosystem as a whole. Are we saying that sometimes messages register as a command, and as such it'll sometimes fail?

I went ahead and tried \minecraft:msg Aerothix0 Hi (Aerothix0 being myself), and it seems there wasn't an issue on my end? Though, it was to myself, and I'm not sure if that's handled differently from whispering to someone else or not.

I'm sorry, I'm making it more difficult because my server is so small that there's not a lot of opportunity with other players to keep testing things hahaha.... the one person I think who might be the one actually causing problems nachocheesefries is the one who gets on the least, and I suspect that he might be the problem simply because I've been closely monitoring the server a...

rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Hey sorry, I admittedly kind of piggy bagged your issue here because the exact same error you reported was the one I also got from what I did before.

Now let me go through your message and give some answers:

I didn't have any time to process and test the commands @Folas1337 was referring to in order to verify if we had the same issue.
You'd need to install Skript and use the exact same thing I did to be able to reproduce it, I would not really suggest doing so and also you'd come to the very same outcome.

Are we saying that sometimes messages register as a command, and as such it'll sometimes fail?
The exact opposite. Commands sometimes register as messages or rather they are signed with your key due to Minecraft's somewhat new chat system. Thus when you use something like /msg <user> Hello, it will be signed without you even realizing and that has probably/potentially lead to the problem your friend was experiencing.

I went ahead and tried `\minecraft:msg Aer...

rustic folioBOT
#

No I appreciate the breakdown actually. Being completely out of my element, it helped me to see what you were getting at and where our problems intersected as well as where they may differ (if they are indeed separate problems). I actually had the same idea as you, to just update and see if it happens again hahaha (which I did just last night). Unfortunately, without a reliable way to recreate it as of now, I have to agree that I just kind of have to hope that even if our problems aren't the same, then they intersected enough that a fix for you was a fix for me (or that it really was just a hiccup). Especially since you had a reliably reproducible bug that had the same output.

Though, I'd like a little more reassurance on my reverse proxy, either through exploration to see if it's causing problems, or maybe some enlightenment on how Minecraft handles secure signing and if it has anything to do with IPs connecting to the server (and essentially if local IPs can break the chain o...

rustic folioBOT
#

Expected behavior

Within the WorldGuard protected area, attacking while sprinting or using (Knockback:Enchantment) should not trigger Knockback

Observed/Actual behavior

Attacking while sprinting or using (Knockback:Enchantment) both triggered Knockback and activated (Knockback:Enchantment)

Steps/models to reproduce

placing a boat on land or in the water
attacking a boat with a weapon that has 'Knockback:Enchantment', or attacking a boat with an empty hand while sprinting

https://github.com/user-attachments/assets/24caf6b2-6909-410c-afc2-c588fbbcc4b8

Plugin and Datapack List

worldedit-bukkit-7.3.6 , worldguard-bukkit-7.0.12-dist

Paper version

This server is running Paper version 1.21.1-98-master@9b1ee0d (2024-09-24T02:24:07Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)

Other

Is there a missing related event?
I cannot confirm if this is due to WorldGuard; if it is, please feel free to disable it

rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Stack trace

https://mclo.gs/sute7H7

Plugin and Datapack List

[12:43:31 INFO]: Server Plugins (15):
[12:43:31 INFO]: Bukkit Plugins:
[12:43:31 INFO]: - CMI, CMILib, CommandRestrictor, CustomEnchants, LuckPerms, PlaceholderAPI, ProtocolLib, SimpleClaims, SimpleCrates, SkySquidPlugin
[12:43:31 INFO]: Vault, ViaBackwards, ViaVersion, WorldEdit, WorldGuard

Actions to reproduce (if known)

I am using a plugin that allows players when shooting with a special bow to ride the arrow and dismount it when it lands, I using the following events EntityShootBowEvent , EntityDismountEvent and ProjectileHitEvent

If a player shoots the arrow in overworld through a nether portal it will crash the server.

Paper version

[12:43:43 INFO]: This server is running Paper version 1.21.1-99-master@1bc02e6 (2024-09-25T02:41:02Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are running the latest version
Previous version: 1.21.1-2312-d7a7c91 (MC: 1.21.1)

Other

Is this ...

rustic folioBOT
#

Generally, LGTM, however should be merged into the Add support for Proxy Protocol patch itself instead of a new patch.

Additionally, if you are moving things around anyway, this might be nicer to be right under the other logging in bind(), e.g. under the velocity native logs, just to keep everything neatly in one place.

So I'm still new with patching and the way paper does it's changes, however. I assume I would have to edit the Add support for Proxy Protocol patch through the CONTRIBUTION instructions? And then put it together with the rest of the logging?

rustic folioBOT
rustic folioBOT
rustic folioBOT
#

Have you 100% narrowed it down to the plugin you are working on (i.e. have you removed all other plugins and checked that it still happens)?

If so, can you provide more code or your plugin source for how you are doing this? I could not replicate this issue using the simple

@EventHandler
public void onEvent(EntityShootBowEvent event) {
    event.getProjectile().addPassenger(event.getEntity());
}

I shot a bow at a nether portal in the overworld, and was tp'd to the nether after riding the arrow.

#

Have you 100% narrowed it down to the plugin you are working on (i.e. have you removed all other plugins and checked that it still happens)?

If so, can you provide more code or your plugin source for how you are doing this? I could not replicate this issue using the simple

@EventHandler
public void onEvent(EntityShootBowEvent event) {
    event.getProjectile().addPassenger(event.getEntity());
}

I shot a bow at a nether portal in the overworld, and was tp'd to the nether after riding the arrow.

Hi yes its my plugin, here is the code I am using for it.
https://mclo.gs/IRA8k8l

rustic folioBOT
#

After for research, this might not be a Paper issue, but rather a plugin just removing an entity in the wrong place. Basically non-player entities are destroyed and recreated when they change dimensions. As part of the changing dimensions process, before they are removed, all their passengers (in this case the player) are dismounted. This fires the EntityDismountEvent. If in the handler for the dismount event, you remove the entity, then the entity's scheduler is retired. But the entity isn't actually being completely removed, and the CraftEntity wrapper is transferred over to the new nms Entity (and that keeps the now retired scheduler) which then causes this exception/crash.

rustic folioBOT
rustic folioBOT
#

Expected behavior

Using LivingEntity#setCollidable(boolean) should disable or enable collisions with the living entity.

Observed/Actual behavior

When setting collidable to false on Player, it collides with another entity no matter if it is set to false or true.

Steps/models to reproduce

  1. Spawn entity or obtain entity instance
  2. Get player instance
  3. Use setCollidable(false) on the player
  4. Collide with the entity :)

Plugin and Datapack List

[16:52:55 INFO]: Server Plugins (4):
[16:52:55 INFO]: Bukkit Plugins:
[16:52:55 INFO]: - FastAsyncWorldEdit, Multiverse-Core, ServerFramework, VoidGen

Paper version

[16:53:13 INFO]: Checking version, please wait...
[16:53:14 INFO]: This server is running Paper version 1.21.1-99-master@1bc02e6 (2024-09-25T02:41:02Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are running the latest version
Previous version: 1.21.1-98-9b1ee0d (MC: 1.21.1)

Other

No response

rustic folioBOT
#

I have this logic which you say, should work:
Player player = context.getArgument("player", PlayerSelectorArgumentResolver.class).resolve(context.getSource()).getFirst(); disguiseManager.disguiseAsEntity(player, entityType.getEntityClass(), entity -> { player.getCollidableExemptions().add(entity.getUniqueId()); if (entity instanceof LivingEntity livingEntity) { livingEntity.setAI(false); livingEntity.getCollidableExemptions().add(player.getUniqueId()); } });
However this does not work.

rustic folioBOT
rustic folioBOT
#

The more I look at the diff the less happy I am with it. The explosion invocation code is already ugly in vanilla with the large amounts of overload. Having to add a boolean param to that many method seems annoying.

Not that we can really get around overloading internal methods here, but maybe a Consumer would be more suited to avoid further diff if we expand the API even more?

E.g. something like these two https://pastebin.com/4c1ZhtL0 https://pastebin.com/J6QwPZ4Q

I implemented these changes and everything seems to work.

rustic folioBOT
rustic folioBOT
rustic folioBOT