someaddons / ClickableAdvancements

Minecraft mod for clickable advancements
2 stars 0 forks source link

[Bug]: incompatibility with advancement-reload mod #24

Open 42atomys opened 1 week ago

42atomys commented 1 week ago

Describe the bug you're experiencing

Hi, I'm the creator of the mod advancements-reloaded, and a player reached out a few weeks ago regarding an incompatibility between our mods. I tried to find the source code to understand what you're calling to attempt to fix the issue, but your code isn't public. Before implementing what your mod does, I’d prefer to discuss with you to ensure compatibility. :)

Original issue : https://github.com/42atomys/fabric-advancementinfo-reloaded/issues/16

Reproducability

  1. Minecraft version 1.21 / 1.21.1 The list below don't list above 1.19
  2. Have both mods installed
  3. Try to click on an advancement, the new UI dont open.

Mod up to date

Minecraft version

1.19

Modloader version

Fabric

Logs

n/a

someaddons commented 2 days ago

you just need to select the right branch to view the code :D https://github.com/someaddons/ClickableAdvancements/tree/fabric1.21

you're probably interested in: https://github.com/someaddons/ClickableAdvancements/blob/f30d69d74f0adb5b1e0a5212d04ae1d8eeda4ad9/src/main/java/com/clickadv/event/ClientEventHandler.java#L93

if I understood it right that you're adding a more fancy advancement window instead of the vanilla one. Though if yours did extend the vanilla type and replaced the screen during setScreen with your custom one it should work by default without any need of explicit compat

42atomys commented 13 hours ago

Good thing, I don't think to look at branches !

I have rework a lot of code and AdvancementReloadedScreen aren't extended from AdvancementScreen but from Screen directly (act as replacement) to prevent a lot of mixin !

In my case I have also rework some part of the AdvancementTab to increase performance when rendering thousand of advancement in the same tab and the line https://github.com/someaddons/ClickableAdvancements/blob/f30d69d74f0adb5b1e0a5212d04ae1d8eeda4ad9/src/main/java/com/clickadv/event/ClientEventHandler.java#L109 cannot be called because scroll aren't present anymore 😢

I think the "same" behavior will be AdvancementReloadedTad#move, need to be tried.

Maybe the "best solution" are to create an interface of your need, in addition of the vanilla behavior, and I will match it (ensure future compatibility with others mods in the same time).

WDYT ?