Skip to content

Implement #canTranslate in GlobalTranslatorImpl and call #canTranslate of its sources #1252

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

Open
wants to merge 2 commits into
base: main/4
Choose a base branch
from

Conversation

ivi-kiwi
Copy link

Similar to the other methods in the GlobalTranslator, it simply iterates through the sources and checks whether a translation is possible there.

The reason for the proposed change is that if you add a source to the GlobalTranslator which overwrites #canTranslate, the logic in there is not taken into account.

In my individual case, this is necessary because I have my own implementations of TranslatableComponent in my project, to which my Translator only reacts if the instance passed to #translate is of the type of my own implementation and therefore my Translator is no longer addressed at all.

Copy link
Member

@kezz kezz left a comment

Choose a reason for hiding this comment

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

See build failures and suggestion.

For reference, although this PR is a good change, your use case of having your own implementation of TranslatableComponent is entirely unsupported and will break in the upcoming Adventure 5.0 release. I would suggest removing your use case for that or migrating to virtual components instead.

@ivi-kiwi
Copy link
Author

For reference, although this PR is a good change, your use case of having your own implementation of TranslatableComponent is entirely unsupported and will break in the upcoming Adventure 5.0 release. I would suggest removing your use case for that or migrating to virtual components instead.

Oh, then I'll look into finding a better solution for my case. But it was a handy solution, and it worked very well until the change. 😅

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

Successfully merging this pull request may close these issues.

2 participants