Update SlotWithIndex and InventorySlot Comparisons

[Fusezion]

Description

This PR aims to change how the comparison of SlotWihtIndex behaviors, old behavior would cause some relatively less than desired behavior if two different inventories were comparing with the same index.

This would be a mild breaking change to some degree on servers, however this change should attempt to make the comparison a bit more stricter. Only two areas in skript actually set the raw index accordingly and that's clicked slot and event-slot for InventoryClickEvent

Everywhere else handles it as this.index = this.rawIndex = index

an example of this change being in effect is

on inventory click:
    event-slot is slot 4 of top inventory of player
    broadcast "Clicked slot 4 of top Inventory"

Before this would return true for both the bottom and top, now only when it's the top

---

Target Minecraft Versions: any
Requirements: none
Related Issues: #5928

[Efnilite]

what do you think about deprecating isSameSlot and overriding equals instead with the new correct behaviour?

[Fusezion]

what do you think about deprecating isSameSlot and overriding equals instead with the new correct behaviour?

I'd be perfectly with that type of implementation, however I'd like to hear one or two more team member's opinions before I go and do that change

[sovdeeth]

what do you think about deprecating isSameSlot and overriding equals instead with the new correct behaviour?

I'd be perfectly with that type of implementation, however I'd like to hear one or two more team member's opinions before I go and do that change

Makes sense to me, remember to change the slot comparator accordingly and check for any breaking changes.

[Fusezion]

Alright I've gone ahead and committed the deprecation as well as update their related classes.
I've changed the slot to slot comparison, I was unable to find any other calls that need fixed. At most this might be some Slot#equals(Slot) that existed prior and would be prone to poor behavior in the past.

Do we believe the Slot -> Number comparison should also be updated to raw index if possible? I was testing and realized that the below code would pass where as slot 0 of player's top inventory is event-slot will not.

on inventory click:
    event-slot is 0
    broadcast "passed"

[sovdeeth]

Alright I've gone ahead and committed the deprecation as well as update their related classes. I've changed the slot to slot comparison, I was unable to find any other calls that need fixed. At most this might be some Slot#equals(Slot) that existed prior and would be prone to poor behavior in the past.

Do we believe the Slot -> Number comparison should also be updated to raw index if possible? I was testing and realized that the below code would pass where as slot 0 of player's top inventory is event-slot will not.

on inventory click:
    event-slot is 0
    broadcast "passed"

No, I think it should maintain the same behavior as index of event-slot (which shouldn't change based on the current inventory view). Also, have you tested things like event-slot is player's tool, or event-slot is slot 0 of player's inventory?

[Fusezion]

Also, have you tested things like event-slot is player's tool, or event-slot is slot 0 of player's inventory?

l've tested the latter, but I will test the former shortly

[sovdeeth]

Based on some testing, I believe the rawIndex approach is fundamentally flawed and there's a better and easier solution by implementing isSameSlot on InventorySlot to compare inventories as well as indices. See https://discord.com/channels/135877399391764480/836220422223036467/1353113589841531021 for discussion.

[sovdeeth]

Sorry to flip flop my opinion here but after looking into it further I don't think equals is appropriate here. The behavior implemented does not satisfy the constraints the javadocs for equals() set out. I think it's best to stick with isSameSlot.

[Fusezion]

I've reverted the isSameSlot deprecation and equals, along with added a few public methods for developers to get the core object references in the classes.

i.e. DisplayEntitySlot getItemDisplay(), ThrowableProjectileSlot getProjectile(), while not currently used outside of their isSameSlot comparisons this at least grants access to developers freely.