Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix conversion issues between Legacy and MiniMessage #5702

Merged
merged 4 commits into from
Feb 26, 2024
Merged

Fix conversion issues between Legacy and MiniMessage #5702

merged 4 commits into from
Feb 26, 2024

Conversation

pop4959
Copy link
Member

@pop4959 pop4959 commented Feb 24, 2024

Fixes #5652 (and probably numerous other issues...)

image

@pop4959 pop4959 added type: bugfix PRs that fix bugs in EssentialsX. module: main Issues or PRs for the main Essentials module misc: adventure Issues and PRs related to the Adventure refactor. Temporary label during 2.21.x cycle. labels Feb 24, 2024
@Bobcat00
Copy link
Contributor

Good news and bad news: It fixes the obfuscated formatting in the player's prefix, but broke the rest of the /whois output (God mode and below).

Testing with Paper version git-Paper-435 (MC: 1.20.4)

Before PR (using latest dev build Essentials v2.21.0-dev+56-1929d41):

image

After PR (using Essentials v2.21.0-dev+57-a81725b):

image

@mdcfe
Copy link
Member

mdcfe commented Feb 24, 2024

Fixing specific incorrect formatting in /whois is probably more sensible to have as a separate PR after this is merged.

mdcfe
mdcfe previously approved these changes Feb 24, 2024
View reviewed changes
@pop4959
Copy link
Member Author

pop4959 commented Feb 25, 2024

fixed the parsed placeholder bug (this was caused by going through the ser/deser process which escapes the output - this is already injection proof so we just need to make sure not to do this for parsed placeholders

new before/after:
566c81ef-6658-40ae-bb65-569b5d3d09f1
a3627ddd-bf61-47ac-9530-142c8edce0cb

@pop4959 pop4959 requested a review from mdcfe February 25, 2024 09:04
@Bobcat00
Copy link
Contributor

This looks OK to me. Tested with Paper version git-Paper-435 (MC: 1.20.4) and Essentials v2.21.0-dev+62-5216246. Tested commands /list, /whois, /seen, /msg, and /me.

@pop4959 pop4959 requested a review from JRoy February 25, 2024 18:06
@JRoy JRoy enabled auto-merge (squash) February 26, 2024 01:37
@JRoy JRoy disabled auto-merge February 26, 2024 01:37
@JRoy JRoy changed the title Fix broken legacy to mini conversion Fix conversion issues between Legacy and MiniMessage Feb 26, 2024
@JRoy JRoy enabled auto-merge (squash) February 26, 2024 01:38
@JRoy JRoy merged commit 1b1b511 into EssentialsX:2.x Feb 26, 2024
1 check passed
@pop4959 pop4959 deleted the fix-5652 branch February 26, 2024 01:39
HarvelsX pushed a commit to HarvelsX/Essentials that referenced this pull request Jun 2, 2024
@pop4959 @HarvelsX
Fix conversion issues between Legacy and MiniMessage (EssentialsX#5702)
a203a04 
Fixes EssentialsX#5652
Starmism pushed a commit to valence-smp/Essentials that referenced this pull request Jan 8, 2025
@pop4959 @Starmism
Fix conversion issues between Legacy and MiniMessage (EssentialsX#5702)
c23528f 
Fixes EssentialsX#5652
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JRoy JRoy JRoy approved these changes

@mdcfe mdcfe Awaiting requested review from mdcfe

Assignees
No one assigned
Labels
misc: adventure Issues and PRs related to the Adventure refactor. Temporary label during 2.21.x cycle. module: main Issues or PRs for the main Essentials module type: bugfix PRs that fix bugs in EssentialsX.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Prefix formatting bleeds into display name
4 participants
@pop4959 @Bobcat00 @mdcfe @JRoy