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

Mod is incompatible with other mods that add timestamps #23

Closed
70CentsApple opened this issue Jun 24, 2023 · 10 comments · Fixed by #24 or #32
Closed

Mod is incompatible with other mods that add timestamps #23

70CentsApple opened this issue Jun 24, 2023 · 10 comments · Fixed by #24 or #32
Assignees
Labels
bug Something isn't working fixed This issue is fixed, just not released yet
Milestone

Comments

@70CentsApple
Copy link
Contributor

Some mods (e.g. Tweakeroo, Chat Tools etc.) adds a timestamp in front of the messages, however it doesn't work very well with Compact Chat. Also, I found a former issue that is similar to my problem that is #21.

I can give you a RegEx that can matches Timestamps like [12:34:56] or [12:34], that is \[?\d{1,2}:\d{2}(:\d{2})*\]? * .

@caoimhebyrne
Copy link
Owner

I'll take a look at this again. Thanks for the report!

@caoimhebyrne caoimhebyrne added the bug Something isn't working label Jun 24, 2023
@caoimhebyrne caoimhebyrne self-assigned this Jun 24, 2023
@caoimhebyrne caoimhebyrne changed the title [Feature Request] Compatible with timestamps Mod is incompatible with other mods that add timestamps Jun 24, 2023
@caoimhebyrne
Copy link
Owner

caoimhebyrne commented Jun 28, 2023

TL;DR: I have to look into this a bit more to see if I can find a nice solution. Sorry about the delay on this!

Tweakeroo does a bit of manipulation on the text (a lot more than I'd like them to), which makes it quite hard to compare the text when removing the timestamps.

The main issue with that is, our mixin runs before Tweakeroo's (probably due to the name of our mixin ChatHudMixin vs theirs MixinChatHud), so the message that we first get (to append the occurrences count) doesn't contain the timestamp. The only time we see a message with a timestamp is when we go to remove old occurrences of the message. This is fine, but again, not ideal, as they modify the hierarchy of the message a bit, making it hard to see if the messages are equal after removing the timestamp.

A Text component in Minecraft comes in many forms, and can have many different properties.

A message, modified by Tweakeroo, with the timestamp stripped by Compact Chat

image

A normal chat message

image

Yes, I've tried just comparing them with a getString, that brings suboptimal results.

@caoimhebyrne
Copy link
Owner

Hey! Sorry about the delay on this, I'm hopefully going to get back to this soon @70CentsApple.

@70CentsApple
Copy link
Contributor Author

no matter, you can finish it whenever you like :)

@caoimhebyrne
Copy link
Owner

I've made an upstream patch to Tweakeroo to help add support for CompactChat, it will also need changes on my side. If there's any other mods that you're aware of, can you make sure they follow similar functionality to what I've demonstrated in my patch?

maruohon/tweakeroo#447

@70CentsApple
Copy link
Contributor Author

I've made an upstream patch to Tweakeroo to help add support for CompactChat, it will also need changes on my side. If there's any other mods that you're aware of, can you make sure they follow similar functionality to what I've demonstrated in my patch?

maruohon/tweakeroo#447

yeah thank you

caoimhebyrne added a commit that referenced this issue Jul 14, 2023
Fixes #23.

If you're using Tweakeroo, this requires an upstream change: maruohon/tweakeroo#447
@caoimhebyrne
Copy link
Owner

I think ChatTools should be compatible with this now, I've committed it to another branch until I do some more testing, see 3568670.

https://github.com/70CentsApple/ChatTools/blob/1.20/src/main/java/net/apple70cents/chattools/mixin/ChatHudMixin.java

caoimhebyrne added a commit that referenced this issue Jul 14, 2023
Fixes #23.

If you're using Tweakeroo, this requires an upstream change: maruohon/tweakeroo#447
@caoimhebyrne caoimhebyrne reopened this Jul 14, 2023
@caoimhebyrne
Copy link
Owner

Going to keep this open until the release is public.

@caoimhebyrne caoimhebyrne added the fixed This issue is fixed, just not released yet label Jul 14, 2023
@caoimhebyrne caoimhebyrne added this to the 2.0.2 milestone Jul 14, 2023
@caoimhebyrne
Copy link
Owner

This has now been published to Modrinth and GitHub Releases.

@70CentsApple
Copy link
Contributor Author

Hey! I'm sorry to inform you that such issue can be easily reproduced once again in MC 1.20.4 due to some probable reasons. However, I'm glad to inform that I have already figure it out and I could make a PR for that! @caoimhebyrne

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed This issue is fixed, just not released yet
Projects
None yet
2 participants