search

MoonflowerTeam / pollen

⚙️ Library for all Moonflower mods. Adds bridges for Fabric and Forge using Architectury along with utilities such as our Pinwheel bedrock animation API.
https://www.curseforge.com/minecraft/mc-mods/pollen
Other
32 stars 19 forks source link

Data modification system is broken #36

Closed ghost closed 2 years ago

ghost commented 2 years ago

What version are you seeing the problem on?

1.18.2

Describe the issue

## Info
*Fabric Loader*: 1.14.5
*Pollen*: Tested on both latest released (1.4.1) and in a locally built version (https://github.com/MoonflowerTeam/pollen/commit/b2cff5cfd4f8a1ee2b2229e446afce5df166cf85)

### Main Issue

The data modification system never loads/reloads, due to `ReloadStartListener#onReloadStart` only being called on the client - `ReloadableResourceManager#createReload` (Which you inject to run those specific listeners) is the reload creator for the client.

This can be seen by checking the log, the second contains only pollen and fabric api, and the first, a mixin patch I made.
https://gist.github.com/alkyaly/c6b5f4248381b75df02d5976681e4508 - The logging for the modifier count only appears on the first (*Line 90*), indicating that only there the delegating listener apply for the `SidedReloader` is being called.

### Inject Shenanigans

Additionally, the fix https://github.com/MoonflowerTeam/pollen/commit/553f78a5e0ef1013faafad9aa014a21313ba1b9f to #30, makes the data modification system "fail" silently instead of hard failing, making none of the modifications to be added to the map.

Some mods, notably the `Quilt Standard Libraries` utilize of a similar inject to `ServerResourcesMixin#addListeners`, Injects are not chainable when cancelling/setting the ret value, making only the first mixin to be applied run its code, which is why the `serverReloader` was null, as it was never set or added to the list.

If possible, I shall say that using the resource manager api from Fabric API may be better.
If it's not possible to utilize the api, the second issue may be solved by using Mixin-Extras `@ModifyReturnValue` instead, which is chainable.

latest.log

https://gist.github.com/alkyaly/c6b5f4248381b75df02d5976681e4508
LambdAurora commented 2 years ago

Further information on fixing this issue: