Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Seems like #4681 has been reintroducted. #6811

Open
1 task done
friyes opened this issue Jun 23, 2024 · 6 comments
Open
1 task done

Seems like #4681 has been reintroducted. #6811

friyes opened this issue Jun 23, 2024 · 6 comments
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. PR available Issues which have a yet-to-be merged PR resolving it

Comments

@friyes
Copy link

friyes commented Jun 23, 2024

Skript/Server Version

[20:50:36 INFO]: [Skript] Skript's aliases can be found here: https://github.com/SkriptLang/skript-aliases
[20:50:36 INFO]: [Skript] Skript's documentation can be found here: https://docs.skriptlang.org/
[20:50:36 INFO]: [Skript] Skript's tutorials can be found here: https://docs.skriptlang.org/tutorials
[20:50:36 INFO]: [Skript] Server Version: 1.20.6-137-bd5867a (MC: 1.20.6)
[20:50:36 INFO]: [Skript] Skript Version: 2.8.7 (skriptlang-github)
[20:50:36 INFO]: [Skript] Installed Skript Addons:
[20:50:36 INFO]: [Skript]  - skript-placeholders v1.6.0 (https://github.com/APickledWalrus/skript-placeholders)
[20:50:36 INFO]: [Skript]  - skNoise v1.0
[20:50:36 INFO]: [Skript]  - skript-particle v1.3.1 (https://github.com/sovdeeth/skript-particle)
[20:50:36 INFO]: [Skript]  - DiSky v4.17.2
[20:50:36 INFO]: [Skript]  - skript-reflect v2.4 (https://github.com/SkriptLang/skript-reflect)
[20:50:36 INFO]: [Skript]  - SkBee v3.5.5 (https://github.com/ShaneBeee/SkBee)
[20:50:36 INFO]: [Skript]  - SkJson v3.0.91
[20:50:36 INFO]: [Skript] Installed dependencies:
[20:50:36 INFO]: [Skript]  - Vault v1.7.3-b131
[20:50:36 INFO]: [Skript]  - WorldGuard v7.0.10+d9424b1

Bug Description

Like #4681 says,
Doing this,

on inventory open:
    broadcast event-inventory's holder's location

Will broadcast nothing when opening a double chest.

Expected Behavior

I expected to get the location like for single chests.

Steps to Reproduce

Upload and reload the example script.
Place a single chest and a double chest.
Right click both.
Observe that only the single chest returns its location.

Errors or Screenshots

No response

Other

No response

Agreement

  • I have read the guidelines above and affirm I am following them with this report.
@friyes friyes changed the title Seems like #4681 has been reintroducted...? Seems like #4681 has been reintroducted. Jun 23, 2024
@sovdeeth
Copy link
Member

You're sure it's double chests and not #6424 ?

@friyes
Copy link
Author

friyes commented Jun 23, 2024

You're sure it's double chests and not #6424 ?

This might be true,
broadcast event-inventory's holder returns,
'chest' at -66.5, 70.5, 133.5 in world 'world' for double chests and
BlockState{type=CHEST,location=Location{world=CraftWorld{name=world},x=-67.0,y=70.0,z=135.0,pitch=0.0,yaw=0.0}} for single chests...

@ShaneBeee
Copy link
Contributor

can confirm this
After talking to friyes on discord, I tried:
broadcast "Loc: %location of holder of event-inventory%"
for a single chest it sends the location
for a double chest it sends none

@sovdeeth sovdeeth reopened this Jun 23, 2024
@sovdeeth
Copy link
Member

Misclick whoops!

I meant to say that yeah, looks like double chests are silly and need their own check

@APickledWalrus
Copy link
Member

A holder instanceof DoubleChest check needs added to this converter:

Converters.registerConverter(InventoryHolder.class, Location.class, holder -> {
if (holder instanceof Entity)
return ((Entity) holder).getLocation();
if (holder instanceof Block)
return ((Block) holder).getLocation();
if (holder instanceof BlockState)
return BlockUtils.getLocation(((BlockState) holder).getBlock());
return null;
});

It can prefer to return the location of the left-side of the chest. Should be rather simple.

@APickledWalrus APickledWalrus added bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. good first issue An issue that would be good for a first-time contributor to make a PR for labels Jun 23, 2024
@Pikachu920
Copy link
Member

regression test too, please!

@sovdeeth sovdeeth added PR available Issues which have a yet-to-be merged PR resolving it and removed good first issue An issue that would be good for a first-time contributor to make a PR for labels Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. PR available Issues which have a yet-to-be merged PR resolving it
Projects
None yet
Development

No branches or pull requests

5 participants