Sinytra / ForgifiedFabricAPI

Fabric API implemented on top of NeoForge
https://sinytra.org/docs
Apache License 2.0
113 stars 15 forks source link

Jukebox plays Music Disc again on the "off" end #87

Closed ItoKyoshiro closed 9 months ago

ItoKyoshiro commented 9 months ago

Describe the bug

When a Music Disc ends playing, it start playing again (with redstone emitted from the Jukebox and everything) if you have a Hopper below it. It extract the Disc but still plays the music fully

Steps to reproduce

1.Play a Music Disc onto a Jukebox 2.Extract it via Hopper (when Disc ends, since Hopper is locked by Jukebox) 3.Music Disc start playing again (locking the Hopper, which already has the Music Disc)

Logs

No response

Additional context

I used Mellohi as a test because it's the shorter one (01:36) but can confirm it happens with Far (02:54) and Wait (03:58), so my guess is this is extended to all the Discs.

MODS: Connector-1.0.0-beta.32+1.20.1 ConnectorExtras-1.8.0+1.20.1 fabric-api-0.90.7+1.10.3+1.20.1

niklaswimmer commented 9 months ago

I was able to replicate this issue. The music starts again only once, but the jukebox seems to be in an invalid state afterwards, as it is not possible to interact with it anymore (it also constantly outputs a redstone signal, even after the music stops).

Because Connector itself does not have any mixins (as far as I know) that modify the Jukebox or Hopper behavior I currently suspect this to be a bug in sinytra/forgifiedfabricapi. I will investigate further and report back.

niklaswimmer commented 9 months ago

Jeah this is definitely a problem with the forgified fabric transfer api. Maybe @Su5eD can move this issue while I try to figure out why this does not happen with the normal fabric transfer api and how we can fix this?

Su5eD commented 9 months ago

Sure

niklaswimmer commented 9 months ago

I traced this back to a bug in the standard Fabric API, see more at fabricmc/fabric#3485.

We could fix the issue on our side if we somehow not provide an IItemHandler for Vanilla block entities. I feel like that is an appropriate thing to do, because really it is not our job to make Vanilla inventories appear as IItemHandler's - that seems like Forge's job. However, @Su5eD will have to decide.