Dehardcode Jukebox to allow modders to extend.

[Waterpicker]

It not currently possible to extend JukeboxEntity and RecordItem logic for inserting a record into a jukebox only works specifically for the vanilla jukebox.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[ChampionAsh5357]

What about jukebox behavior on parrots and allays? In addition, it feels sort of strange to only have a method of inserting and not getting/removing. Could you justify this?

[Waterpicker]

Thank you for bringing this up. A similar hardcoded situation exists for their AIs. Correcting that now.

[Waterpicker]

And updated for Allay and Parrots.

[ChampionAsh5357]

In general, I still think there needs to be justification for not including a getter or a removing method for the record.

[Waterpicker]

In general, I still think there needs to be justification for not including a getter or a removing method for the record.

I can do the getter easily. The removing method is a bit tricky. The 'remove' logic of the jukebox is heavily coupled with the onUse method. Aka I would have to cobble together a method from scratch.

[sciwhiz12]

Just making some notes on formatting and javadocs, and one note about the naming of the extension interface. I'll leave it up to others (or my future self) to review the functioning of the PR.

[ChampionAsh5357]

There are some comments above you didn't address. Additionally, for the ones you did, you should resolve the conversation.

[TeamSpen210]

As a convenience, perhaps this could delegate to getRecord() if not a vanilla jukebox?

[ChampionAsh5357]

I'm confused, why would a boolean method delegate to #getRecord? Additionally, #getRecord already delegates to this.

[TeamSpen210]

What it could do is call this.getRecord(), then return whether that returned an non-empty item. That way custom jukebox blocks that don't have a cached has-record value could only implement getRecord(), and have hasRecord() do the right thing.

[ChampionAsh5357]

The value of #getRecord would be called twice, then. If that strategy was adopted, then it would make no sense to have a #hasRecord at all and just implement the logic into a single method.

This is not to mention I can see cases where somebody decides to check the item and wonder why the client and server are desynced to some capacity. To implement that more complex logic, they would need to properly handle any tags which may change the logic. They would be overriding both methods anyways in that case. As such, I don't think it makes sense to do so.

[ChampionAsh5357]

As a quick aside, how it JukeboxBlock#dropRecording, JukeboxBlock#getAnalogOutputSignal, JukeboxBlockEntity#playRecordTick, and the save methods in the JukeboxBlockEntity being considered when dealing with the new extension interface for #getRecord, because it seems like that still doesn't redirect to these new calls.

[autoforge]

@Waterpicker, this pull request has conflicts, please resolve them for this PR to move forward.