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.

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

Fix desync charactorvisitors and Caxton compatibility #14

Merged
merged 17 commits into from
Feb 9, 2025

Conversation

Wetterquarz
Copy link
Contributor

@Wetterquarz Wetterquarz commented Jan 7, 2025

This pull request aims to fix #12 (#16) and improve compatibility.

The fix of #12 has currently been achieved.
Compatibility (#1) has not been tested. Caxton is now compatible as of 5f80a30 (fix keve1227/furigana#3).

I have changed the architecture from "$^text(ruby)" converting to "\ue9c0text\ue9c1ruby\ue9c2" now replacing "$^text(ruby)" with a single character (\uFFFC) and then embedding the text and ruby in the net.minecraft.text.Style class.

As this is a major rewrite, there is currently still some differences in rendering which need to be fixed:

  • Ruby is displayed a few pixels lower than before in Above-mode (Fixed with 6f41c86)
  • Kanji are not equally spaced out in the gap but bunched in the middle (Fixed with 6f41c86)
  • Autocomplete suggestions are rendered too far left (Fixed with 4cf1f3f)

@keve1227
Copy link
Owner

I'm out of town for the weekend but I'm planning to take a closer look at this marvel on Tuesday 😁

@keve1227 keve1227 changed the base branch from main to 1.21.3 January 25, 2025 12:28
@Wetterquarz Wetterquarz changed the title Fix desync charactorvisitors Fix desync charactorvisitors and Caxton compatibility Jan 28, 2025
@Wetterquarz Wetterquarz marked this pull request as ready for review January 31, 2025 15:04
@keve1227 keve1227 force-pushed the fix-desync-charactorvisitors branch from f34ad52 to 6cc2e84 Compare February 6, 2025 14:34
I don't know why this didn't cause infinite recursion 🤔
@keve1227 keve1227 force-pushed the fix-desync-charactorvisitors branch from 86da988 to 8e71e47 Compare February 6, 2025 19:40
Copy link
Owner

@keve1227 keve1227 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I marked getWidthMutex as volatile since I can't imagine any kind of "mutex" that wouldn't be. I'm just not sure what getWidthMutex and isSeparatingDrawCall are and if maybe they should both be marked volatile.

@keve1227
Copy link
Owner

keve1227 commented Feb 7, 2025

I must thank you for this. Using a single character per ruby and storing the details in the style is a really elegant solution to the line wrapping and overflow issues. If you feel this PR is ready, I'm happy to merge it.

  • Autocomplete suggestions are rendered too far left

I'm not entirely sure what this point is referring to. Has this been resolved?

@Wetterquarz
Copy link
Contributor Author

Wetterquarz commented Feb 7, 2025

I must thank you for this. Using a single character per ruby and storing the details in the style is a really elegant solution to the line wrapping and overflow issues. If you feel this PR is ready, I'm happy to merge it.

  • Autocomplete suggestions are rendered too far left

I'm not entirely sure what this point is referring to. Has this been resolved?

It is about the chat, where you can autocomplete names or parts of commands and ghosts of the suggestions is slightly shifted when not using Caxton. Unfortunately, it is still not fixed and have yet to find the source of the problem.

Edit: I just found the problem is more generally characters to the right of cursor are rendered to far left.

@Wetterquarz
Copy link
Contributor Author

@keve1227, I believe it is now ready for merging.

(Due to me breaking the width receiver mixin with 1cec407)
@keve1227
Copy link
Owner

keve1227 commented Feb 9, 2025

Now it's ready.

@keve1227 keve1227 merged commit 0c7303c into keve1227:1.21.3 Feb 9, 2025
2 checks passed
keve1227 added a commit that referenced this pull request Feb 9, 2025
Update to MC 1.21.4 based on PR #14
@keve1227 keve1227 mentioned this pull request Feb 10, 2025
keve1227 added a commit that referenced this pull request Feb 10, 2025
Fix desync charactervisitors and Caxton compatibility
keve1227 added a commit that referenced this pull request Feb 10, 2025
Fix desync charactervisitors and Caxton compatibility
keve1227 added a commit that referenced this pull request Feb 10, 2025
Fix desync charactervisitors and Caxton compatibility
keve1227 added a commit that referenced this pull request Feb 10, 2025
Fix desync charactervisitors and Caxton compatibility
@Wetterquarz Wetterquarz deleted the fix-desync-charactorvisitors branch February 11, 2025 17:23
@Tahlpok
Copy link

Tahlpok commented Feb 15, 2025

Hey, I was abble to try it it worked ! Unfortunatly most of the mod are still stucked in 1.21.1 so i cannot play with the 1.21.3 mod. Is there a way to lunch this mod in this version ? or do you think that you can also correct the bug in 1.21.1.

Thank you in advance !

@Wetterquarz
Copy link
Contributor Author

Hey, I was abble to try it it worked ! Unfortunatly most of the mod are still stucked in 1.21.1 so i cannot play with the 1.21.3 mod. Is there a way to lunch this mod in this version ? or do you think that you can also correct the bug in 1.21.1.

Thank you in advance !

I have started work on this and, except for a small problem which won't get in the way, it seems to work. This work can be seen in #18. You can also download the artifact from there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Advancement Menu Causes Game Freeze With Some Advancements Visible
3 participants