search

chrrs / scribble

Fabric mod to expertly edit your books with rich formatting options and page utilities
https://modrinth.com/mod/scribble
MIT License
4 stars 3 forks source link

Symbol chat compatibility #28

Open replaceitem opened 1 week ago

replaceitem commented 1 week ago

Hi there, there is currently a problem again with my mod Symbol chat, where Symbol fonts don't work because it redirects the creation of the selection manager. Then your mod replaces that afterwards with the RichSelectionManager: https://github.com/replaceitem/symbol-chat/issues/55

I realized this approach of symbol chat was kind of bad (replacing the whole selection manager to just modify the insert method), so i am currently rewriting it to instead redirect/wrapOp the selectionManager.insert call in the book edit screen. However, i noticed your mod also injects before that call and cancels the method, meaning that wouldn't work.

I think ideally you would use WrapOperation to replace the inject call with your implementation https://github.com/chrrs/scribble/blob/30ec1f6c502825eb7e9749161ec3e7e47c1ecff9/src/main/java/me/chrr/scribble/mixin/BookEditScreenMixin.java#L679-L680 without cancelling the method, so i can do a WrapOperation too and add code before and after, and modify an argument.

I am not sure though whether you can even use the WrapOperation of mixin extra, since you support other loaders than just fabric (which has this built in now) that might not work with mixin extras?

I would appreciate any input on this!

chrrs commented 1 week ago

Hi! That seems fair, I'll rewrite some of the mixins to be more friendly to other mods. MixinExtras shouldn't be a problem, as it's built-in into later versions of neoforge, and for earlier versions I don't mind JiJ'ing it.

You're right about the selection manager, I think your new approach should work once I've changed the mixin! Please do mention if there's any other problem, I'll see what I can do!

replaceitem commented 1 week ago

Thanks a lot, no pressure though, this is a minor issue. I will likely push my changes to symbol chat later today, also using mixins extra now.

chrrs commented 1 week ago

I ended up using redirects instead, I tested this with the latest commit from Symbol Chat, and it looks like everything works now!

replaceitem commented 1 week ago

Nice, thank you. Although redirect is supposed to have hard conflicts if two mods want to redirect the same thing. Maybe it still worked because symbol chat uses wrap operation now and the mixin was applied first/last? Not sure...