ProtocolSupport / ProtocolSupportPocketIssues

Issue-tracker for ProtocolSupport pocket edition.
6 stars 0 forks source link

Signs are missing on Paper build 552 and newer #67

Closed magicus closed 3 years ago

magicus commented 5 years ago

Signs are not showing for PE players.

There seem to be some special cases that make signs show up. If a PE player places a new sign, it shows up for other PE players. However, after logout, it has once again disappeared. Also, according to AlexCreeper25 on discord: "If u break a protected sign (eg protected by worldguard), it will then show up".

I'm tagging this as affects_gameplay, since signs can contain vital instructions for gameplay.

Edit: WORKAROUND: Use Paper 551 or earlier.

magicus commented 5 years ago

To clarify: The entire block is invisible. It's not just a matter of the text of the sign missing.

magicus commented 5 years ago

It turned out that signs placed free-standing on the ground look OK, so it's just wall signs.

Edit: No, this was a mistake from my part. I saw the newly placed sign, but after logout, it too had disappeared.

Can this has something to do with the newly introduced different-kinds-of-wood signs in 1.9?

Or is it some kind of meta-data mapping NBT thing that has changed?

magicus commented 5 years ago

Some update on signs functionality:

So maybe something happens if the sign loads as part of a chunk..?

colinrgodsey commented 5 years ago

so one hint for the signs: im pretty sure its related to a paper update. i had signs working fine, and upgraded only paper, then my signs broke

Beaness commented 5 years ago

Paper recently made a patch update that is related with signs

Beaness commented 5 years ago

Found a "recently" commit: https://github.com/PaperMC/Paper/commit/b2d7ef4f3cf309eb55db388fb2d1c5a7bdb1d24d

magicus commented 5 years ago

@SiebeDW Interesting find. I tried running paper with -DPaper.maxSignLength=-1, but unfortunately this did not help. Not sure if there's anything else in that patch that can mess things up, but it doesn't look so malign.

magicus commented 5 years ago

@colinrgodsey Thanks for the input! I tested with paper build 503 (build just before 20 January), and there signs worked. And the latest, 566, does not work. So, it's bisection time!

magicus commented 5 years ago

Bisection results:

I suspect the patch https://github.com/PaperMC/Paper/commit/a684cde66bc0e7d4c92a0e95393fd25ee82304b0 which is one of three commits in that build. For the record, I tested running with -DPaper.excessiveSignsLimit=-1 as suggested in the patch, and it didn't help restore signs.

magicus commented 5 years ago

Yeah, they are doing fishy things in that patch. Basically, they do:

        +                // Paper start - send signs separately
    +                if (tileentity instanceof TileEntitySign) {
    +                    if (SKIP_EXCESSIVE_SIGNS_LIMIT < 0 || ++totalSigns < SKIP_EXCESSIVE_SIGNS_LIMIT) {
    +                        extraPackets.add(tileentity.getUpdatePacket());
    +                    }
    +                    continue;
    +                }

and then they send the contents of extraPackets separately. Note the continue, which makes TileEntitySign skip the rest of the processing. And clearly, setting Paper.excessiveSignsLimit to -1 does not remove the "send signs separately" thingy, just removes the limit per chunk.

Beaness commented 5 years ago

So since this actually is like a paper problem since the cause is their patch, what about creating an issue on their issue tracker?

Artuto commented 5 years ago

I mean; Paper is a Minecraft Java project. Dont know how they will take an issue regarding another platform even if its a plugin.

Beaness commented 5 years ago

Well but still they broke it

magicus commented 5 years ago

I'm not claiming it's actually a Paper problem. They changed how they expose chunks, and PSPE broke. I think the easiest course of action is to adapt PSPE to handle the new format, if it's reasonably possible. If not, then we might have to ask Paper to do something about it. Since there are no formal specs here, we can't claim they violate the spec or anything.

I believe with the input given above, someone in the PSPE developer community (like @colinrgodsey) will probably know if it's possible to handle the new chunk format in PSPE.

magicus commented 5 years ago

In the meantime, use the workaround. That's a perfectly legitimate solution for PSPE at the current pre-alpha level of development.

magicus commented 5 years ago

Shev suggested on discord: "You just need to attach an empty tile data on chunk send for this block And you can actually use existing was tile remapper for it. Tho you will have to check that tile data doesn't exist already. This can also be an issues for some of the pc versions too. I know that 1.10 doesn't like when tile data is missing."